usaxena95 updated this revision to Diff 478814.
usaxena95 marked 4 inline comments as done.
usaxena95 added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138914/new/

https://reviews.llvm.org/D138914

Files:
  clang/include/clang/AST/ASTConcept.h
  clang/include/clang/AST/ExprConcepts.h
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp
  clang/test/PCH/cxx2a-requires-expr.cpp

Index: clang/test/PCH/cxx2a-requires-expr.cpp
===================================================================
--- clang/test/PCH/cxx2a-requires-expr.cpp
+++ clang/test/PCH/cxx2a-requires-expr.cpp
@@ -12,12 +12,13 @@
 
 template<typename T>
 bool f() {
-  // CHECK: requires (T t) { t++; { t++ } noexcept -> C; { t++ } -> C2<int>; typename T::a; requires T::val; };
+  // CHECK: requires (T t) { t++; { t++ } noexcept -> C; { t++ } -> C2<int>; typename T::a; requires T::val; requires C<typename T::val> || (C<typename T::val> || C<T>); };
   return requires (T t) {
     t++;
     { t++ } noexcept -> C;
     { t++ } -> C2<int>;
     typename T::a;
     requires T::val;
+    requires C<typename T::val> || (C<typename T::val> || C<T>);
   };
 }
Index: clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp
===================================================================
--- clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp
+++ clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp
@@ -19,7 +19,8 @@
 
 template<typename T>
 struct X {
-    template<typename U> requires requires (U u) { requires sizeof(u) == sizeof(T); } // expected-note{{because 'sizeof (u) == sizeof(T)' would be invalid: invalid application of 'sizeof' to an incomplete type 'void'}}
+    template<typename U> requires requires (U u) { requires sizeof(u) == sizeof(T); } 
+    // expected-note@-1 {{because 'sizeof (u) == sizeof(T)' would be invalid: : invalid application of 'sizeof' to an incomplete type 'void'}}
     struct r4 {};
 };
 
@@ -41,7 +42,7 @@
   template<typename T>
   concept C2 = requires (T a) {
       requires sizeof(a) == 4; // OK
-      requires a == 0; // expected-note{{because 'a == 0' would be invalid: constraint variable 'a' cannot be used in an evaluated context}}
+      requires a == 0; // expected-note{{because 'a == 0' would be invalid: : constraint variable 'a' cannot be used in an evaluated context}}
     };
   static_assert(C2<int>); // expected-note{{because 'int' does not satisfy 'C2'}} expected-error{{static assertion failed}}
 }
@@ -51,3 +52,115 @@
   X.next();
 };
 
