Make evaluation of nested requirement consistent with requires expr.

Fixes: #45563
```
template<class T>  concept True = true;

template <class T>
concept C1 = requires (T) {
   requires True<typename T::value> || True<T>;
};

template <class T>
constexpr bool foo()
requires True<typename T::value> || True<T> {
    return true;
}
static_assert(C1<double>); // Previously failed due to SFINAE error
static_assert(foo<int>()); // but this works fine.
```
The issue here is the discrepancy between how a [nested requirement is evaluated](https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaTemplateInstantiate.cpp#L2331) Vs how a [non-nested requirement is evaluated](https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaConcept.cpp#L167-L200).

This patch makes constraint checking consistent for nested requirement
and trailing requires expressions by reusing the same evaluator.

Differential Revision: https://reviews.llvm.org/D138914

clang/include/clang/AST/ASTConcept.h

@@ -59,6 +59,13 @@ class ConstraintSatisfaction : public llvm::FoldingSetNode {
   static void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &C,
                       const NamedDecl *ConstraintOwner,
                       ArrayRef<TemplateArgument> TemplateArgs);
+
+  bool HasSubstitutionFailure() {
+    for (const auto &Detail : Details)
+      if (Detail.second.dyn_cast<SubstitutionDiagnostic *>())
+        return true;
+    return false;
+  }
 };
 
 /// Pairs of unsatisfied atomic constraint expressions along with the
@@ -91,9 +98,13 @@ struct ASTConstraintSatisfaction final :
 
   ASTConstraintSatisfaction(const ASTContext &C,
                             const ConstraintSatisfaction &Satisfaction);
+  ASTConstraintSatisfaction(const ASTContext &C,
+                            const ASTConstraintSatisfaction &Satisfaction);
 
   static ASTConstraintSatisfaction *
   Create(const ASTContext &C, const ConstraintSatisfaction &Satisfaction);
+  static ASTConstraintSatisfaction *
+  Rebuild(const ASTContext &C, const ASTConstraintSatisfaction &Satisfaction);
 };
 
 /// \brief Common data class for constructs that reference concepts with

clang/include/clang/AST/ASTNodeTraverser.h

@@ -246,7 +246,7 @@ class ASTNodeTraverser
                     .getTypeConstraint()
                     ->getImmediatelyDeclaredConstraint());
       } else if (auto *NR = dyn_cast<concepts::NestedRequirement>(R)) {
-        if (!NR->isSubstitutionFailure())
+        if (!NR->hasInvalidConstraint())
           Visit(NR->getConstraintExpr());
       }
     });

clang/include/clang/AST/ExprConcepts.h

@@ -24,6 +24,7 @@
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/SourceLocation.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/TrailingObjects.h"
 #include <utility>
 #include <string>
