c++: ICE with ()-init and TARGET_EXPR eliding [PR116424]

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
Here we crash on a cp_gimplify_expr/TARGET_EXPR assert:

      gcc_checking_assert (!TARGET_EXPR_ELIDING_P (*expr_p)
                           || !TREE_ADDRESSABLE (TREE_TYPE (*expr_p)));

We cannot elide the TARGET_EXPR because we're taking its address.

It is set as eliding in massage_init_elt.  I've tried to not set
TARGET_EXPR_ELIDING_P when the context is not direct-initialization.
That didn't work: even when it's not direct-initialization now, it
can become one later, for instance, after split_nonconstant_init.
One problem is that replace_placeholders_for_class_temp_r will replace
placeholders in non-eliding TARGET_EXPRs with the slot, but if we then
elide the TARGET_EXPR, we end up with a "stray" VAR_DECL and crash.
(Only some TARGET_EXPRs are handled by replace_decl.)

I thought I'd have to go back to
<https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651163.html> but
then I realized that this problem occurrs only with ()-init but not
{}-init.  With {}-init, there is no problem, because we are clearing
TARGET_EXPR_ELIDING_P in process_init_constructor_record:

       /* We can't actually elide the temporary when initializing a
          potentially-overlapping field from a function that returns by
          value.  */
       if (ce->index
           && TREE_CODE (next) == TARGET_EXPR
           && unsafe_copy_elision_p (ce->index, next))
         TARGET_EXPR_ELIDING_P (next) = false;

But that does not happen for ()-init because we have no ce->index.
()-init doesn't allow brace elision so we don't really reshape them.

But I can just move the clearing a few lines down and then it handles
both ()-init and {}-init.

	PR c++/116424

gcc/cp/ChangeLog:

	* typeck2.cc (process_init_constructor_record): Move the clearing of
	TARGET_EXPR_ELIDING_P down.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/paren-init38.C: New test.
---
 gcc/cp/typeck2.cc                         | 14 +++++++-------
 gcc/testsuite/g++.dg/cpp2a/paren-init38.C | 20 ++++++++++++++++++++
 2 files changed, 27 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init38.C


base-commit: bdcd30e4711943cae70a1b47f8a63e96a94c02a0

On 8/28/24 11:21 AM, Marek Polacek wrote:
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

OK for trunk/13/14.

> -- >8 --
> Here we crash on a cp_gimplify_expr/TARGET_EXPR assert:
> 
>        gcc_checking_assert (!TARGET_EXPR_ELIDING_P (*expr_p)
>                             || !TREE_ADDRESSABLE (TREE_TYPE (*expr_p)));
> 
> We cannot elide the TARGET_EXPR because we're taking its address.
> 
> It is set as eliding in massage_init_elt.  I've tried to not set
> TARGET_EXPR_ELIDING_P when the context is not direct-initialization.
> That didn't work: even when it's not direct-initialization now, it
> can become one later, for instance, after split_nonconstant_init.
> One problem is that replace_placeholders_for_class_temp_r will replace
> placeholders in non-eliding TARGET_EXPRs with the slot, but if we then
> elide the TARGET_EXPR, we end up with a "stray" VAR_DECL and crash.
> (Only some TARGET_EXPRs are handled by replace_decl.)
> 
> I thought I'd have to go back to
> <https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651163.html> but
> then I realized that this problem occurrs only with ()-init but not
> {}-init.  With {}-init, there is no problem, because we are clearing
> TARGET_EXPR_ELIDING_P in process_init_constructor_record:
> 
>         /* We can't actually elide the temporary when initializing a
>            potentially-overlapping field from a function that returns by
>            value.  */
>         if (ce->index
>             && TREE_CODE (next) == TARGET_EXPR
>             && unsafe_copy_elision_p (ce->index, next))
>           TARGET_EXPR_ELIDING_P (next) = false;
> 
> But that does not happen for ()-init because we have no ce->index.
> ()-init doesn't allow brace elision so we don't really reshape them.
> 
> But I can just move the clearing a few lines down and then it handles
> both ()-init and {}-init.
> 
> 	PR c++/116424
> 
> gcc/cp/ChangeLog:
> 
> 	* typeck2.cc (process_init_constructor_record): Move the clearing of
> 	TARGET_EXPR_ELIDING_P down.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/paren-init38.C: New test.
> ---
>   gcc/cp/typeck2.cc                         | 14 +++++++-------
>   gcc/testsuite/g++.dg/cpp2a/paren-init38.C | 20 ++++++++++++++++++++
>   2 files changed, 27 insertions(+), 7 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/paren-init38.C
> 
> diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc
> index 30a6fbe95c9..a0c8f833ac1 100644
> --- a/gcc/cp/typeck2.cc
> +++ b/gcc/cp/typeck2.cc
> @@ -1774,13 +1774,6 @@ process_init_constructor_record (tree type, tree init, int nested, int flags,
>   	    {
>   	      gcc_assert (ce->value);
>   	      next = massage_init_elt (fldtype, next, nested, flags, complain);
> -	      /* We can't actually elide the temporary when initializing a
> -		 potentially-overlapping field from a function that returns by
> -		 value.  */
> -	      if (ce->index
> -		  && TREE_CODE (next) == TARGET_EXPR
> -		  && unsafe_copy_elision_p (ce->index, next))
> -		TARGET_EXPR_ELIDING_P (next) = false;
>   	      ++idx;
>   	    }
>   	}
> @@ -1873,6 +1866,13 @@ process_init_constructor_record (tree type, tree init, int nested, int flags,
>   	    }
>   	}
>   
> +      /* We can't actually elide the temporary when initializing a
> +	 potentially-overlapping field from a function that returns by
> +	 value.  */
> +      if (TREE_CODE (next) == TARGET_EXPR
> +	  && unsafe_copy_elision_p (field, next))
> +	TARGET_EXPR_ELIDING_P (next) = false;
> +
>         if (is_empty_field (field)
>   	  && !TREE_SIDE_EFFECTS (next))
>   	/* Don't add trivial initialization of an empty base/field to the
> diff --git a/gcc/testsuite/g++.dg/cpp2a/paren-init38.C b/gcc/testsuite/g++.dg/cpp2a/paren-init38.C
> new file mode 100644
> index 00000000000..58743e051da
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/paren-init38.C
> @@ -0,0 +1,20 @@
> +// PR c++/116424
> +// { dg-do compile { target c++20 } }
> +
> +struct dd {
> +  char *ptr;
> +  dd();
> +  dd(dd &&__str);
> +};
> +struct v {
> +  dd n{};
> +  int f = -1;
> +  v operator|(const v &other) const;
> +};
> +struct cc : v {};
> +static const cc a;
> +static const cc b;
> +static const cc c1(a | b);
> +static const cc c2{a | b};
> +static const cc c3 = cc(a | b);
> +static const cc c4 = cc{a | b};
> 
> base-commit: bdcd30e4711943cae70a1b47f8a63e96a94c02a0