+namespace SubstitutionFailureNestedRequires {
+template<class T>  concept True = true;
+template<class T>  concept False = false;
+
+struct S { double value; };
+
+template <class T>
+concept Pipes = requires (T x) {
+   requires True<decltype(x.value)> || True<T> || False<T>;
+   requires False<T> || True<T> || True<decltype(x.value)>;
+};
+
+template <class T>
+concept Amps1 = requires (T x) {
+   requires True<decltype(x.value)> && True<T> && !False<T>; // #Amps1
+};
+template <class T>
+concept Amps2 = requires (T x) {
+   requires True<T> && True<decltype(x.value)>;
+};
+
+static_assert(Pipes<S>);
+static_assert(Pipes<double>);
+
+static_assert(Amps1<S>);
+static_assert(!Amps1<double>);
+
+static_assert(Amps2<S>);
+static_assert(!Amps2<double>);
+
+template<class T>
+void foo1() requires requires (T x) { // #foo1
+  requires
+  True<decltype(x.value)> // #foo1Value
+  && True<T>;
+} {}
+template<class T> void fooPipes() requires Pipes<T> {}
+template<class T> void fooAmps1() requires Amps1<T> {} // #fooAmps1
+void foo() {
+  foo1<S>();
+  foo1<int>(); // expected-error {{no matching function for call to 'foo1'}}
+  // expected-note@#foo1Value {{because 'True<decltype(x.value)> && True<T>' would be invalid: : member reference base type 'int' is not a structure or union}}
+  // expected-note@#foo1 {{candidate template ignored: constraints not satisfied [with T = int]}}
+  fooPipes<S>();
+  fooPipes<int>();
+  fooAmps1<S>();
+  fooAmps1<int>(); // expected-error {{no matching function for call to 'fooAmps1'}}
+  // expected-note@#fooAmps1 {{candidate template ignored: constraints not satisfied [with T = int]}}
+  // expected-note@#fooAmps1 {{because 'int' does not satisfy 'Amps1'}}
+  // expected-note@#Amps1 {{because 'True<decltype(x.value)> && True<T> && !False<T>' would be invalid: : member reference base type 'int' is not a structure or union}}
+}
+
+template<class T>
+concept HasNoValue = requires (T x) {
+  requires !True<decltype(x.value)> && True<T>;
+};
+// FIXME: 'int' does not satisfy 'HasNoValue' currently since `!True<decltype(x.value)>` is an invalid expression.
+// But, in principle, it should be constant-evaluated to true.
+// This happens also for requires expression and is not restricted to nested requirement.
+static_assert(!HasNoValue<int>);
+static_assert(!HasNoValue<S>);
+
+template<class T> constexpr bool NotAConceptTrue = true;
+template <class T>
+concept SFinNestedRequires = requires (T x) {
+    // SF in a non-concept specialisation should also be evaluated to false.
+   requires NotAConceptTrue<decltype(x.value)> || NotAConceptTrue<T>;
+};
+static_assert(SFinNestedRequires<int>);
+static_assert(SFinNestedRequires<S>);
+template <class T>
+void foo() requires SFinNestedRequires<T> {} 
+void bar() { 
+  foo<int>();
+  foo<S>();
+}
+namespace ErrorExpressions_NotSF {
+template<typename T> struct X { static constexpr bool value = T::value; }; // #X_Value
+struct True { static constexpr bool value = true; };
+struct False { static constexpr bool value = false; };
+template<typename T> concept C = true;
+template<typename T> concept F = false;
+
+template<typename T> requires requires(T) { requires C<T> || X<T>::value; } void foo();
+
+template<typename T> requires requires(T) { requires C<T> && X<T>::value; } void bar(); // #bar
+template<typename T> requires requires(T) { requires F<T> || (X<T>::value && C<T>); } void baz();
+
+void func() {
+  foo<True>();
+  foo<False>();
+  foo<int>();
+
+  bar<True>();
+  bar<False>(); 
+  // expected-error@-1 {{no matching function for call to 'bar'}}
+  // expected-note@#bar {{while substituting template arguments into constraint expression here}}
+  // expected-note@#bar {{while checking the satisfaction of nested requirement requested here}}
+  // expected-note@#bar {{candidate template ignored: constraints not satisfied [with T = False]}}
+  // expected-note@#bar {{because 'X<False>::value' evaluated to false}}
+
+  bar<int>(); 
+  // expected-note@-1 {{while checking constraint satisfaction for template 'bar<int>' required here}} \
+  // expected-note@-1 {{in instantiation of function template specialization}}
+  // expected-note@#bar {{in instantiation of static data member}}
+  // expected-note@#bar {{in instantiation of requirement here}}
+  // expected-note@#bar {{while checking the satisfaction of nested requirement requested here}}
+  // expected-note@#bar {{while substituting template arguments into constraint expression here}}
+  // expected-error@#X_Value {{type 'int' cannot be used prior to '::' because it has no members}}
+}
+}
+}
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===================================================================
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -494,9 +494,16 @@
       auto *NestedReq = cast<concepts::NestedRequirement>(R);
       Record.push_back(concepts::Requirement::RK_Nested);
       Record.push_back(NestedReq->isSubstitutionFailure());
