public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: unevaluated array new-expr size constantness [PR108219]
@ 2023-02-22 19:45 Patrick Palka
  2023-02-28  0:00 ` Jason Merrill
  0 siblings, 1 reply; 7+ messages in thread
From: Patrick Palka @ 2023-02-22 19:45 UTC (permalink / raw)
  To: gcc-patches; +Cc: jason, Patrick Palka

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).

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.

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>);
-- 
2.39.2.501.gd9d677b2d8


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] c++: unevaluated array new-expr size constantness [PR108219]
  2023-02-22 19:45 [PATCH] c++: unevaluated array new-expr size constantness [PR108219] Patrick Palka
@ 2023-02-28  0:00 ` Jason Merrill
  2023-03-01 15:32   ` Patrick Palka
  0 siblings, 1 reply; 7+ messages in thread
From: Jason Merrill @ 2023-02-28  0:00 UTC (permalink / raw)
  To: Patrick Palka, gcc-patches

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>);


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] c++: unevaluated array new-expr size constantness [PR108219]
  2023-02-28  0:00 ` Jason Merrill
@ 2023-03-01 15:32   ` Patrick Palka
  2023-03-01 16:38     ` Jason Merrill
  0 siblings, 1 reply; 7+ messages in thread
From: Patrick Palka @ 2023-03-01 15:32 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Patrick Palka, gcc-patches

On Mon, 27 Feb 2023, Jason Merrill wrote:

> 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.

Ah, makes sense.  Fixed in the below patch.

> 
> > 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?

That seems like a good compromise between proper constant evaluation
and not constant evaluating at all, though I wonder how 'fold' behaves
w.r.t. to undefined behavior such as division by zero and signed overflow?
IIUC proper constant evaluation treats UB as non-constant, so it might
be potentially problematic if 'fold' instea dtakes advantage of UB.  Or
maybe since we're in an unevaluated context it doesn't matter?

-- >8 --

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

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
these sizes are expressed as NON_LVALUE_EXPR location wrappers around
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).

This patch fixes this by making maybe_constant_value and
fold_non_dependent_expr at least try folding simple unevaluated operands
via fold(), which will evaluate simple arithmetic, look through location
wrappers, perform integral conversions, etc.

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): Move up early exit
	test for unevaluated operands.  Call fold on unevaluated
	operands.
	(fold_non_dependent_expr_template): Add early exit test for
	CONSTANT_CLASS_P nodes.  Call fold on unevaluated operands.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/new6.C: New test.
	* g++.dg/cpp2a/concepts-new1.C: New test.
