PR50402: Use proper constant evaluation rules for checking constraint

satisfaction.

Previously we used the rules for constant folding in a non-constant
context, meaning that we'd incorrectly accept foldable non-constant
expressions and that std::is_constant_evaluated() would evaluate to
false.

clang/lib/Sema/SemaConcept.cpp

@@ -172,9 +172,11 @@ calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr,
   SmallVector<PartialDiagnosticAt, 2> EvaluationDiags;
   Expr::EvalResult EvalResult;
   EvalResult.Diag = &EvaluationDiags;
-  if (!SubstitutedAtomicExpr.get()->EvaluateAsRValue(EvalResult, S.Context)) {
+  if (!SubstitutedAtomicExpr.get()->EvaluateAsConstantExpr(EvalResult,
-      // C++2a [temp.constr.atomic]p1
+                                                           S.Context) ||
-      //   ...E shall be a constant expression of type bool.
+      !EvaluationDiags.empty()) {
+    // C++2a [temp.constr.atomic]p1
+    //   ...E shall be a constant expression of type bool.
     S.Diag(SubstitutedAtomicExpr.get()->getBeginLoc(),
            diag::err_non_constant_constraint_expression)
         << SubstitutedAtomicExpr.get()->getSourceRange();
@@ -183,6 +185,8 @@ calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr,
     return true;
   }
 
+  assert(EvalResult.Val.isInt() &&
+         "evaluating bool expression didn't produce int");
   Satisfaction.IsSatisfied = EvalResult.Val.getInt().getBoolValue();
   if (!Satisfaction.IsSatisfied)
     Satisfaction.Details.emplace_back(ConstraintExpr,
@@ -214,6 +218,13 @@ static bool calculateConstraintSatisfaction(
           Sema::SFINAETrap Trap(S);
           SubstitutedExpression = S.SubstExpr(const_cast<Expr *>(AtomicExpr),
                                               MLTAL);
+          // Substitution might have stripped off a contextual conversion to
+          // bool if this is the operand of an '&&' or '||'. For example, we
+          // might lose an lvalue-to-rvalue conversion here. If so, put it back
+          // before we try to evaluate.
+          if (!SubstitutedExpression.isInvalid())
+            SubstitutedExpression =
+                S.PerformContextuallyConvertToBool(SubstitutedExpression.get());
           if (SubstitutedExpression.isInvalid() || Trap.hasErrorOccurred()) {
             // C++2a [temp.constr.atomic]p1
             //   ...If substitution results in an invalid type or expression, the
@@ -523,9 +534,9 @@ static void diagnoseWellFormedUnsatisfiedConstraintExpr(Sema &S,
       diagnoseWellFormedUnsatisfiedConstraintExpr(S, BO->getRHS(),
                                                   /*First=*/false);
       return;
-    case BO_LAnd:
+    case BO_LAnd: {
-      bool LHSSatisfied;
+      bool LHSSatisfied =
-      BO->getLHS()->EvaluateAsBooleanCondition(LHSSatisfied, S.Context);
+          BO->getLHS()->EvaluateKnownConstInt(S.Context).getBoolValue();
       if (LHSSatisfied) {
         // LHS is true, so RHS must be false.
         diagnoseWellFormedUnsatisfiedConstraintExpr(S, BO->getRHS(), First);
@@ -535,12 +546,13 @@ static void diagnoseWellFormedUnsatisfiedConstraintExpr(Sema &S,
       diagnoseWellFormedUnsatisfiedConstraintExpr(S, BO->getLHS(), First);
 
       // RHS might also be false
-      bool RHSSatisfied;
+      bool RHSSatisfied =
-      BO->getRHS()->EvaluateAsBooleanCondition(RHSSatisfied, S.Context);
+          BO->getRHS()->EvaluateKnownConstInt(S.Context).getBoolValue();
       if (!RHSSatisfied)
         diagnoseWellFormedUnsatisfiedConstraintExpr(S, BO->getRHS(),
                                                     /*First=*/false);
       return;
+    }
     case BO_GE:
     case BO_LE:
     case BO_GT:
@@ -551,8 +563,12 @@ static void diagnoseWellFormedUnsatisfiedConstraintExpr(Sema &S,
           BO->getRHS()->getType()->isIntegerType()) {
         Expr::EvalResult SimplifiedLHS;
         Expr::EvalResult SimplifiedRHS;
-        BO->getLHS()->EvaluateAsInt(SimplifiedLHS, S.Context);
+        BO->getLHS()->EvaluateAsInt(SimplifiedLHS, S.Context,
-        BO->getRHS()->EvaluateAsInt(SimplifiedRHS, S.Context);
+                                    Expr::SE_NoSideEffects,
+                                    /*InConstantContext=*/true);
+        BO->getRHS()->EvaluateAsInt(SimplifiedRHS, S.Context,
+                                    Expr::SE_NoSideEffects,
+                                    /*InConstantContext=*/true);
         if (!SimplifiedLHS.Diag && ! SimplifiedRHS.Diag) {
           S.Diag(SubstExpr->getBeginLoc(),
                  diag::note_atomic_constraint_evaluated_to_false_elaborated)

clang/test/SemaTemplate/concepts.cpp

@@ -126,3 +126,31 @@ namespace PackInTypeConstraint {
      ...); // expected-error {{does not contain any unexpanded}}
   }
 }
+
+namespace BuiltinIsConstantEvaluated {
+  // Check that we do all satisfaction and diagnostic checks in a constant context.
+  template<typename T> concept C = __builtin_is_constant_evaluated(); // expected-warning {{always}}
+  static_assert(C<int>);
+
+  template<typename T> concept D = __builtin_is_constant_evaluated() == true; // expected-warning {{always}}
+  static_assert(D<int>);
+
+  template<typename T> concept E = __builtin_is_constant_evaluated() == true && // expected-warning {{always}}
+                                   false; // expected-note {{'false' evaluated to false}}
+  static_assert(E<int>); // expected-error {{failed}} expected-note {{because 'int' does not satisfy 'E'}}
+
+  template<typename T> concept F = __builtin_is_constant_evaluated() == false; // expected-warning {{always}}
+  // expected-note@-1 {{'__builtin_is_constant_evaluated() == false' (1 == 0)}}
+  static_assert(F<int>); // expected-error {{failed}} expected-note {{because 'int' does not satisfy 'F'}}
+
+  template<typename T> concept G = __builtin_is_constant_evaluated() && // expected-warning {{always}}
+                                   false; // expected-note {{'false' evaluated to false}}
+  static_assert(G<int>); // expected-error {{failed}} expected-note {{because 'int' does not satisfy 'G'}}
+}
+
+namespace NoConstantFolding {
+  // Ensure we use strict constant evaluation rules when checking satisfaction.
+  int n;
+  template <class T> concept C = &n + 3 - 3 == &n; // expected-error {{non-constant expression}} expected-note {{cannot refer to element 3 of non-array object}}
+  static_assert(C<void>); // expected-note {{while checking}}
+}

clang/test/SemaTemplate/cxx2a-constraint-caching.cpp

@@ -14,7 +14,7 @@ constexpr bool foo() requires (f(T()), true) { return true; }
 
 namespace a {
   struct A {};
-  void f(A a);
+  constexpr void f(A a) {}
 }
 
 static_assert(C<a::A>);
@@ -23,7 +23,7 @@ static_assert(foo<a::A>());
 namespace a {
   // This makes calls to f ambiguous, but the second check will still succeed
   // because the constraint satisfaction results are cached.
-  void f(A a, int = 2);
+  constexpr void f(A a, int = 2) {}
 }
 #ifdef NO_CACHE
 static_assert(!C<a::A>);
@@ -31,4 +31,4 @@ static_assert(!foo<a::A>());
 #else
 static_assert(C<a::A>);
 static_assert(foo<a::A>());
-#endif
+#endif

clang/test/SemaTemplate/instantiate-requires-clause.cpp

@@ -62,8 +62,8 @@ static_assert((S3<int>::f(), true));
 template<typename T>
 struct S4 {
     template<typename>
-    constexpr void foo() requires (*this, true) { }
+    constexpr void foo() requires (decltype(this)(), true) { }
-    constexpr void goo() requires (*this, true) { }
+    constexpr void goo() requires (decltype(this)(), true) { }
 };
 
 static_assert((S4<int>{}.foo<int>(), S4<int>{}.goo(), true));