-      if (NestedReq->isSubstitutionFailure()){
+      Record.push_back(NestedReq->hasInvalidConstraint());
+      if (NestedReq->isSubstitutionFailure()) {
+        llvm_unreachable("NestedRequirement does not have Substitution "
+                         "diagnostics anymore.");
         addSubstitutionDiagnostic(Record,
                                   NestedReq->getSubstitutionDiagnostic());
+      }
+      if (NestedReq->hasInvalidConstraint()) {
+        Record.AddString(NestedReq->getInvalidConstraintEntity());
+        addConstraintSatisfaction(Record, *NestedReq->Satisfaction);
       } else {
         Record.AddStmt(NestedReq->Value.get<Expr *>());
         if (!NestedReq->isDependent())
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===================================================================
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -895,11 +895,27 @@
                   std::move(*Req));
       } break;
       case concepts::Requirement::RK_Nested: {
-        if (/* IsSubstitutionDiagnostic */Record.readInt()) {
+        bool IsSubstitutionDiagnostic = Record.readInt();
+        bool HasInvalidConstraint = Record.readInt();
+        if (IsSubstitutionDiagnostic) {
+          llvm_unreachable("NestedRequirement does not have Substitution "
+                           "diagnostics anymore.");
           R = new (Record.getContext()) concepts::NestedRequirement(
               readSubstitutionDiagnostic(Record));
           break;
         }
+        if (HasInvalidConstraint) {
+          std::string InvalidConstraint = Record.readString();
+          char *InvalidConstraintBuf =
+              new (Record.getContext()) char[InvalidConstraint.size()];
+          std::copy(InvalidConstraint.begin(), InvalidConstraint.end(),
+                    InvalidConstraintBuf);
+          R = new (Record.getContext()) concepts::NestedRequirement(
+              Record.getContext(),
+              StringRef(InvalidConstraintBuf, InvalidConstraint.size()),
+              readConstraintSatisfaction(Record));
+          break;
+        }
         Expr *E = Record.readExpr();
         if (E->isInstantiationDependent())
           R = new (Record.getContext()) concepts::NestedRequirement(E);
Index: clang/lib/Sema/TreeTransform.h
===================================================================
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3519,6 +3519,8 @@
   concepts::NestedRequirement *
   RebuildNestedRequirement(
       concepts::Requirement::SubstitutionDiagnostic *SubstDiag) {
+    llvm_unreachable(
+        "NestedRequirement does not have Substitution diagnostics anymore.");
     return SemaRef.BuildNestedRequirement(SubstDiag);
   }
 
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -19,6 +19,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprConcepts.h"
 #include "clang/AST/PrettyDeclStackTrace.h"
+#include "clang/AST/Type.h"
 #include "clang/AST/TypeVisitor.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/Stack.h"
@@ -26,6 +27,7 @@
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
+#include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaConcept.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
@@ -2306,6 +2308,8 @@
   if (!Req->isDependent() && !AlwaysRebuild())
     return Req;
   if (Req->isSubstitutionFailure()) {
+    llvm_unreachable(
+        "NestedRequirement does not have Substitution diagnostics anymore.");
     if (AlwaysRebuild())
       return RebuildNestedRequirement(
           Req->getSubstitutionDiagnostic());
@@ -2328,36 +2332,29 @@
         Req->getConstraintExpr()->getSourceRange());
     if (ConstrInst.isInvalid())
       return nullptr;
-    TransConstraint = TransformExpr(Req->getConstraintExpr());
-    if (!TransConstraint.isInvalid()) {
-      bool CheckSucceeded =
-          SemaRef.CheckConstraintExpression(TransConstraint.get());
-      (void)CheckSucceeded;
-      assert((CheckSucceeded || Trap.hasErrorOccurred()) &&
-                                   "CheckConstraintExpression failed, but "
-                                   "did not produce a SFINAE error");
-    }
-    // Use version of CheckConstraintSatisfaction that does no substitutions.
-    if (!TransConstraint.isInvalid() &&
-        !TransConstraint.get()->isInstantiationDependent() &&
-        !Trap.hasErrorOccurred()) {
-      bool CheckFailed = SemaRef.CheckConstraintSatisfaction(
-          TransConstraint.get(), Satisfaction);
-      (void)CheckFailed;
-      assert((!CheckFailed || Trap.hasErrorOccurred()) &&
-                                 "CheckConstraintSatisfaction failed, "
-                                 "but did not produce a SFINAE error");
-    }
-    if (TransConstraint.isInvalid() || Trap.hasErrorOccurred())
-      return RebuildNestedRequirement(createSubstDiag(SemaRef, Info,
-          [&] (llvm::raw_ostream& OS) {
-              Req->getConstraintExpr()->printPretty(OS, nullptr,
-                                                    SemaRef.getPrintingPolicy());
-          }));
+    llvm::SmallVector<Expr *> Result;
+    if (!SemaRef.CheckConstraintSatisfaction(
+            nullptr, {Req->getConstraintExpr()}, Result, TemplateArgs,
+            Req->getConstraintExpr()->getSourceRange(), Satisfaction))
+      TransConstraint = Result[0];
+    assert(!Trap.hasErrorOccurred() && "Substitution failures must be handled "
+                                       "by CheckConstraintSatisfaction.");
   }
-  if (TransConstraint.get()->isInstantiationDependent())
+  if (TransConstraint.isUsable() &&
+      TransConstraint.get()->isInstantiationDependent())
     return new (SemaRef.Context)
         concepts::NestedRequirement(TransConstraint.get());
+  if (TransConstraint.isInvalid() || !TransConstraint.get() ||
+      Satisfaction.HasSubstitutionFailure()) {
+    SmallString<128> Entity;
+    llvm::raw_svector_ostream OS(Entity);
+    Req->getConstraintExpr()->printPretty(OS, nullptr,
+                                          SemaRef.getPrintingPolicy());
+    char *EntityBuf = new (SemaRef.Context) char[Entity.size()];
+    std::copy(Entity.begin(), Entity.end(), EntityBuf);
+    return new (SemaRef.Context) concepts::NestedRequirement(
+        SemaRef.Context, StringRef(EntityBuf, Entity.size()), Satisfaction);
+  }
   return new (SemaRef.Context) concepts::NestedRequirement(
       SemaRef.Context, TransConstraint.get(), Satisfaction);
 }
Index: clang/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -9028,6 +9028,8 @@
 concepts::NestedRequirement *
 Sema::BuildNestedRequirement(
     concepts::Requirement::SubstitutionDiagnostic *SubstDiag) {
+  llvm_unreachable("Nested requirement with non-dependent constraint must be "
+                   "constructed with a ConstraintSatisfaction object");
   return new (Context) concepts::NestedRequirement(SubstDiag);
 }
 
