diff --git a/clang/include/clang/AST/ASTConcept.h b/clang/include/clang/AST/ASTConcept.h --- a/clang/include/clang/AST/ASTConcept.h +++ b/clang/include/clang/AST/ASTConcept.h @@ -59,6 +59,13 @@ static void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &C, const NamedDecl *ConstraintOwner, ArrayRef TemplateArgs); + + bool HasSubstitutionFailure() { + for (const auto &Detail : Details) + if (Detail.second.dyn_cast()) + return true; + return false; + } }; /// Pairs of unsatisfied atomic constraint expressions along with the diff --git a/clang/include/clang/AST/ExprConcepts.h b/clang/include/clang/AST/ExprConcepts.h --- a/clang/include/clang/AST/ExprConcepts.h +++ b/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 #include @@ -403,6 +404,8 @@ class NestedRequirement : public Requirement { llvm::PointerUnion 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() && + "NestedRequirement does not have Substitution diagnostics anymore."); return Value.is(); } + 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."); diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2888,7 +2888,7 @@ 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< diff --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp --- a/clang/lib/AST/StmtPrinter.cpp +++ b/clang/lib/AST/StmtPrinter.cpp @@ -2516,7 +2516,12 @@ } else { auto *NestedReq = cast(Req); OS << "requires "; - if (NestedReq->isSubstitutionFailure()) + if (NestedReq->isSubstitutionFailure()) { + llvm_unreachable("NestedRequirement does not have Substitution " + "diagnostics anymore."); + OS << "<>"; + } + if (NestedReq->hasInvalidConstraint()) OS << "<>"; else PrintExpr(NestedReq->getConstraintExpr()); diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp --- a/clang/lib/Sema/SemaConcept.cpp +++ b/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(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; + for (auto &Pair : Req->getConstraintSatisfaction()) { + if (auto *SubstDiag = Pair.second.dyn_cast()) + S.Diag(SubstDiag->first, diag::note_nested_requirement_substitution_error) + << (int)First << Req->getInvalidConstraintEntity() << SubstDiag->second; + else + diagnoseWellFormedUnsatisfiedConstraintExpr( + S, Pair.second.dyn_cast(), First); + First = false; + } } - static void diagnoseWellFormedUnsatisfiedConstraintExpr(Sema &S, Expr *SubstExpr, - bool First = true) { + bool First) { SubstExpr = SubstExpr->IgnoreParenImpCasts(); if (BinaryOperator *BO = dyn_cast(SubstExpr)) { switch (BO->getOpcode()) { diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/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); } diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/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 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); } diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h --- a/clang/lib/Sema/TreeTransform.h +++ b/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); } diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp --- a/clang/lib/Serialization/ASTReaderStmt.cpp +++ b/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); diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp --- a/clang/lib/Serialization/ASTWriterStmt.cpp +++ b/clang/lib/Serialization/ASTWriterStmt.cpp @@ -494,9 +494,16 @@ auto *NestedReq = cast(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()); if (!NestedReq->isDependent()) diff --git a/clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp b/clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp --- a/clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp +++ b/clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp @@ -51,3 +51,115 @@ X.next(); }; +namespace SubstitutionFailureNestedRequires { +template concept True = true; +template concept False = false; + +struct S { double value; }; + +template +concept Pipes = requires (T x) { + requires True || True || False; + requires False || True || True; +}; + +template +concept Amps1 = requires (T x) { + requires True && True && !False; // #Amps1 +}; +template +concept Amps2 = requires (T x) { + requires True && True; +}; + +static_assert(Pipes); +static_assert(Pipes); + +static_assert(Amps1); +static_assert(!Amps1); + +static_assert(Amps2); +static_assert(!Amps2); + +template +void foo1() requires requires (T x) { // #foo1 + requires + True // #foo1Value + && True; +} {} +template void fooPipes() requires Pipes {} +template void fooAmps1() requires Amps1 {} // #fooAmps1 +void foo() { + foo1(); + foo1(); // expected-error {{no matching function for call to 'foo1'}} + // expected-note@#foo1Value {{because 'True && True' 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(); + fooPipes(); + fooAmps1(); + fooAmps1(); // 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 && True && !False' would be invalid: member reference base type 'int' is not a structure or union}} +} + +template +concept HasNoValue = requires (T x) { + requires !True && True; +}; +// FIXME: 'int' does not satisfy 'HasNoValue' currently since `!True` 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); +static_assert(!HasNoValue); + +template constexpr bool NotAConceptTrue = true; +template +concept SFinNestedRequires = requires (T x) { + // SF in a non-concept specialisation should also be evaluated to false. + requires NotAConceptTrue || NotAConceptTrue; +}; +static_assert(SFinNestedRequires); +static_assert(SFinNestedRequires); +template +void foo() requires SFinNestedRequires {} +void bar() { + foo(); + foo(); +} +namespace ErrorExpressions_NotSF { +template 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 concept C = true; +template concept F = false; + +template requires requires(T) { requires C || X::value; } void foo(); + +template requires requires(T) { requires C && X::value; } void bar(); // #bar +template requires requires(T) { requires F || (X::value && C); } void baz(); + +void func() { + foo(); + foo(); + foo(); + + bar(); + bar(); + // 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::value' evaluated to false}} + + bar(); + // expected-note@-1 {{while checking constraint satisfaction for template 'bar' 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}} +} +} +} diff --git a/clang/test/PCH/cxx2a-requires-expr.cpp b/clang/test/PCH/cxx2a-requires-expr.cpp --- a/clang/test/PCH/cxx2a-requires-expr.cpp +++ b/clang/test/PCH/cxx2a-requires-expr.cpp @@ -12,12 +12,13 @@ template bool f() { - // CHECK: requires (T t) { t++; { t++ } noexcept -> C; { t++ } -> C2; typename T::a; requires T::val; }; + // CHECK: requires (T t) { t++; { t++ } noexcept -> C; { t++ } -> C2; typename T::a; requires T::val; requires C || (C || C); }; return requires (T t) { t++; { t++ } noexcept -> C; { t++ } -> C2; typename T::a; requires T::val; + requires C || (C || C); }; }