@@ -401,57 +402,61 @@ class ExprRequirement : public Requirement {
 /// \brief A requires-expression requirement which is satisfied when a general
 /// constraint expression is satisfied ('nested' requirements).
 class NestedRequirement : public Requirement {
-  llvm::PointerUnion<Expr *, SubstitutionDiagnostic *> Value;
+  Expr *Constraint = nullptr;
   const ASTConstraintSatisfaction *Satisfaction = nullptr;
+  bool HasInvalidConstraint = false;
+  StringRef InvalidConstraintEntity;
 
 public:
   friend ASTStmtReader;
   friend ASTStmtWriter;
 
-  NestedRequirement(SubstitutionDiagnostic *SubstDiag) :
-      Requirement(RK_Nested, /*IsDependent=*/false,
-                  /*ContainsUnexpandedParameterPack*/false,
-                  /*IsSatisfied=*/false), Value(SubstDiag) {}
-
-  NestedRequirement(Expr *Constraint) :
-      Requirement(RK_Nested, /*IsDependent=*/true,
-                  Constraint->containsUnexpandedParameterPack()),
-      Value(Constraint) {
+  NestedRequirement(Expr *Constraint)
+      : Requirement(RK_Nested, /*IsDependent=*/true,
+                    Constraint->containsUnexpandedParameterPack()),
+        Constraint(Constraint) {
     assert(Constraint->isInstantiationDependent() &&
            "Nested requirement with non-dependent constraint must be "
            "constructed with a ConstraintSatisfaction object");
   }
 
   NestedRequirement(ASTContext &C, Expr *Constraint,
-                    const ConstraintSatisfaction &Satisfaction) :
-      Requirement(RK_Nested, Constraint->isInstantiationDependent(),
-                  Constraint->containsUnexpandedParameterPack(),
-                  Satisfaction.IsSatisfied),
-      Value(Constraint),
-      Satisfaction(ASTConstraintSatisfaction::Create(C, Satisfaction)) {}
-
-  bool isSubstitutionFailure() const {
-    return Value.is<SubstitutionDiagnostic *>();
-  }
-
-  SubstitutionDiagnostic *getSubstitutionDiagnostic() const {
-    assert(isSubstitutionFailure() &&
-           "getSubstitutionDiagnostic() may not be called when there was no "
-           "substitution failure.");
-    return Value.get<SubstitutionDiagnostic *>();
+                    const ConstraintSatisfaction &Satisfaction)
+      : Requirement(RK_Nested, Constraint->isInstantiationDependent(),
+                    Constraint->containsUnexpandedParameterPack(),
+                    Satisfaction.IsSatisfied),
+        Constraint(Constraint),
+        Satisfaction(ASTConstraintSatisfaction::Create(C, Satisfaction)) {}
+
+  NestedRequirement(StringRef InvalidConstraintEntity,
+                    const ASTConstraintSatisfaction *Satisfaction)
+      : Requirement(RK_Nested,
+                    /*IsDependent=*/false,
+                    /*ContainsUnexpandedParameterPack*/ false,
+                    Satisfaction->IsSatisfied),
+        Satisfaction(Satisfaction), HasInvalidConstraint(true),
+        InvalidConstraintEntity(InvalidConstraintEntity) {}
+
+  NestedRequirement(ASTContext &C, StringRef InvalidConstraintEntity,
+                    const ConstraintSatisfaction &Satisfaction)
+      : NestedRequirement(InvalidConstraintEntity,
+                          ASTConstraintSatisfaction::Create(C, Satisfaction)) {}
+
+  bool hasInvalidConstraint() const { return HasInvalidConstraint; }
+
+  StringRef getInvalidConstraintEntity() {
+    assert(hasInvalidConstraint());
+    return InvalidConstraintEntity;
   }
 
   Expr *getConstraintExpr() const {
-    assert(!isSubstitutionFailure() && "getConstraintExpr() may not be called "
-                                       "on nested requirements with "
-                                       "substitution failures.");
-    return Value.get<Expr *>();
+    assert(!hasInvalidConstraint() &&
+           "getConstraintExpr() may not be called "
+           "on nested requirements with invalid constraint.");
+    return Constraint;
   }
 
   const ASTConstraintSatisfaction &getConstraintSatisfaction() const {
-    assert(!isSubstitutionFailure() && "getConstraintSatisfaction() may not be "
-                                       "called on nested requirements with "
-                                       "substitution failures.");
     return *Satisfaction;
   }
 

clang/include/clang/AST/RecursiveASTVisitor.h

@@ -600,7 +600,7 @@ bool RecursiveASTVisitor<Derived>::TraverseConceptExprRequirement(
 template <typename Derived>
 bool RecursiveASTVisitor<Derived>::TraverseConceptNestedRequirement(
     concepts::NestedRequirement *R) {
-  if (!R->isSubstitutionFailure())
+  if (!R->hasInvalidConstraint())
     return getDerived().TraverseStmt(R->getConstraintExpr());
   return true;
 }

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -2893,7 +2893,7 @@ def note_type_requirement_substitution_error : Note<
 def note_type_requirement_unknown_substitution_error : Note<
   "%select{and|because}0 '%1' would be invalid">;
 def note_nested_requirement_substitution_error : Note<
-  "%select{and|because}0 '%1' would be invalid: %2">;
+  "%select{and|because}0 '%1' would be invalid%2">;
 def note_nested_requirement_unknown_substitution_error : Note<
   "%select{and|because}0 '%1' would be invalid">;
 def note_ambiguous_atomic_constraints : Note<

clang/include/clang/Sema/Sema.h

@@ -8509,8 +8509,8 @@ class Sema final {
       concepts::Requirement::SubstitutionDiagnostic *SubstDiag);
   concepts::NestedRequirement *BuildNestedRequirement(Expr *E);
   concepts::NestedRequirement *
-  BuildNestedRequirement(
-      concepts::Requirement::SubstitutionDiagnostic *SubstDiag);
+  BuildNestedRequirement(StringRef InvalidConstraintEntity,
+                         const ASTConstraintSatisfaction &Satisfaction);
   ExprResult ActOnRequiresExpr(SourceLocation RequiresKWLoc,
                                RequiresExprBodyDecl *Body,
                                ArrayRef<ParmVarDecl *> LocalParameters,

clang/lib/AST/ASTConcept.cpp

@@ -19,32 +19,48 @@
 #include "llvm/ADT/FoldingSet.h"
 using namespace clang;
 
+namespace {
+void CreatUnsatisfiedConstraintRecord(
+    const ASTContext &C, const UnsatisfiedConstraintRecord &Detail,
+    UnsatisfiedConstraintRecord *TrailingObject) {
+  if (Detail.second.is<Expr *>())
+    new (TrailingObject) UnsatisfiedConstraintRecord{
+        Detail.first,
+        UnsatisfiedConstraintRecord::second_type(Detail.second.get<Expr *>())};
+  else {
+    auto &SubstitutionDiagnostic =
+        *Detail.second.get<std::pair<SourceLocation, StringRef> *>();
+    unsigned MessageSize = SubstitutionDiagnostic.second.size();
+    char *Mem = new (C) char[MessageSize];
+    memcpy(Mem, SubstitutionDiagnostic.second.data(), MessageSize);
+    auto *NewSubstDiag = new (C) std::pair<SourceLocation, StringRef>(
+        SubstitutionDiagnostic.first, StringRef(Mem, MessageSize));
+    new (TrailingObject) UnsatisfiedConstraintRecord{
+        Detail.first, UnsatisfiedConstraintRecord::second_type(NewSubstDiag)};
+  }
+}
+} // namespace
+
 ASTConstraintSatisfaction::ASTConstraintSatisfaction(
     const ASTContext &C, const ConstraintSatisfaction &Satisfaction)
     : NumRecords{Satisfaction.Details.size()},
       IsSatisfied{Satisfaction.IsSatisfied}, ContainsErrors{
                                                  Satisfaction.ContainsErrors} {
-  for (unsigned I = 0; I < NumRecords; ++I) {
-    auto &Detail = Satisfaction.Details[I];
-    if (Detail.second.is<Expr *>())
-      new (getTrailingObjects<UnsatisfiedConstraintRecord>() + I)
-         UnsatisfiedConstraintRecord{Detail.first,
-                                     UnsatisfiedConstraintRecord::second_type(
-                                         Detail.second.get<Expr *>())};
-    else {
-      auto &SubstitutionDiagnostic =
-          *Detail.second.get<std::pair<SourceLocation, StringRef> *>();
-      unsigned MessageSize = SubstitutionDiagnostic.second.size();
-      char *Mem = new (C) char[MessageSize];
-      memcpy(Mem, SubstitutionDiagnostic.second.data(), MessageSize);
-      auto *NewSubstDiag = new (C) std::pair<SourceLocation, StringRef>(
-          SubstitutionDiagnostic.first, StringRef(Mem, MessageSize));
-      new (getTrailingObjects<UnsatisfiedConstraintRecord>() + I)
-         UnsatisfiedConstraintRecord{Detail.first,
-                                     UnsatisfiedConstraintRecord::second_type(
-                                         NewSubstDiag)};
-    }
-  }
+  for (unsigned I = 0; I < NumRecords; ++I)
+    CreatUnsatisfiedConstraintRecord(
+        C, Satisfaction.Details[I],
+        getTrailingObjects<UnsatisfiedConstraintRecord>() + I);
+}
+
+ASTConstraintSatisfaction::ASTConstraintSatisfaction(
+    const ASTContext &C, const ASTConstraintSatisfaction &Satisfaction)
+    : NumRecords{Satisfaction.NumRecords},
+      IsSatisfied{Satisfaction.IsSatisfied},
+      ContainsErrors{Satisfaction.ContainsErrors} {
+  for (unsigned I = 0; I < NumRecords; ++I)
+    CreatUnsatisfiedConstraintRecord(
+        C, *(Satisfaction.begin() + I),
+        getTrailingObjects<UnsatisfiedConstraintRecord>() + I);
 }
 
 ASTConstraintSatisfaction *
@@ -57,6 +73,14 @@ ASTConstraintSatisfaction::Create(const ASTContext &C,
   return new (Mem) ASTConstraintSatisfaction(C, Satisfaction);
 }
 
+ASTConstraintSatisfaction *ASTConstraintSatisfaction::Rebuild(
+    const ASTContext &C, const ASTConstraintSatisfaction &Satisfaction) {
+  std::size_t size =
+      totalSizeToAlloc<UnsatisfiedConstraintRecord>(Satisfaction.NumRecords);
+  void *Mem = C.Allocate(size, alignof(ASTConstraintSatisfaction));
+  return new (Mem) ASTConstraintSatisfaction(C, Satisfaction);
+}
+
 void ConstraintSatisfaction::Profile(
     llvm::FoldingSetNodeID &ID, const ASTContext &C,
     const NamedDecl *ConstraintOwner, ArrayRef<TemplateArgument> TemplateArgs) {

clang/lib/AST/StmtPrinter.cpp

@@ -2528,7 +2528,7 @@ void StmtPrinter::VisitRequiresExpr(RequiresExpr *E) {
     } else {
       auto *NestedReq = cast<concepts::NestedRequirement>(Req);
       OS << "requires ";
-      if (NestedReq->isSubstitutionFailure())
+      if (NestedReq->hasInvalidConstraint())
         OS << "<<error-expression>>";
       else
         PrintExpr(NestedReq->getConstraintExpr());

clang/lib/AST/StmtProfile.cpp

@@ -1622,8 +1622,8 @@ void StmtProfiler::VisitRequiresExpr(const RequiresExpr *S) {
     } else {
       ID.AddInteger(concepts::Requirement::RK_Nested);
       auto *NestedReq = cast<concepts::NestedRequirement>(Req);
-      ID.AddBoolean(NestedReq->isSubstitutionFailure());
-      if (!NestedReq->isSubstitutionFailure())
+      ID.AddBoolean(NestedReq->hasInvalidConstraint());
+      if (!NestedReq->hasInvalidConstraint())
         Visit(NestedReq->getConstraintExpr());
     }
   }

clang/lib/Sema/SemaConcept.cpp

@@ -180,7 +180,8 @@ calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr,
       //    is checked. If that is satisfied, the disjunction is satisfied.
       //    Otherwise, the disjunction is satisfied if and only if the second
       //    operand is satisfied.
-      return BO.recreateBinOp(S, LHSRes);
+      // LHS is instantiated while RHS is not. Skip creating invalid BinaryOp.
+      return LHSRes;
 
     if (BO.isAnd() && !IsLHSSatisfied)
       // [temp.constr.op] p2
@@ -189,7 +190,8 @@ calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr,
       //    is checked. If that is not satisfied, the conjunction is not
       //    satisfied. Otherwise, the conjunction is satisfied if and only if
       //    the second operand is satisfied.
-      return BO.recreateBinOp(S, LHSRes);
+      // LHS is instantiated while RHS is not. Skip creating invalid BinaryOp.
+      return LHSRes;
 
     ExprResult RHSRes = calculateConstraintSatisfaction(
         S, BO.getRHS(), Satisfaction, std::forward<AtomicEvaluator>(Evaluator));
@@ -330,7 +332,8 @@ static ExprResult calculateConstraintSatisfaction(
           // 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())
+          if (SubstitutedExpression.isUsable() &&
+              !SubstitutedExpression.isInvalid())
             SubstitutedExpression =
                 S.PerformContextuallyConvertToBool(SubstitutedExpression.get());
           if (SubstitutedExpression.isInvalid() || Trap.hasErrorOccurred()) {
@@ -898,31 +901,28 @@ static void diagnoseUnsatisfiedRequirement(Sema &S,
     return;
   }
 }
+static void diagnoseWellFormedUnsatisfiedConstraintExpr(Sema &S,
+                                                        Expr *SubstExpr,
+                                                        bool First = true);
 
 static void diagnoseUnsatisfiedRequirement(Sema &S,
                                            concepts::NestedRequirement *Req,
                                            bool First) {
-  if (Req->isSubstitutionFailure()) {
-    concepts::Requirement::SubstitutionDiagnostic *SubstDiag =
-        Req->getSubstitutionDiagnostic();
-    if (!SubstDiag->DiagMessage.empty())
-      S.Diag(SubstDiag->DiagLoc,
-             diag::note_nested_requirement_substitution_error)
-             << (int)First << SubstDiag->SubstitutedEntity
-             << SubstDiag->DiagMessage;
+  using SubstitutionDiagnostic = std::pair<SourceLocation, StringRef>;
+  for (auto &Pair : Req->getConstraintSatisfaction()) {
+    if (auto *SubstDiag = Pair.second.dyn_cast<SubstitutionDiagnostic *>())
+      S.Diag(SubstDiag->first, diag::note_nested_requirement_substitution_error)
+          << (int)First << Req->getInvalidConstraintEntity() << SubstDiag->second;
     else
-      S.Diag(SubstDiag->DiagLoc,
-             diag::note_nested_requirement_unknown_substitution_error)
-          << (int)First << SubstDiag->SubstitutedEntity;
-    return;
+      diagnoseWellFormedUnsatisfiedConstraintExpr(
+          S, Pair.second.dyn_cast<Expr *>(), First);
+    First = false;
   }
-  S.DiagnoseUnsatisfiedConstraint(Req->getConstraintSatisfaction(), First);
 }
 
-
 static void diagnoseWellFormedUnsatisfiedConstraintExpr(Sema &S,
                                                         Expr *SubstExpr,
-                                                        bool First = true) {
+                                                        bool First) {
   SubstExpr = SubstExpr->IgnoreParenImpCasts();
   if (BinaryOperator *BO = dyn_cast<BinaryOperator>(SubstExpr)) {
     switch (BO->getOpcode()) {

clang/lib/Sema/SemaExprCXX.cpp

@@ -9086,9 +9086,11 @@ Sema::BuildNestedRequirement(Expr *Constraint) {
 }
 
 concepts::NestedRequirement *
-Sema::BuildNestedRequirement(
-    concepts::Requirement::SubstitutionDiagnostic *SubstDiag) {
-  return new (Context) concepts::NestedRequirement(SubstDiag);
+Sema::BuildNestedRequirement(StringRef InvalidConstraintEntity,
+                       const ASTConstraintSatisfaction &Satisfaction) {
+  return new (Context) concepts::NestedRequirement(
+      InvalidConstraintEntity,
+      ASTConstraintSatisfaction::Rebuild(Context, Satisfaction));
 }
 
 RequiresExprBodyDecl *