usaxena95 updated this revision to Diff 478819. usaxena95 added a comment. Remove unintended whitespaces and new lines.
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/include/clang/Basic/DiagnosticSemaKinds.td 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 @@ -51,3 +51,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/Basic/DiagnosticSemaKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ 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< 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