---
 gcc/cp/constexpr.cc                        | 17 ++++++++++++-----
 gcc/testsuite/g++.dg/cpp0x/new6.C          | 13 +++++++++++++
 gcc/testsuite/g++.dg/cpp2a/concepts-new1.C | 13 +++++++++++++
 3 files changed, 38 insertions(+), 5 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 b4d3e95bbd5..d71abe6beed 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -8523,6 +8523,11 @@ maybe_constant_value (tree t, tree decl /* = NULL_TREE */,
     /* No caching or evaluation needed.  */
     return t;
 
+  /* Don't constant evaluate an unevaluated non-manifestly-constant operand,
+     but at least try folding simple expressions.  */
+  if (cp_unevaluated_operand && manifestly_const_eval != mce_true)
+    return fold (t);
+
   if (manifestly_const_eval != mce_unknown)
     return cxx_eval_outermost_constant_expr (t, true, true,
 					     manifestly_const_eval, false, decl);
@@ -8544,10 +8549,6 @@ maybe_constant_value (tree t, tree decl /* = NULL_TREE */,
       return r;
     }
 
-  /* Don't evaluate an unevaluated operand.  */
-  if (cp_unevaluated_operand)
-    return t;
-
   uid_sensitive_constexpr_evaluation_checker c;
   r = cxx_eval_outermost_constant_expr (t, true, true,
 					manifestly_const_eval, false, decl);
@@ -8612,8 +8613,14 @@ fold_non_dependent_expr_template (tree t, tsubst_flags_t complain,
 	  return t;
 	}
 
+      if (CONSTANT_CLASS_P (t))
+	/* No evaluation needed.  */
+	return t;
+
+      /* Don't constant evaluate an unevaluated non-manifestly-constant operand,
+	 but at least try folding simple expressions.  */
       if (cp_unevaluated_operand && !manifestly_const_eval)
-	return t;
+	return fold (t);
 
       tree r = cxx_eval_outermost_constant_expr (t, true, true,
 						 mce_value (manifestly_const_eval),
diff --git a/gcc/testsuite/g++.dg/cpp0x/new6.C b/gcc/testsuite/g++.dg/cpp0x/new6.C
new file mode 100644
index 00000000000..d8f11441423
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/new6.C
@@ -0,0 +1,13 @@
+// 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[0-1]) q; // { dg-error "negative" }
+  decltype(new int[1*-1]) r; // { dg-error "negative" }
+}
+
+decltype(new int[-1]) p; // { dg-error "negative" }
+decltype(new int[0-1]) q; // { dg-error "negative" }
+decltype(new int[1*-1]) r; // { 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>);
-- 
2.40.0.rc0.57.g454dfcbddf


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] c++: unevaluated array new-expr size constantness [PR108219]
  2023-03-01 15:32   ` Patrick Palka
@ 2023-03-01 16:38     ` Jason Merrill
  2023-03-01 17:20       ` Patrick Palka
  0 siblings, 1 reply; 7+ messages in thread
From: Jason Merrill @ 2023-03-01 16:38 UTC (permalink / raw)
  To: Patrick Palka; +Cc: gcc-patches

On 3/1/23 10:32, Patrick Palka wrote:
> On Mon, 27 Feb 2023, Jason Merrill wrote:
> 
>> 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.
> 
> Ah, makes sense.  Fixed in the below patch.
> 
>>
>>> 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?
> 
> That seems like a good compromise between proper constant evaluation
> and not constant evaluating at all, though I wonder how 'fold' behaves
> w.r.t. to undefined behavior such as division by zero and signed overflow?

'fold' doesn't fold division by zero, but I think we should only return 
the result of 'fold' at this point if it is in fact constant, not if 
it's a non-constant simplification.

> IIUC proper constant evaluation treats UB as non-constant, so it might
> be potentially problematic if 'fold' instea dtakes advantage of UB.  Or
> maybe since we're in an unevaluated context it doesn't matter?
> 
> -- >8 --
> 
> Subject: [PATCH] c++: unevaluated array new-expr size constantness [PR108219]
> 
> 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
> these sizes are expressed as NON_LVALUE_EXPR location wrappers around
> 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).
> 
> This patch fixes this by making maybe_constant_value and
> fold_non_dependent_expr at least try folding simple unevaluated operands
> via fold(), which will evaluate simple arithmetic, look through location
> wrappers, perform integral conversions, etc.
> 
> 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): Move up early exit
> 	test for unevaluated operands.  Call fold on unevaluated
> 	operands.
> 	(fold_non_dependent_expr_template): Add early exit test for
> 	CONSTANT_CLASS_P nodes.  Call fold on unevaluated operands.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/new6.C: New test.
> 	* g++.dg/cpp2a/concepts-new1.C: New test.
> ---
>   gcc/cp/constexpr.cc                        | 17 ++++++++++++-----
>   gcc/testsuite/g++.dg/cpp0x/new6.C          | 13 +++++++++++++
>   gcc/testsuite/g++.dg/cpp2a/concepts-new1.C | 13 +++++++++++++
>   3 files changed, 38 insertions(+), 5 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 b4d3e95bbd5..d71abe6beed 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -8523,6 +8523,11 @@ maybe_constant_value (tree t, tree decl /* = NULL_TREE */,
>       /* No caching or evaluation needed.  */
>       return t;
>   
> +  /* Don't constant evaluate an unevaluated non-manifestly-constant operand,
> +     but at least try folding simple expressions.  */
> +  if (cp_unevaluated_operand && manifestly_const_eval != mce_true)
> +    return fold (t);
> +
>     if (manifestly_const_eval != mce_unknown)
>       return cxx_eval_outermost_constant_expr (t, true, true,
>   					     manifestly_const_eval, false, decl);
> @@ -8544,10 +8549,6 @@ maybe_constant_value (tree t, tree decl /* = NULL_TREE */,
>         return r;
>       }
>   
> -  /* Don't evaluate an unevaluated operand.  */
> -  if (cp_unevaluated_operand)
> -    return t;
> -
>     uid_sensitive_constexpr_evaluation_checker c;
>     r = cxx_eval_outermost_constant_expr (t, true, true,
>   					manifestly_const_eval, false, decl);
> @@ -8612,8 +8613,14 @@ fold_non_dependent_expr_template (tree t, tsubst_flags_t complain,
>   	  return t;
>   	}
>   
> +      if (CONSTANT_CLASS_P (t))
> +	/* No evaluation needed.  */
> +	return t;
> +
> +      /* Don't constant evaluate an unevaluated non-manifestly-constant operand,
> +	 but at least try folding simple expressions.  */
>         if (cp_unevaluated_operand && !manifestly_const_eval)
> -	return t;
> +	return fold (t);
>   
>         tree r = cxx_eval_outermost_constant_expr (t, true, true,
>   						 mce_value (manifestly_const_eval),
> diff --git a/gcc/testsuite/g++.dg/cpp0x/new6.C b/gcc/testsuite/g++.dg/cpp0x/new6.C
> new file mode 100644
> index 00000000000..d8f11441423
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/new6.C
> @@ -0,0 +1,13 @@
> +// 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[0-1]) q; // { dg-error "negative" }
> +  decltype(new int[1*-1]) r; // { dg-error "negative" }
> +}
> +
> +decltype(new int[-1]) p; // { dg-error "negative" }
> +decltype(new int[0-1]) q; // { dg-error "negative" }
> +decltype(new int[1*-1]) r; // { 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>);


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] c++: unevaluated array new-expr size constantness [PR108219]
  2023-03-01 16:38     ` Jason Merrill
