On 2/15/23 13:37, Marek Polacek wrote:
On Wed, Feb 15, 2023 at 02:39:16PM -0500, Jason Merrill wrote:
On 2/9/23 09:39, Marek Polacek wrote:
In constexpr-nsdmi3.C, with -fno-elide-constructors, we don't elide
the Y::Y(const Y&) call used to initialize o.c. So store_init_value
-> cxx_constant_init must constexpr-evaluate the call to Y::Y(const Y&)
in cxx_eval_call_expression. It's a trivial function, so we do the
"Shortcut trivial constructor/op=" code and rather than evaluating
the function, we just create an assignment
o.c = *(const struct Y &) (const struct Y *) &(&<PLACEHOLDER_EXPR struct
X>)->b
which is a MODIFY_EXPR, so the preeval code in cxx_eval_store_expression
clears .ctor and .object, therefore we can't replace the PLACEHOLDER_EXPR
whereupon we crash at
/* A placeholder without a referent. We can get here when
checking whether NSDMIs are noexcept, or in massage_init_elt;
just say it's non-constant for now. */
gcc_assert (ctx->quiet);
The PLACEHOLDER_EXPR can also be on the LHS as in constexpr-nsdmi10.C.
I don't think we can do much here, but I noticed that the whole
trivial_fn_p (fun) block is only entered when -fno-elide-constructors.
This is true since GCC 9; it wasn't easy to bisect what changes made it
so, but r240845 is probably one of them. -fno-elide-constructors is an
option for experiments only so it's not clear to me why we'd still want
to shortcut trivial constructor/op=. I propose to remove the code and
add a checking assert to make sure we're not getting a trivial_fn_p
unless -fno-elide-constructors.
Hmm, trivial op= doesn't ever hit this code?
With -fno-elide-constructors we hit the trivial_fn_p block twice in
constexpr-nsdmi9.C, once for "constexpr Y::Y(const Y&)" and then for
"constexpr Y& Y::operator=(Y&&)". So it does hit the code, but only
with -fno-elide-constructors.
Odd, I'm not sure why that would make a difference for assignment.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? I don't
think I want to backport this.
PR c++/101073
gcc/cp/ChangeLog:
* constexpr.cc (cxx_eval_call_expression): Replace shortcutting trivial
constructor/op= with a checking assert.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/constexpr-nsdmi3.C: New test.
* g++.dg/cpp1y/constexpr-nsdmi10.C: New test.
---
gcc/cp/constexpr.cc | 25 +++----------------
gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi3.C | 17 +++++++++++++
.../g++.dg/cpp1y/constexpr-nsdmi10.C | 18 +++++++++++++
3 files changed, 38 insertions(+), 22 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-nsdmi3.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-nsdmi10.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 564766c8a00..1d53dcf0f20 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -2865,28 +2865,9 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree
t,
ctx = &new_ctx;
}
- /* Shortcut trivial constructor/op=. */
- if (trivial_fn_p (fun))
- {
- tree init = NULL_TREE;
- if (call_expr_nargs (t) == 2)
- init = convert_from_reference (get_nth_callarg (t, 1));
- else if (TREE_CODE (t) == AGGR_INIT_EXPR
- && AGGR_INIT_ZERO_FIRST (t))
- init = build_zero_init (DECL_CONTEXT (fun), NULL_TREE, false);
- if (init)
- {
- tree op = get_nth_callarg (t, 0);
- if (is_dummy_object (op))
- op = ctx->object;
- else
- op = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (op)), op);
- tree set = build2 (MODIFY_EXPR, TREE_TYPE (op), op, init);
I think the problem is using MODIFY_EXPR instead of INIT_EXPR to represent a
constructor; that's why cxx_eval_store_expression thinks it's OK to
preevaluate. This should properly use those two tree codes for op= and
ctor, respectively.
Maybe it was so that the RHS in SET could refer to the op in the LHS?
I think it was just an oversight. You need INIT_EXPR for the rhs to
refer to the lhs.
- new_ctx.call = &new_call;
- return cxx_eval_constant_expression (&new_ctx, set, lval,
- non_constant_p, overflow_p);
- }
- }
+ /* We used to shortcut trivial constructor/op= here, but nowadays
+ we can only get a trivial function here with -fno-elide-constructors. */
+ gcc_checking_assert (!trivial_fn_p (fun) || !flag_elide_constructors);
...but if this optimization is so rarely triggered, this simplification is
OK too.
I'd say that's better so that we don't have to update the code (like
r234345 did).
Indeed, the patch is OK.
Jason
