Re: [PATCH] c++: unevaluated array new-expr size constantness [PR108219]

[Jason Merrill <jason@redhat.com>]

On 2/22/23 14:45, Patrick Palka wrote:
> Here we're mishandling the unevaluated array new-expressions due to a
> supposed non-constant array size ever since r12-5253-g4df7f8c79835d569,
> made us no longer perform constant evaluation of non-manifestly-constant
> expressions within unevaluated contexts.  This shouldn't make a
> difference here since the array sizes are constant literals, except
> they're actually NON_LVALUE_EXPR location wrappers wrapping INTEGER_CST,
> wrappers which used to get stripped as part of constant evaluation and
> now no longer do.  Moreover it means build_vec_init can't constant fold
> the 'maxindex' passed from build_new_1 (since it uses maybe_constant_value
> with mce_unknown).

Hmm, now that you mention it I think the

   if (manifestly_const_eval != mce_unknown)

change in maybe_constant_value isn't quite right, we don't want to force 
evaluation in unevaluated mce_false context either.

> This patch fixes the first issue by making maybe_constant_value and
> fold_non_dependent_expr_template shortcut handling location wrappers
> around constant nodes, and the second issue by using fold_build2_loc
> instead of cp_build_binary_op when computing the maxindex to pass to
> build_vec_init.

Maybe in unevaluated mce_unknown/false context maybe_constant_value 
should call fold?

> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk/12?
> 
> 	PR c++/108219
> 	PR c++/108218
> 
> gcc/cp/ChangeLog:
> 
> 	* constexpr.cc (maybe_constant_value): Extend the constant node
> 	shortcut to look through location wrappers too.
> 	(fold_non_dependent_expr_template): Mirror the constant node
> 	shortcut from maybe_constant_value.
> 	* init.cc (build_new_1): Use fold_build2_loc instead
> 	of cp_build_binary_op to build a MINUS_EXPR representing the
> 	maximum index.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/new6.C: New test.
> 	* g++.dg/cpp2a/concepts-new1.C: New test.
> ---
>   gcc/cp/constexpr.cc                        |  8 ++++++--
>   gcc/cp/init.cc                             | 18 ++++++++----------
>   gcc/testsuite/g++.dg/cpp0x/new6.C          |  9 +++++++++
>   gcc/testsuite/g++.dg/cpp2a/concepts-new1.C | 13 +++++++++++++
>   4 files changed, 36 insertions(+), 12 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/new6.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-new1.C
> 
> diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
> index aa2c14355f8..d38c4c80415 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -8538,9 +8538,9 @@ maybe_constant_value (tree t, tree decl /* = NULL_TREE */,
>   	t = mark_non_constant (t);
>         return t;
>       }
> -  else if (CONSTANT_CLASS_P (t))
> +  else if (CONSTANT_CLASS_OR_WRAPPER_P (t))
>       /* No caching or evaluation needed.  */
> -    return t;
> +    return tree_strip_any_location_wrapper (t);
>   
>     if (manifestly_const_eval != mce_unknown)
>       return cxx_eval_outermost_constant_expr (t, true, true,
> @@ -8631,6 +8631,10 @@ fold_non_dependent_expr_template (tree t, tsubst_flags_t complain,
>   	  return t;
>   	}
>   
> +      if (CONSTANT_CLASS_OR_WRAPPER_P (t))
> +	/* No evaluation needed.  */
> +	return tree_strip_any_location_wrapper (t);
> +
>         if (cp_unevaluated_operand && !manifestly_const_eval)
>   	return t;
>   
> diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
> index 705a5b3bdb6..574d2e2586c 100644
> --- a/gcc/cp/init.cc
> +++ b/gcc/cp/init.cc
> @@ -3653,16 +3653,14 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
>                   error ("parenthesized initializer in array new");
>   	      return error_mark_node;
>               }
> -	  init_expr
> -	    = build_vec_init (data_addr,
> -			      cp_build_binary_op (input_location,
> -						  MINUS_EXPR, outer_nelts,
> -						  integer_one_node,
> -						  complain),
> -			      vecinit,
> -			      explicit_value_init_p,
> -			      /*from_array=*/0,
> -                              complain);
> +	  tree maxindex = fold_build2_loc (input_location, MINUS_EXPR,
> +					   TREE_TYPE (outer_nelts),
> +					   outer_nelts,
> +					   build_one_cst (TREE_TYPE
> +							  (outer_nelts)));
> +	  init_expr = build_vec_init (data_addr, maxindex, vecinit,
> +				      explicit_value_init_p, /*from_array=*/0,
> +				      complain);
>   	}
>         else
>   	{
> diff --git a/gcc/testsuite/g++.dg/cpp0x/new6.C b/gcc/testsuite/g++.dg/cpp0x/new6.C
> new file mode 100644
> index 00000000000..17a669b42d0
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/new6.C
> @@ -0,0 +1,9 @@
> +// PR c++/108218
> +// { dg-do compile { target c++11 } }
> +
> +template<class T>
> +void f() {
> +  decltype(new int[-1]) p; // { dg-error "negative" }
> +}
> +
> +decltype(new int[-1]) q; // { dg-error "negative" }
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-new1.C b/gcc/testsuite/g++.dg/cpp2a/concepts-new1.C
> new file mode 100644
> index 00000000000..62007205108
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-new1.C
> @@ -0,0 +1,13 @@
> +// PR c++/108219
> +// { dg-do compile { target c++20 } }
> +
> +template<class T>
> +concept C = requires { new T[1]{{ 42 }}; };
> +
> +template<class T>
> +concept D = requires { new T[2][1]{{{ 42 }}, {{ 42 }}}; };
> +
> +struct A { A(int); };
> +
> +static_assert(C<A>);
> +static_assert(D<A>);