On 3/14/24 17:26, Marek Polacek wrote:
In the following patch, I'm taking a different tack. I believe
we ought to use TARGET_EXPR_ELIDING_P. The gimplify_arg bit I'm
talking about below is this:
/* Also strip a TARGET_EXPR that would force an extra copy. */
if (TREE_CODE (*arg_p) == TARGET_EXPR)
{
tree init = TARGET_EXPR_INITIAL (*arg_p);
if (init
&& !VOID_TYPE_P (TREE_TYPE (init)))
*arg_p = init;
}
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/13?
-- >8 --
This ICE started with the fairly complicated r13-765. We crash in
gimplify_var_or_parm_decl because a stray VAR_DECL leaked there.
The problem is ultimately that potential_prvalue_result_of wasn't
correctly handling arrays and replace_placeholders_for_class_temp_r
replaced a PLACEHOLDER_EXPR in a TARGET_EXPR which is used in the
context of copy elision. If I have
M m[2] = { M{""}, M{""} };
then we don't invoke the M(const M&) copy-ctor.
One part of the fix is to use TARGET_EXPR_ELIDING_P rather than
potential_prvalue_result_of. That unfortunately doesn't handle the
case like
struct N { N(M); };
N arr[2] = { M{""}, M{""} };
because TARGET_EXPRs that initialize a function argument are not
marked TARGET_EXPR_ELIDING_P even though gimplify_arg drops such
TARGET_EXPRs on the floor. We can use a pset to avoid replacing
placeholders in them.
I made an attempt to use set_target_expr_eliding in
convert_for_arg_passing but that regressed constexpr-diag1.C, and does
not seem like a prudent change in stage 4 anyway.
I tried the same thing to see what you mean, and that doesn't look like
a regression to me, just a different (and more accurate) diagnostic.
But you're right that this patch is safer, and the other approach can
wait for stage 1. Will you queue that up? In the mean time, this patch
is OK.
I just realized this could also check !TARGET_EXPR_ELIDING_P; there's no point
to adding an eliding TARGET_EXPR into the pset.
...with this change.
Jason
