On Wed, Jan 29, 2025 at 08:03:37AM -0500, Jason Merrill wrote:
> On 1/27/25 6:19 PM, Marek Polacek wrote:
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/14?
> >
> > -- >8 --
> > We've had a wrong-code problem since r14-4140, due to which we
> > forget to initialize a variable.
> >
> > In consteval39.C, we evaluate
> >
> > struct QQQ q;
> > <<cleanup_point <<< Unknown tree: expr_stmt
> > QQQ::QQQ (&q, TARGET_EXPR <D.2687, <<< Unknown tree: aggr_init_expr
> > 5
> > __ct_comp
> > D.2687
> > (struct basic_string_view *) <<< Unknown tree: void_cst >>>
> > (const char *) "" >>>>) >>>>>;
> >
> > into
> >
> > struct QQQ q;
> > <<cleanup_point <<< Unknown tree: expr_stmt
> > {.data={._M_len=42, ._M_str=0}} >>>>>;
> >
> > and then the useless expr_stmt is dropped on the floor, so q isn't
> > initialized. As pre-r14-4140, we need to handle constructors specially.
>
> Hmm, why didn't the following code in eval_outermost make this a
> rejects-valid bug rather than wrong-code?
>
> > /* If T is calling a constructor to initialize an object,
> > reframe
> > it as an AGGR_INIT_EXPR to avoid trying to modify an object
> > from outside the constant evaluation, which will fail even if
> > the value is actually constant (is_constant_evaluated3.C). */
So yes, we go here, create an AGGR_INIT_EXPR, then evaluate it into
{.data={._M_len=42, ._M_str=0}}. What should give an error?
> Your change should share code with this block doing the same thing in
> cp_fold_r:
>
> > if (TREE_CODE (r) != CALL_EXPR)
> > {
> > if (DECL_CONSTRUCTOR_P (callee))
> > {
> > loc = EXPR_LOCATION (x);
> > tree a = CALL_EXPR_ARG (x, 0);
> > bool return_this = targetm.cxx.cdtor_returns_this ();
> > if (return_this)
> > a = cp_save_expr (a);
> > tree s = build_fold_indirect_ref_loc (loc, a);
> > r = cp_build_init_expr (s, r);
> > if (return_this)
> > r = build2_loc (loc, COMPOUND_EXPR, TREE_TYPE (x), r,
> > fold_convert_loc (loc, TREE_TYPE (x), a));
> > }
> > x = r;
> > break;
> > }
>
> Like there, we shouldn't need this for AGGR_INIT_EXPR, only CALL_EXPR.
Okay, but that then means that I can't call cxx_constant_value with an
object, otherwise I don't think I can unify the code above. Which is
fine, we go down the eval_outermost/AGGR_INIT_EXPR path as mentioned
above. At least I hope that is okay.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/14?
-- >8 --
We've had a wrong-code problem since r14-4140, due to which we
forget to initialize a variable.
In consteval39.C, we evaluate
struct QQQ q;
<<cleanup_point <<< Unknown tree: expr_stmt
QQQ::QQQ (&q, TARGET_EXPR <D.2687, <<< Unknown tree: aggr_init_expr
5
__ct_comp
D.2687
(struct basic_string_view *) <<< Unknown tree: void_cst >>>
(const char *) "" >>>>) >>>>>;
into
struct QQQ q;
<<cleanup_point <<< Unknown tree: expr_stmt
{.data={._M_len=42, ._M_str=0}} >>>>>;
and then the useless expr_stmt is dropped on the floor, so q isn't
initialized. As pre-r14-4140, we need to handle constructors specially.
With this patch, we generate:
struct QQQ q;
<<cleanup_point <<< Unknown tree: expr_stmt
q = {.data={._M_len=42, ._M_str=0}} >>>>>;
initializing q properly.
PR c++/117501
gcc/cp/ChangeLog:
* cp-gimplify.cc (cp_build_init_expr_for_ctor): New.
(cp_fold_immediate_r): Call it.
(cp_fold): Break out code into cp_build_init_expr_for_ctor.
gcc/testsuite/ChangeLog:
* g++.dg/cpp2a/consteval39.C: New test.
* g++.dg/cpp2a/consteval40.C: New test.
---
gcc/cp/cp-gimplify.cc | 41 ++++++++++++++++--------
gcc/testsuite/g++.dg/cpp2a/consteval39.C | 27 ++++++++++++++++
gcc/testsuite/g++.dg/cpp2a/consteval40.C | 25 +++++++++++++++
3 files changed, 80 insertions(+), 13 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval39.C
create mode 100644 gcc/testsuite/g++.dg/cpp2a/consteval40.C
diff --git a/gcc/cp/cp-gimplify.cc b/gcc/cp/cp-gimplify.cc
index 4ec3de13008..bdbdcafec54 100644
--- a/gcc/cp/cp-gimplify.cc
+++ b/gcc/cp/cp-gimplify.cc
@@ -1182,6 +1182,27 @@ taking_address_of_imm_fn_error (tree expr, tree decl)
maybe_explain_promoted_consteval (loc, decl);
}
+/* Build up an INIT_EXPR to initialize the object of a constructor. CALL
+ is the CALL_EXPR for the constructor call; INIT is the initializer. */
+
+static tree
+cp_build_init_expr_for_ctor (tree call, tree init)
+{
+ tree a = CALL_EXPR_ARG (call, 0);
+ if (is_dummy_object (a))
+ return init;
+ const bool return_this = targetm.cxx.cdtor_returns_this ();
+ const location_t loc = EXPR_LOCATION (call);
+ if (return_this)
+ a = cp_save_expr (a);
+ tree s = build_fold_indirect_ref_loc (loc, a);
+ init = cp_build_init_expr (s, init);
+ if (return_this)
+ init = build2_loc (loc, COMPOUND_EXPR, TREE_TYPE (call), init,
+ fold_convert_loc (loc, TREE_TYPE (call), a));
+ return init;
+}
+
/* A subroutine of cp_fold_r to handle immediate functions. */
static tree
@@ -1297,7 +1318,12 @@ cp_fold_immediate_r (tree *stmt_p, int *walk_subtrees,
void *data_)
}
/* We've evaluated the consteval function call. */
if (call_p)
- *stmt_p = e;
+ {
+ if (code == CALL_EXPR && DECL_CONSTRUCTOR_P (decl))
+ *stmt_p = cp_build_init_expr_for_ctor (stmt, e);
+ else
+ *stmt_p = e;
+ }
}
/* We've encountered a function call that may turn out to be consteval
later. Store its caller so that we can ensure that the call is
@@ -3422,18 +3448,7 @@ cp_fold (tree x, fold_flags_t flags)
if (TREE_CODE (r) != CALL_EXPR)
{
if (DECL_CONSTRUCTOR_P (callee))
- {
- loc = EXPR_LOCATION (x);
- tree a = CALL_EXPR_ARG (x, 0);
- bool return_this = targetm.cxx.cdtor_returns_this ();
- if (return_this)
- a = cp_save_expr (a);
- tree s = build_fold_indirect_ref_loc (loc, a);
- r = cp_build_init_expr (s, r);
- if (return_this)
- r = build2_loc (loc, COMPOUND_EXPR, TREE_TYPE (x), r,
- fold_convert_loc (loc, TREE_TYPE (x), a));
- }
+ r = cp_build_init_expr_for_ctor (x, r);
x = r;
break;
}
diff --git a/gcc/testsuite/g++.dg/cpp2a/consteval39.C
b/gcc/testsuite/g++.dg/cpp2a/consteval39.C
new file mode 100644
index 00000000000..523e8260eab
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/consteval39.C
@@ -0,0 +1,27 @@
+// PR c++/117501
+// { dg-do run { target c++20 } }
+
+constexpr unsigned
+length ()
+{
+ bool __trans_tmp_1 = __builtin_is_constant_evaluated();
+ if (__trans_tmp_1)
+ return 42;
+ return 1;
+}
+struct basic_string_view {
+ constexpr basic_string_view(const char *) : _M_len{length()}, _M_str{} {}
+ long _M_len;
+ char _M_str;
+};
+struct QQQ {
+ consteval QQQ(basic_string_view d) : data(d) {}
+ basic_string_view data;
+};
+int
+main ()
+{
+ QQQ q("");
+ if (q.data._M_len != 42)
+ __builtin_abort ();
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/consteval40.C
b/gcc/testsuite/g++.dg/cpp2a/consteval40.C
new file mode 100644
index 00000000000..4d3ba20092b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/consteval40.C
@@ -0,0 +1,25 @@
+// PR c++/117501
+// { dg-do run { target c++20 } }
+
+constexpr int
+twiddle (int i)
+{
+ if (__builtin_is_constant_evaluated ())
+ return 3;
+ return i;
+}
+struct S {
+ constexpr S(int i) : i{twiddle (i)} {}
+ int i;
+};
+struct Q {
+ consteval Q(S s_) : s{s_, s_} {}
+ S s[2];
+};
+int
+main ()
+{
+ Q q(twiddle (42));
+ if (q.s[0].i != 3 || q.s[1].i != 3)
+ __builtin_abort ();
+}
base-commit: 1e819a997dd5507e52cafc540656fc15160322fd
--
2.48.1
