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