Re: [PATCH] c++: build_new_1 and non-dep array size [PR111929]

[Jason Merrill <jason@redhat.com>]

On 10/24/23 13:03, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> like the right approach?
> 
> -- >8 --
> 
> This PR is another instance of NON_DEPENDENT_EXPR having acted as an
> "analysis barrier" for middle-end routines, and now that it's gone we
> may end up passing weird templated trees (that have a generic tree code)
> to the middle-end which leads to an ICE.  In the testcase below the
> non-dependent array size 'var + 42' is expressed as an ordinary
> PLUS_EXPR, but whose operand types have different precisions -- long and
> int respectively -- naturally because templated trees encode only the
> syntactic form of an expression devoid of e.g. implicit conversions
> (typically).  This type incoherency triggers a wide_int assert during
> the call to size_binop in build_new_1 which requires the operand types
> have the same precision.
> 
> This patch fixes this by replacing our incremental folding of 'size'
> within build_new_1 with a single call to cp_fully_fold (which is a no-op
> in template context) once 'size' is fully built.

This is OK, but we could probably also entirely skip a lot of the 
calculation in a template, since we don't care about any values.  Can we 
skip the entire if (array_p) block?

> 	PR c++/111929
> 
> gcc/cp/ChangeLog:
> 
> 	* init.cc (build_new_1): Use convert, build2, build3 instead of
> 	fold_convert, size_binop and fold_build3 when building 'size'.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/template/non-dependent28.C: New test.
> ---
>   gcc/cp/init.cc                                  | 9 +++++----
>   gcc/testsuite/g++.dg/template/non-dependent28.C | 6 ++++++
>   2 files changed, 11 insertions(+), 4 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/template/non-dependent28.C
> 
> diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
> index d48bb16c7c5..56c1b5e9f5e 100644
> --- a/gcc/cp/init.cc
> +++ b/gcc/cp/init.cc
> @@ -3261,7 +3261,7 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
>         max_outer_nelts = wi::udiv_trunc (max_size, inner_size);
>         max_outer_nelts_tree = wide_int_to_tree (sizetype, max_outer_nelts);
>   
> -      size = size_binop (MULT_EXPR, size, fold_convert (sizetype, nelts));
> +      size = build2 (MULT_EXPR, sizetype, size, convert (sizetype, nelts));
>   
>         if (TREE_CODE (cst_outer_nelts) == INTEGER_CST)
>   	{
> @@ -3344,7 +3344,7 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
>         /* Use a class-specific operator new.  */
>         /* If a cookie is required, add some extra space.  */
>         if (array_p && TYPE_VEC_NEW_USES_COOKIE (elt_type))
> -	size = size_binop (PLUS_EXPR, size, cookie_size);
> +	size = build2 (PLUS_EXPR, sizetype, size, cookie_size);
>         else
>   	{
>   	  cookie_size = NULL_TREE;
> @@ -3358,8 +3358,8 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
>         if (cxx_dialect >= cxx11 && flag_exceptions)
>   	errval = throw_bad_array_new_length ();
>         if (outer_nelts_check != NULL_TREE)
> -	size = fold_build3 (COND_EXPR, sizetype, outer_nelts_check,
> -			    size, errval);
> +	size = build3 (COND_EXPR, sizetype, outer_nelts_check, size, errval);
> +      size = cp_fully_fold (size);
>         /* Create the argument list.  */
>         vec_safe_insert (*placement, 0, size);
>         /* Do name-lookup to find the appropriate operator.  */
> @@ -3418,6 +3418,7 @@ build_new_1 (vec<tree, va_gc> **placement, tree type, tree nelts,
>         /* If size is zero e.g. due to type having zero size, try to
>   	 preserve outer_nelts for constant expression evaluation
>   	 purposes.  */
> +      size = cp_fully_fold (size);
>         if (integer_zerop (size) && outer_nelts)
>   	size = build2 (MULT_EXPR, TREE_TYPE (size), size, outer_nelts);
>   
> diff --git a/gcc/testsuite/g++.dg/template/non-dependent28.C b/gcc/testsuite/g++.dg/template/non-dependent28.C
> new file mode 100644
> index 00000000000..3e45154f61d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/non-dependent28.C
> @@ -0,0 +1,6 @@
> +// PR c++/111929
> +
> +template<class>
> +void f(long var) {
> +  new int[var + 42];
> +}