@ 2023-03-01 17:20       ` Patrick Palka
  2023-03-01 17:54         ` Jason Merrill
  0 siblings, 1 reply; 7+ messages in thread
From: Patrick Palka @ 2023-03-01 17:20 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Patrick Palka, gcc-patches

On Wed, 1 Mar 2023, Jason Merrill wrote:

> On 3/1/23 10:32, Patrick Palka wrote:
> > On Mon, 27 Feb 2023, Jason Merrill wrote:
> > 
> > > 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.
> > 
> > Ah, makes sense.  Fixed in the below patch.
> > 
> > > 
> > > > 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?
> > 
> > That seems like a good compromise between proper constant evaluation
> > and not constant evaluating at all, though I wonder how 'fold' behaves
> > w.r.t. to undefined behavior such as division by zero and signed overflow?
> 
> 'fold' doesn't fold division by zero, but I think we should only return the
> result of 'fold' at this point if it is in fact constant, not if it's a
> non-constant simplification.

Sounds good, I wasn't sure if 'fold' could return a non-constant
simplification.  I suppose we want to be pretty conservative with the
constantness test, so I went with CONSTANT_CLASS_P && !TREE_OVERFLOW.

Like so?  Smoke tested so far, bootstrap and regtest on
x86_64-pc-linu-xgnu in progress.

-- >8 --

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

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
these sizes are expressed as NON_LVALUE_EXPR location wrappers around
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).

This patch fixes this by making maybe_constant_value and
fold_non_dependent_expr at least try folding simple unevaluated operands
via fold(), which will evaluate simple arithmetic, look through location
wrappers, perform integral conversions, etc.

Co-authored-by: Jason Merrill <jason@redhat.com>

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): Move up early exit
	test for unevaluated operands.  Try reducing an unevaluated
	operand to a constant via fold.
	(fold_non_dependent_expr_template): Add early exit test for
	CONSTANT_CLASS_P nodes.  Try reducing an unevaluated operand
	to a constant via fold.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/new6.C: New test.
	* g++.dg/cpp2a/concepts-new1.C: New test.