Index: clang/lib/Sema/SemaConcept.cpp
===================================================================
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -169,7 +169,8 @@
       //    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
@@ -178,7 +179,8 @@
       //    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));
@@ -286,7 +288,8 @@
           // 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()) {
@@ -853,6 +856,9 @@
     return;
   }
 }
+static void diagnoseWellFormedUnsatisfiedConstraintExpr(Sema &S,
+                                                        Expr *SubstExpr,
+                                                        bool First = true);
 
 static void diagnoseUnsatisfiedRequirement(Sema &S,
                                            concepts::NestedRequirement *Req,
@@ -871,13 +877,21 @@
           << (int)First << SubstDiag->SubstitutedEntity;
     return;
   }
-  S.DiagnoseUnsatisfiedConstraint(Req->getConstraintSatisfaction(), First);
+  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
+      diagnoseWellFormedUnsatisfiedConstraintExpr(
+          S, Pair.second.dyn_cast<Expr *>(), First);
+    First = false;
+  }
 }
 
-
 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()) {
Index: clang/lib/AST/StmtPrinter.cpp
===================================================================
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -2516,7 +2516,12 @@
     } else {
       auto *NestedReq = cast<concepts::NestedRequirement>(Req);
       OS << "requires ";
-      if (NestedReq->isSubstitutionFailure())
+      if (NestedReq->isSubstitutionFailure()) {
+        llvm_unreachable("NestedRequirement does not have Substitution "
+                         "diagnostics anymore.");
+        OS << "<<error-expression>>";
+      }
+      if (NestedReq->hasInvalidConstraint())
         OS << "<<error-expression>>";
       else
         PrintExpr(NestedReq->getConstraintExpr());
Index: clang/include/clang/AST/ExprConcepts.h
===================================================================
--- clang/include/clang/AST/ExprConcepts.h
+++ 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>
@@ -403,6 +404,8 @@
 class NestedRequirement : public Requirement {
   llvm::PointerUnion<Expr *, SubstitutionDiagnostic *> Value;
   const ASTConstraintSatisfaction *Satisfaction = nullptr;
+  bool HasInvalidConstraint = false;
+  StringRef InvalidConstraintEntity;
 
 public:
   friend ASTStmtReader;
@@ -423,18 +426,37 @@
   }
 
   NestedRequirement(ASTContext &C, Expr *Constraint,
-                    const ConstraintSatisfaction &Satisfaction) :
-      Requirement(RK_Nested, Constraint->isInstantiationDependent(),
-                  Constraint->containsUnexpandedParameterPack(),
-                  Satisfaction.IsSatisfied),
-      Value(Constraint),
-      Satisfaction(ASTConstraintSatisfaction::Create(C, Satisfaction)) {}
+                    const ConstraintSatisfaction &Satisfaction)
+      : Requirement(RK_Nested, Constraint->isInstantiationDependent(),
+                    Constraint->containsUnexpandedParameterPack(),
+                    Satisfaction.IsSatisfied),
+        Value(Constraint),
+        Satisfaction(ASTConstraintSatisfaction::Create(C, Satisfaction)) {}
+
+  NestedRequirement(ASTContext &C, StringRef InvalidConstraintEntity,
+                    const ConstraintSatisfaction &Satisfaction)
+      : Requirement(RK_Nested,
+                    /*IsDependent=*/false,
+                    /*ContainsUnexpandedParameterPack*/ false,
+                    Satisfaction.IsSatisfied),
+        Satisfaction(ASTConstraintSatisfaction::Create(C, Satisfaction)),
+        HasInvalidConstraint(true),
+        InvalidConstraintEntity(InvalidConstraintEntity) {}
 
   bool isSubstitutionFailure() const {
+    assert(!Value.is<SubstitutionDiagnostic *>() &&
+           "NestedRequirement does not have Substitution diagnostics anymore.");
     return Value.is<SubstitutionDiagnostic *>();
   }
 
+  bool hasInvalidConstraint() { return HasInvalidConstraint; }
+  StringRef getInvalidConstraintEntity() {
+    assert(hasInvalidConstraint());
+    return InvalidConstraintEntity;
+  }
+
   SubstitutionDiagnostic *getSubstitutionDiagnostic() const {
+    llvm_unreachable("NestedRequirement does not have Substitution diagnostics anymore.");
     assert(isSubstitutionFailure() &&
            "getSubstitutionDiagnostic() may not be called when there was no "
            "substitution failure.");
Index: clang/include/clang/AST/ASTConcept.h
===================================================================
--- clang/include/clang/AST/ASTConcept.h
+++ clang/include/clang/AST/ASTConcept.h
@@ -59,6 +59,13 @@
   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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to