On 4/12/22 15:48, Patrick Palka wrote:
On Wed, Feb 16, 2022 at 2:47 PM Patrick Palka <ppa...@redhat.com> wrote:
On Tue, 15 Feb 2022, Jason Merrill wrote:
On 2/15/22 17:00, Patrick Palka wrote:
On Tue, 15 Feb 2022, Jason Merrill wrote:
On 2/15/22 15:13, Patrick Palka wrote:
On Tue, 15 Feb 2022, Patrick Palka wrote:
Here we're crashing from potential_constant_expression because it
tries
to perform trial evaluation of the first operand '(bool)__r' of the
conjunction (which is overall wrapped in a NON_DEPENDENT_EXPR), but
cxx_eval_constant_expression ICEs on unhandled trees (of which
CAST_EXPR
is one).
Since cxx_eval_constant_expression always treats NON_DEPENDENT_EXPR
as non-constant, and since NON_DEPENDENT_EXPR is also opaque to
instantiate_non_dependent_expr, it seems futile to have p_c_e_1 ever
return true for NON_DEPENDENT_EXPR, so let's just instead return false
and avoid recursing.
Well, in a template we use pce1 to decide whether to complain about
something
that needs to be constant but can't be. We aren't trying to get a value
yet.
Makes sense.. though for NON_DEPENDENT_EXPR in particular, ISTM this
tree is always used in a context where a constant expression isn't
required, e.g. in the build_x_* functions.
Fair enough. The patch is OK with a comment to that effect.
Thanks, I committed the following as r12-7264:
Would it be OK to backport this now for 11.3, or shall we wait until
after 11.3 is released? It's been two months, and the only reported
fallout from this patch was PR104620, where we began to fail to
diagnose an invalid consteval call within a template ahead of time
(and it turned out we previously were diagnosing it only by accident).
The patch also fixed PR103443 (non-dependent consteval call
incorrectly rejected).
OK.
-- >8 --
Subject: [PATCH] c++: treat NON_DEPENDENT_EXPR as not potentially constant
[PR104507]
Here we're crashing from potential_constant_expression because it tries
to perform trial evaluation of the first operand '(bool)__r' of the
conjunction (which is overall wrapped in a NON_DEPENDENT_EXPR), but
cxx_eval_constant_expression ICEs on unsupported trees (of which CAST_EXPR
is one). The sequence of events is:
1. build_non_dependent_expr for the array subscript yields
NON_DEPENDENT_EXPR<<<(bool)__r && __s>>> ? 1 : 2
2. cp_build_array_ref calls fold_non_dependent_expr on this subscript
(after this point, processing_template_decl is cleared)
3. during which, the COND_EXPR case of tsubst_copy_and_build calls
fold_non_dependent_expr on the first operand
4. during which, we crash from p_c_e_1 because it attempts trial
evaluation of the CAST_EXPR '(bool)__r'.
Note that even if this crash didn't happen, fold_non_dependent_expr
from cp_build_array_ref would still ultimately be one big no-op here
since neither constexpr evaluation nor tsubst handle NON_DEPENDENT_EXPR.
In light of this and of the observation that we should never see
NON_DEPENDENT_EXPR in a context where a constant expression is needed
(it's used primarily in the build_x_* family of functions), it seems
futile for p_c_e_1 to ever return true for NON_DEPENDENT_EXPR. And the
otherwise inconsistent handling of NON_DEPENDENT_EXPR between p_c_e_1,
cxx_evaluate_constexpr_expression and tsubst apparently leads to weird
bugs such as this one.
PR c++/104507
gcc/cp/ChangeLog:
* constexpr.cc (potential_constant_expression_1)
<case NON_DEPENDENT_EXPR>: Return false instead of recursing.
Assert tf_error isn't set.
gcc/testsuite/ChangeLog:
* g++.dg/template/non-dependent21.C: New test.
---
gcc/cp/constexpr.cc | 9 ++++++++-
gcc/testsuite/g++.dg/template/non-dependent21.C | 9 +++++++++
2 files changed, 17 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/template/non-dependent21.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 7274c3b760e..4716694cb71 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -9065,6 +9065,14 @@ potential_constant_expression_1 (tree t, bool want_rval,
bool strict, bool now,
case BIND_EXPR:
return RECUR (BIND_EXPR_BODY (t), want_rval);
+ case NON_DEPENDENT_EXPR:
+ /* Treat NON_DEPENDENT_EXPR as non-constant: it's not handled by
+ constexpr evaluation or tsubst, so fold_non_dependent_expr can't
+ do anything useful with it. And we shouldn't see it in a context
+ where a constant expression is strictly required, hence the assert. */
+ gcc_checking_assert (!(flags & tf_error));
+ return false;
+
case CLEANUP_POINT_EXPR:
case MUST_NOT_THROW_EXPR:
case TRY_CATCH_EXPR:
@@ -9072,7 +9080,6 @@ potential_constant_expression_1 (tree t, bool want_rval,
bool strict, bool now,
case EH_SPEC_BLOCK:
case EXPR_STMT:
case PAREN_EXPR:
- case NON_DEPENDENT_EXPR:
/* For convenience. */
case LOOP_EXPR:
case EXIT_EXPR:
diff --git a/gcc/testsuite/g++.dg/template/non-dependent21.C
b/gcc/testsuite/g++.dg/template/non-dependent21.C
new file mode 100644
index 00000000000..89900837b8b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/non-dependent21.C
@@ -0,0 +1,9 @@
+// PR c++/104507
+
+extern const char *_k_errmsg[];
+
+template<class>
+const char* DoFoo(int __r, int __s) {
+ const char* n = _k_errmsg[(bool)__r && __s ? 1 : 2];
+ return n;
+}
--
2.35.1.129.gb80121027d
