[gcc r11-9843] c++: treat NON_DEPENDENT_EXPR as not potentially constant [PR104507]

[Patrick Palka]

https://gcc.gnu.org/g:c8aaa9cca96207b5674048972c82a338ef81ce7e

commit r11-9843-gc8aaa9cca96207b5674048972c82a338ef81ce7e
Author: Patrick Palka <ppalka@redhat.com>
Date:   Wed Feb 16 12:41:35 2022 -0500

    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.c (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.
    
    (cherry picked from commit c19f317a78c0e4c1b51d0e5a8e4c0a3b985b7a8e)

Diff:
---
 gcc/cp/constexpr.c                              | 9 ++++++++-
 gcc/testsuite/g++.dg/template/non-dependent21.C | 9 +++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 284726711b8..c13c920ade3 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -8726,6 +8726,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:
@@ -8733,7 +8741,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;
+}