Re: [PATCH] c++: ICE with temporary of class type in array DMI [PR109966]

[Jason Merrill <jason@redhat.com>]

On 3/11/24 19:27, Marek Polacek wrote:
> 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.  I think the fix is
> to detect such a case in potential_prvalue_result_of.
> 
> 	PR c++/109966
> 
> gcc/cp/ChangeLog:
> 
> 	* typeck2.cc (potential_prvalue_result_of): Add walk_subtrees
> 	parameter.  Handle initializing an array from a
> 	brace-enclosed-initializer.
> 	(replace_placeholders_for_class_temp_r): Pass walk_subtrees down to
> 	potential_prvalue_result_of.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp1y/nsdmi-aggr20.C: New test.
> 	* g++.dg/cpp1y/nsdmi-aggr21.C: New test.
> ---
>   gcc/cp/typeck2.cc                         | 27 ++++++++---
>   gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr20.C | 17 +++++++
>   gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr21.C | 59 +++++++++++++++++++++++
>   3 files changed, 96 insertions(+), 7 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr20.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/nsdmi-aggr21.C
> 
> diff --git a/gcc/cp/typeck2.cc b/gcc/cp/typeck2.cc
> index 31198b2f9f5..8b99ce78e9a 100644
> --- a/gcc/cp/typeck2.cc
> +++ b/gcc/cp/typeck2.cc
> @@ -1406,46 +1406,59 @@ digest_init_flags (tree type, tree init, int flags, tsubst_flags_t complain)
>        A a = (A{});	      // initializer
>        A a = (1, A{});	      // initializer
>        A a = true ? A{} : A{};  // initializer
> +     A arr[1] = { A{} };      // initializer
>        auto x = A{}.x;	      // temporary materialization
>        auto x = foo(A{});	      // temporary materialization
>   
>      FULL_EXPR is the whole expression, SUBOB is its TARGET_EXPR subobject.  */
>   
>   static bool
> -potential_prvalue_result_of (tree subob, tree full_expr)
> +potential_prvalue_result_of (tree subob, tree full_expr, int *walk_subtrees)
>   {
> +#define RECUR(t) potential_prvalue_result_of (subob, t, walk_subtrees)
>     if (subob == full_expr)
>       return true;
>     else if (TREE_CODE (full_expr) == TARGET_EXPR)
>       {
>         tree init = TARGET_EXPR_INITIAL (full_expr);
>         if (TREE_CODE (init) == COND_EXPR)
> -	return (potential_prvalue_result_of (subob, TREE_OPERAND (init, 1))
> -		|| potential_prvalue_result_of (subob, TREE_OPERAND (init, 2)));
> +	return (RECUR (TREE_OPERAND (init, 1))
> +		|| RECUR (TREE_OPERAND (init, 2)));
>         else if (TREE_CODE (init) == COMPOUND_EXPR)
> -	return potential_prvalue_result_of (subob, TREE_OPERAND (init, 1));
> +	return RECUR (TREE_OPERAND (init, 1));
>         /* ??? I don't know if this can be hit.  */
>         else if (TREE_CODE (init) == PAREN_EXPR)
>   	{
>   	  gcc_checking_assert (false);
> -	  return potential_prvalue_result_of (subob, TREE_OPERAND (init, 0));
> +	  return RECUR (TREE_OPERAND (init, 0));
>   	}
>       }
> +  /* The array case listed above.  */
> +  else if (TREE_CODE (full_expr) == CONSTRUCTOR
> +	   && TREE_CODE (TREE_TYPE (full_expr)) == ARRAY_TYPE)
> +    for (constructor_elt &e: CONSTRUCTOR_ELTS (full_expr))
> +      if (e.value == subob)
> +	{
> +	  *walk_subtrees = 0;

Why clear walk_subtrees?  Won't that mean we fail to replace any 
placeholders nested within an array element initializer?

Jason