---
 gcc/cp/constexpr.cc                        | 23 +++++++++++++++++-----
 gcc/testsuite/g++.dg/cpp0x/new6.C          | 13 ++++++++++++
 gcc/testsuite/g++.dg/cpp2a/concepts-new1.C | 13 ++++++++++++
 3 files changed, 44 insertions(+), 5 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 b4d3e95bbd5..324968050ba 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -8523,6 +8523,14 @@ maybe_constant_value (tree t, tree decl /* = NULL_TREE */,
     /* No caching or evaluation needed.  */
     return t;
 
+  /* Don't constant evaluate an unevaluated non-manifestly-constant operand,
+     but at least try folding simple expressions to a constant.  */
+  if (cp_unevaluated_operand && manifestly_const_eval != mce_true)
+    {
+      tree r = fold (t);
+      return CONSTANT_CLASS_P (r) && !TREE_OVERFLOW (r) ? r : t;
+    }
+
   if (manifestly_const_eval != mce_unknown)
     return cxx_eval_outermost_constant_expr (t, true, true,
 					     manifestly_const_eval, false, decl);
@@ -8544,10 +8552,6 @@ maybe_constant_value (tree t, tree decl /* = NULL_TREE */,
       return r;
     }
 
-  /* Don't evaluate an unevaluated operand.  */
-  if (cp_unevaluated_operand)
-    return t;
-
   uid_sensitive_constexpr_evaluation_checker c;
   r = cxx_eval_outermost_constant_expr (t, true, true,
 					manifestly_const_eval, false, decl);
@@ -8612,8 +8616,17 @@ fold_non_dependent_expr_template (tree t, tsubst_flags_t complain,
 	  return t;
 	}
 
+      if (CONSTANT_CLASS_P (t))
+	/* No evaluation needed.  */
+	return t;
+
+      /* Don't constant evaluate an unevaluated non-manifestly-constant operand,
+	 but at least try folding simple expressions to a constant.  */
       if (cp_unevaluated_operand && !manifestly_const_eval)
-	return t;
+	{
+	  tree r = fold (t);
+	  return CONSTANT_CLASS_P (r) && !TREE_OVERFLOW (r) ? r : t;
+	}
 
       tree r = cxx_eval_outermost_constant_expr (t, true, true,
 						 mce_value (manifestly_const_eval),
diff --git a/gcc/testsuite/g++.dg/cpp0x/new6.C b/gcc/testsuite/g++.dg/cpp0x/new6.C
new file mode 100644
index 00000000000..d8f11441423
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/new6.C
@@ -0,0 +1,13 @@
+// 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[0-1]) q; // { dg-error "negative" }
+  decltype(new int[1*-1]) r; // { dg-error "negative" }
+}
+
+decltype(new int[-1]) p; // { dg-error "negative" }
+decltype(new int[0-1]) q; // { dg-error "negative" }
+decltype(new int[1*-1]) r; // { 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>);
-- 
2.40.0.rc0.57.g454dfcbddf


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] c++: unevaluated array new-expr size constantness [PR108219]
  2023-03-01 17:20       ` Patrick Palka
@ 2023-03-01 17:54         ` Jason Merrill
  2023-03-01 19:25           ` Patrick Palka
  0 siblings, 1 reply; 7+ messages in thread
From: Jason Merrill @ 2023-03-01 17:54 UTC (permalink / raw)
  To: Patrick Palka; +Cc: gcc-patches

On 3/1/23 12:20, Patrick Palka wrote:
> On Wed, 1 Mar 2023, Jason Merrill wrote:
> 
>> On 3/1/23 10:32, Patrick Palka wrote:
>>> On Mon, 27 Feb 2023, Jason Merrill wrote:
>>>
>>>> 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.
>>>
>>> Ah, makes sense.  Fixed in the below patch.
>>>
>>>>
>>>>> 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?
>>>
>>> That seems like a good compromise between proper constant evaluation
>>> and not constant evaluating at all, though I wonder how 'fold' behaves
>>> w.r.t. to undefined behavior such as division by zero and signed overflow?
>>
>> 'fold' doesn't fold division by zero, but I think we should only return the
>> result of 'fold' at this point if it is in fact constant, not if it's a
>> non-constant simplification.
> 
> Sounds good, I wasn't sure if 'fold' could return a non-constant
> simplification.

Yep, it also folds e.g. x*1 to x.

> I suppose we want to be pretty conservative with the
> constantness test, so I went with CONSTANT_CLASS_P && !TREE_OVERFLOW.

Makes sense.

> Like so?  Smoke tested so far, bootstrap and regtest on
> x86_64-pc-linu-xgnu in progress.
> 
> -- >8 --
> 
> Subject: [PATCH] c++: unevaluated array new-expr size constantness [PR108219]
> 
> 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
> these sizes are expressed as NON_LVALUE_EXPR location wrappers around
> 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).
> 
> This patch fixes this by making maybe_constant_value and
> fold_non_dependent_expr at least try folding simple unevaluated operands
> via fold(), which will evaluate simple arithmetic, look through location
> wrappers, perform integral conversions, etc.
> 
> Co-authored-by: Jason Merrill <jason@redhat.com>
> 
> 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): Move up early exit
> 	test for unevaluated operands.  Try reducing an unevaluated
> 	operand to a constant via fold.
> 	(fold_non_dependent_expr_template): Add early exit test for
> 	CONSTANT_CLASS_P nodes.  Try reducing an unevaluated operand
> 	to a constant via fold.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/new6.C: New test.
> 	* g++.dg/cpp2a/concepts-new1.C: New test.
> ---
>   gcc/cp/constexpr.cc                        | 23 +++++++++++++++++-----
>   gcc/testsuite/g++.dg/cpp0x/new6.C          | 13 ++++++++++++
>   gcc/testsuite/g++.dg/cpp2a/concepts-new1.C | 13 ++++++++++++
>   3 files changed, 44 insertions(+), 5 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 b4d3e95bbd5..324968050ba 100644
> --- a/gcc/cp/constexpr.cc
> +++ b/gcc/cp/constexpr.cc
> @@ -8523,6 +8523,14 @@ maybe_constant_value (tree t, tree decl /* = NULL_TREE */,
>       /* No caching or evaluation needed.  */
>       return t;
>   
> +  /* Don't constant evaluate an unevaluated non-manifestly-constant operand,
> +     but at least try folding simple expressions to a constant.  */
> +  if (cp_unevaluated_operand && manifestly_const_eval != mce_true)
> +    {
> +      tree r = fold (t);
> +      return CONSTANT_CLASS_P (r) && !TREE_OVERFLOW (r) ? r : t;
> +    }
> +
>     if (manifestly_const_eval != mce_unknown)
>       return cxx_eval_outermost_constant_expr (t, true, true,
>   					     manifestly_const_eval, false, decl);
> @@ -8544,10 +8552,6 @@ maybe_constant_value (tree t, tree decl /* = NULL_TREE */,
>         return r;
>       }
>   
> -  /* Don't evaluate an unevaluated operand.  */
> -  if (cp_unevaluated_operand)
> -    return t;
> -
>     uid_sensitive_constexpr_evaluation_checker c;
>     r = cxx_eval_outermost_constant_expr (t, true, true,
>   					manifestly_const_eval, false, decl);
> @@ -8612,8 +8616,17 @@ fold_non_dependent_expr_template (tree t, tsubst_flags_t complain,
>   	  return t;
>   	}
>   
> +      if (CONSTANT_CLASS_P (t))
> +	/* No evaluation needed.  */
> +	return t;
> +      /* Don't constant evaluate an unevaluated non-manifestly-constant operand,
> +	 but at least try folding simple expressions to a constant.  */
>         if (cp_unevaluated_operand && !manifestly_const_eval)
> -	return t;
> +	{
> +	  tree r = fold (t);
> +	  return CONSTANT_CLASS_P (r) && !TREE_OVERFLOW (r) ? r : t;

These two lines could be factored into a fold_to_constant (inline?) 
function.  OK with that change.

> +	}
>   
>         tree r = cxx_eval_outermost_constant_expr (t, true, true,
>   						 mce_value (manifestly_const_eval),
> diff --git a/gcc/testsuite/g++.dg/cpp0x/new6.C b/gcc/testsuite/g++.dg/cpp0x/new6.C
> new file mode 100644
> index 00000000000..d8f11441423
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/new6.C
> @@ -0,0 +1,13 @@
> +// 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[0-1]) q; // { dg-error "negative" }
> +  decltype(new int[1*-1]) r; // { dg-error "negative" }
> +}
> +
> +decltype(new int[-1]) p; // { dg-error "negative" }
> +decltype(new int[0-1]) q; // { dg-error "negative" }
> +decltype(new int[1*-1]) r; // { 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>);


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] c++: unevaluated array new-expr size constantness [PR108219]
  2023-03-01 17:54         ` Jason Merrill
@ 2023-03-01 19:25           ` Patrick Palka
  0 siblings, 0 replies; 7+ messages in thread
From: Patrick Palka @ 2023-03-01 19:25 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Patrick Palka, gcc-patches

On Wed, 1 Mar 2023, Jason Merrill wrote:

> On 3/1/23 12:20, Patrick Palka wrote:
> > On Wed, 1 Mar 2023, Jason Merrill wrote:
> > 
> > > On 3/1/23 10:32, Patrick Palka wrote:
> > > > On Mon, 27 Feb 2023, Jason Merrill wrote:
> > > > 
> > > > > 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.
> > > > 
> > > > Ah, makes sense.  Fixed in the below patch.
> > > > 
> > > > > 
> > > > > > 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?
> > > > 
> > > > That seems like a good compromise between proper constant evaluation
> > > > and not constant evaluating at all, though I wonder how 'fold' behaves
> > > > w.r.t. to undefined behavior such as division by zero and signed
> > > > overflow?
> > > 
> > > 'fold' doesn't fold division by zero, but I think we should only return
> > > the
> > > result of 'fold' at this point if it is in fact constant, not if it's a
> > > non-constant simplification.
> > 
> > Sounds good, I wasn't sure if 'fold' could return a non-constant
> > simplification.
> 
> Yep, it also folds e.g. x*1 to x.
> 
> > I suppose we want to be pretty conservative with the
> > constantness test, so I went with CONSTANT_CLASS_P && !TREE_OVERFLOW.
> 
> Makes sense.
> 
> > Like so?  Smoke tested so far, bootstrap and regtest on
> > x86_64-pc-linu-xgnu in progress.
> > 
> > -- >8 --
> > 
> > Subject: [PATCH] c++: unevaluated array new-expr size constantness
> > [PR108219]
> > 
> > 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
> > these sizes are expressed as NON_LVALUE_EXPR location wrappers around
> > 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).
> > 
> > This patch fixes this by making maybe_constant_value and
> > fold_non_dependent_expr at least try folding simple unevaluated operands
> > via fold(), which will evaluate simple arithmetic, look through location
> > wrappers, perform integral conversions, etc.
> > 
> > Co-authored-by: Jason Merrill <jason@redhat.com>
> > 
> > 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): Move up early exit
> > 	test for unevaluated operands.  Try reducing an unevaluated
> > 	operand to a constant via fold.
> > 	(fold_non_dependent_expr_template): Add early exit test for
> > 	CONSTANT_CLASS_P nodes.  Try reducing an unevaluated operand
> > 	to a constant via fold.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/cpp0x/new6.C: New test.
> > 	* g++.dg/cpp2a/concepts-new1.C: New test.
> > ---
> >   gcc/cp/constexpr.cc                        | 23 +++++++++++++++++-----
> >   gcc/testsuite/g++.dg/cpp0x/new6.C          | 13 ++++++++++++
> >   gcc/testsuite/g++.dg/cpp2a/concepts-new1.C | 13 ++++++++++++
> >   3 files changed, 44 insertions(+), 5 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 b4d3e95bbd5..324968050ba 100644
> > --- a/gcc/cp/constexpr.cc
> > +++ b/gcc/cp/constexpr.cc
> > @@ -8523,6 +8523,14 @@ maybe_constant_value (tree t, tree decl /* =
> > NULL_TREE */,
> >       /* No caching or evaluation needed.  */
> >       return t;
> >   +  /* Don't constant evaluate an unevaluated non-manifestly-constant
> > operand,
> > +     but at least try folding simple expressions to a constant.  */
> > +  if (cp_unevaluated_operand && manifestly_const_eval != mce_true)
> > +    {
> > +      tree r = fold (t);
> > +      return CONSTANT_CLASS_P (r) && !TREE_OVERFLOW (r) ? r : t;
> > +    }
> > +
> >     if (manifestly_const_eval != mce_unknown)
> >       return cxx_eval_outermost_constant_expr (t, true, true,
> >   					     manifestly_const_eval, false,
> > decl);
> > @@ -8544,10 +8552,6 @@ maybe_constant_value (tree t, tree decl /* =
> > NULL_TREE */,
> >         return r;
> >       }
> >   -  /* Don't evaluate an unevaluated operand.  */
> > -  if (cp_unevaluated_operand)
> > -    return t;
> > -
> >     uid_sensitive_constexpr_evaluation_checker c;
> >     r = cxx_eval_outermost_constant_expr (t, true, true,
> >   					manifestly_const_eval, false, decl);
> > @@ -8612,8 +8616,17 @@ fold_non_dependent_expr_template (tree t,
> > tsubst_flags_t complain,
> >   	  return t;
> >   	}
> >   +      if (CONSTANT_CLASS_P (t))
> > +	/* No evaluation needed.  */
> > +	return t;
> > +      /* Don't constant evaluate an unevaluated non-manifestly-constant
> > operand,
> > +	 but at least try folding simple expressions to a constant.  */
> >         if (cp_unevaluated_operand && !manifestly_const_eval)
> > -	return t;
> > +	{
> > +	  tree r = fold (t);
> > +	  return CONSTANT_CLASS_P (r) && !TREE_OVERFLOW (r) ? r : t;
> 
> These two lines could be factored into a fold_to_constant (inline?) function.
> OK with that change.

Thanks a lot, I went with a non-inline function to avoid introducing a
direct dependency on fold-const.h from cp-tree.h (though somehow
defining it inline worked too without needing to directly #include
fold-const.h from cp-tree.h).

Here's what I pushed:

-- >8 --

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

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 expressed
as NON_LVALUE_EXPR location wrappers around 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 MINUS_EXPR
'maxindex' passed from build_new_1 when in an unevaluated context (since
it tries reducing it via maybe_constant_value called with mce_unknown).

This patch fixes these issues by making maybe_constant_value (and
fold_non_dependent_expr) try folding an unevaluated non-manifestly-constant
operand via fold(), as long as it simplifies to a simple constant, rather
than doing no simplification at all.  This covers e.g. simple arithmetic
and casts including stripping of location wrappers around INTEGER_CST.

In passing, this patch also fixes maybe_constant_value to avoid constant
evaluating an unevaluated operand when called with mce_false, by adjusting
the early exit test appropriately.

Co-authored-by: Jason Merrill <jason@redhat.com>

	PR c++/108219
	PR c++/108218

gcc/cp/ChangeLog:

	* constexpr.cc (fold_to_constant): Define.
	(maybe_constant_value): Move up early exit test for unevaluated
	operands.  Try reducing an unevaluated operand to a constant via
	fold_to_constant.
	(fold_non_dependent_expr_template): Add early exit test for
	CONSTANT_CLASS_P nodes.  Try reducing an unevaluated operand
	to a constant via fold_to_constant.
	* cp-tree.h (fold_to_constant): Declare.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/new6.C: New test.
	* g++.dg/cpp2a/concepts-new1.C: New test.
---
 gcc/cp/constexpr.cc                        | 29 ++++++++++++++++++----
 gcc/cp/cp-tree.h                           |  1 +
 gcc/testsuite/g++.dg/cpp0x/new6.C          | 13 ++++++++++
 gcc/testsuite/g++.dg/cpp2a/concepts-new1.C | 13 ++++++++++
 4 files changed, 51 insertions(+), 5 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 89df7d7600c..bcae1cbd973 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -8498,6 +8498,19 @@ fold_simple (tree t)
   return t;
 }
 
+/* Try folding the expression T to a simple constant.
+   Returns that constant, otherwise returns T.  */
+
+tree
+fold_to_constant (tree t)
+{
+  tree r = fold (t);
+  if (CONSTANT_CLASS_P (r) && !TREE_OVERFLOW (r))
+    return r;
+  else
+    return t;
+}
+
 /* If T is a constant expression, returns its reduced value.
    Otherwise, if T does not have TREE_CONSTANT set, returns T.
    Otherwise, returns a version of T without TREE_CONSTANT.
@@ -8523,6 +8536,11 @@ maybe_constant_value (tree t, tree decl /* = NULL_TREE */,
     /* No caching or evaluation needed.  */
     return t;
 
+  /* Don't constant evaluate an unevaluated non-manifestly-constant operand,
+     but at least try folding it to a simple constant.  */
+  if (cp_unevaluated_operand && manifestly_const_eval != mce_true)
+    return fold_to_constant (t);
+
   if (manifestly_const_eval != mce_unknown)
     return cxx_eval_outermost_constant_expr (t, true, true,
 					     manifestly_const_eval, false, decl);
@@ -8544,10 +8562,6 @@ maybe_constant_value (tree t, tree decl /* = NULL_TREE */,
       return r;
     }
 
-  /* Don't evaluate an unevaluated operand.  */
-  if (cp_unevaluated_operand)
-    return t;
-
   uid_sensitive_constexpr_evaluation_checker c;
   r = cxx_eval_outermost_constant_expr (t, true, true,
 					manifestly_const_eval, false, decl);
@@ -8611,9 +8625,14 @@ fold_non_dependent_expr_template (tree t, tsubst_flags_t complain,
 	    }
 	  return t;
 	}
+      else if (CONSTANT_CLASS_P (t))
+	/* No evaluation needed.  */
+	return t;
 
+      /* Don't constant evaluate an unevaluated non-manifestly-constant operand,
+	 but at least try folding it to a simple constant.  */
       if (cp_unevaluated_operand && !manifestly_const_eval)
-	return t;
+	return fold_to_constant (t);
 
       tree r = cxx_eval_outermost_constant_expr (t, true, true,
 						 mce_value (manifestly_const_eval),
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 5595335bbf7..83633ddc7f2 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -8516,6 +8516,7 @@ extern tree fold_non_dependent_init		(tree,
 						 tsubst_flags_t = tf_warning_or_error,
 						 bool = false, tree = NULL_TREE);
 extern tree fold_simple				(tree);
+extern tree fold_to_constant			(tree);
 extern bool reduced_constant_expression_p       (tree);
 extern bool is_instantiation_of_constexpr       (tree);
 extern bool var_in_constexpr_fn                 (tree);
diff --git a/gcc/testsuite/g++.dg/cpp0x/new6.C b/gcc/testsuite/g++.dg/cpp0x/new6.C
new file mode 100644
index 00000000000..d8f11441423
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/new6.C
@@ -0,0 +1,13 @@
+// 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[0-1]) q; // { dg-error "negative" }
+  decltype(new int[1*-1]) r; // { dg-error "negative" }
+}
+
+decltype(new int[-1]) p; // { dg-error "negative" }
+decltype(new int[0-1]) q; // { dg-error "negative" }
+decltype(new int[1*-1]) r; // { 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>);
-- 
2.40.0.rc0.57.g454dfcbddf


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-03-01 19:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-22 19:45 [PATCH] c++: unevaluated array new-expr size constantness [PR108219] Patrick Palka
2023-02-28  0:00 ` Jason Merrill
2023-03-01 15:32   ` Patrick Palka
2023-03-01 16:38     ` Jason Merrill
2023-03-01 17:20       ` Patrick Palka
2023-03-01 17:54         ` Jason Merrill
2023-03-01 19:25           ` Patrick Palka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).