diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -586,9 +586,11 @@ ([temp.func.order]p6.2.1 is not implemented, matching GCC). - Implemented `P0857R0 `_, which specifies constrained lambdas and constrained template *template-parameter*\s. - - Do not hide templated base members introduced via using-decl in derived class (useful specially for constrained members). Fixes `GH50886 `_. +- Substitution failure in concept specialisation now makes the concept unsatisfied instead of + an invalid expression. Fixes evaluation of nested requirements involving such concepts + along with logical binary operators: `Issue 45563 `_. C++2b Feature Support ^^^^^^^^^^^^^^^^^^^^^ 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 @@ -53,6 +53,7 @@ /// given arguments. If this expression is value dependent, this is to be /// ignored. ASTConstraintSatisfaction *Satisfaction; + bool ArgsHasSubstitutionFailure = false; ConceptSpecializationExpr(const ASTContext &C, NestedNameSpecifierLoc NNS, SourceLocation TemplateKWLoc, @@ -60,7 +61,8 @@ NamedDecl *FoundDecl, ConceptDecl *NamedConcept, const ASTTemplateArgumentListInfo *ArgsAsWritten, ImplicitConceptSpecializationDecl *SpecDecl, - const ConstraintSatisfaction *Satisfaction); + const ConstraintSatisfaction *Satisfaction, + bool ArgsHasSubstitutionFailure = false); ConceptSpecializationExpr(const ASTContext &C, ConceptDecl *NamedConcept, ImplicitConceptSpecializationDecl *SpecDecl, @@ -84,10 +86,22 @@ const ConstraintSatisfaction *Satisfaction, bool Dependent, bool ContainsUnexpandedParameterPack); + static ConceptSpecializationExpr * + CreateSubstitutionFailure(const ASTContext &C, NestedNameSpecifierLoc NNS, + SourceLocation TemplateKWLoc, + DeclarationNameInfo ConceptNameInfo, + NamedDecl *FoundDecl, ConceptDecl *NamedConcept, + const ConstraintSatisfaction *Satisfaction); + ArrayRef getTemplateArguments() const { + assert(SpecDecl); return SpecDecl->getTemplateArguments(); } + bool hasSubstitutionFailureInArgs() const { + return ArgsHasSubstitutionFailure; + } + const ImplicitConceptSpecializationDecl *getSpecializationDecl() const { assert(SpecDecl && "Template Argument Decl not initialized"); return SpecDecl; @@ -122,6 +136,7 @@ } SourceLocation getEndLoc() const LLVM_READONLY { + assert(ArgsAsWritten); // If the ConceptSpecializationExpr is the ImmediatelyDeclaredConstraint // of a TypeConstraint written syntactically as a constrained-parameter, // there may not be a template argument list. 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 @@ -2889,6 +2889,8 @@ "%select{and|because}0 '%1' would be invalid">; def note_nested_requirement_substitution_error : Note< "%select{and|because}0 '%1' would be invalid: %2">; +def note_concept_specialisation_substitution_error : Note< + "%select{and|because}0 '%1'">; 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/ComputeDependence.cpp b/clang/lib/AST/ComputeDependence.cpp --- a/clang/lib/AST/ComputeDependence.cpp +++ b/clang/lib/AST/ComputeDependence.cpp @@ -844,12 +844,13 @@ auto TA = TemplateArgumentDependence::None; const auto InterestingDeps = TemplateArgumentDependence::Instantiation | TemplateArgumentDependence::UnexpandedPack; - for (const TemplateArgumentLoc &ArgLoc : - E->getTemplateArgsAsWritten()->arguments()) { - TA |= ArgLoc.getArgument().getDependence() & InterestingDeps; - if (TA == InterestingDeps) - break; - } + if (E->getTemplateArgsAsWritten()) + for (const TemplateArgumentLoc &ArgLoc : + E->getTemplateArgsAsWritten()->arguments()) { + TA |= ArgLoc.getArgument().getDependence() & InterestingDeps; + if (TA == InterestingDeps) + break; + } ExprDependence D = ValueDependent ? ExprDependence::Value : ExprDependence::None; diff --git a/clang/lib/AST/ExprConcepts.cpp b/clang/lib/AST/ExprConcepts.cpp --- a/clang/lib/AST/ExprConcepts.cpp +++ b/clang/lib/AST/ExprConcepts.cpp @@ -36,14 +36,15 @@ NamedDecl *FoundDecl, ConceptDecl *NamedConcept, const ASTTemplateArgumentListInfo *ArgsAsWritten, ImplicitConceptSpecializationDecl *SpecDecl, - const ConstraintSatisfaction *Satisfaction) + const ConstraintSatisfaction *Satisfaction, bool ArgsHasSubstitutionFailure) : Expr(ConceptSpecializationExprClass, C.BoolTy, VK_PRValue, OK_Ordinary), ConceptReference(NNS, TemplateKWLoc, ConceptNameInfo, FoundDecl, NamedConcept, ArgsAsWritten), SpecDecl(SpecDecl), Satisfaction(Satisfaction ? ASTConstraintSatisfaction::Create(C, *Satisfaction) - : nullptr) { + : nullptr), + ArgsHasSubstitutionFailure(ArgsHasSubstitutionFailure) { setDependence(computeDependence(this, /*ValueDependent=*/!Satisfaction)); // Currently guaranteed by the fact concepts can only be at namespace-scope. @@ -53,6 +54,13 @@ ->containsUnexpandedParameterPack())); assert((!isValueDependent() || isInstantiationDependent()) && "should not be value-dependent"); + if (ArgsHasSubstitutionFailure) { + assert(Satisfaction); + assert(!Satisfaction->IsSatisfied); + assert(!Satisfaction->ContainsErrors); + assert(!ArgsAsWritten); + assert(!SpecDecl); + } } ConceptSpecializationExpr::ConceptSpecializationExpr(EmptyShell Empty) @@ -103,6 +111,17 @@ Dependent, ContainsUnexpandedParameterPack); } +ConceptSpecializationExpr *ConceptSpecializationExpr::CreateSubstitutionFailure( + const ASTContext &C, NestedNameSpecifierLoc NNS, + SourceLocation TemplateKWLoc, DeclarationNameInfo ConceptNameInfo, + NamedDecl *FoundDecl, ConceptDecl *NamedConcept, + const ConstraintSatisfaction *Satisfaction) { + return new (C) ConceptSpecializationExpr( + C, NNS, TemplateKWLoc, ConceptNameInfo, FoundDecl, NamedConcept, + /*ArgsAsWritten=*/nullptr, /*SpecDecl=*/nullptr, Satisfaction, + /*ArgsHasSubstitutionFailure=*/true); +} + const TypeConstraint * concepts::ExprRequirement::ReturnTypeRequirement::getTypeConstraint() const { assert(isTypeConstraint()); 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 @@ -2467,9 +2467,12 @@ if (E->getTemplateKWLoc().isValid()) OS << "template "; OS << E->getFoundDecl()->getName(); - printTemplateArgumentList(OS, E->getTemplateArgsAsWritten()->arguments(), - Policy, - E->getNamedConcept()->getTemplateParameters()); + if (E->hasSubstitutionFailureInArgs()) + OS << "<>"; + else + printTemplateArgumentList(OS, E->getTemplateArgsAsWritten()->arguments(), + Policy, + E->getNamedConcept()->getTemplateParameters()); } void StmtPrinter::VisitRequiresExpr(RequiresExpr *E) { 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 @@ -11,6 +11,7 @@ //===----------------------------------------------------------------------===// #include "TreeTransform.h" +#include "clang/AST/ASTConcept.h" #include "clang/Sema/SemaConcept.h" #include "clang/Sema/Sema.h" #include "clang/Sema/SemaInternal.h" @@ -941,6 +942,17 @@ break; } } else if (auto *CSE = dyn_cast(SubstExpr)) { + if (CSE->hasSubstitutionFailureInArgs()) { + for (const auto &Diags : CSE->getSatisfaction()) { + auto *SubstDiag = + Diags.second + .get(); + S.Diag(SubstDiag->first, + diag::note_concept_specialisation_substitution_error) + << (int)First << SubstDiag->second; + } + return; + } if (CSE->getTemplateArgsAsWritten()->NumTemplateArgs == 1) { S.Diag( CSE->getSourceRange().getBegin(), 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 @@ -15,6 +15,7 @@ #include "CoroutineStmtBuilder.h" #include "TypeLocBuilder.h" +#include "clang/AST/ASTConcept.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/DeclTemplate.h" @@ -37,6 +38,7 @@ #include "clang/Sema/ScopeInfo.h" #include "clang/Sema/SemaDiagnostic.h" #include "clang/Sema/SemaInternal.h" +#include "clang/Sema/TemplateDeduction.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/Support/ErrorHandling.h" #include @@ -3477,6 +3479,23 @@ return Result; } + ExprResult RebuildConceptSpecializationSubstitutionFailure( + ConceptSpecializationExpr *E, + ConstraintSatisfaction::SubstitutionDiagnostic *SubstDiags) { + ConstraintSatisfaction Satisfaction; + Satisfaction.IsSatisfied = false; + Satisfaction.ContainsErrors = false; + Satisfaction.Details.emplace_back(E, SubstDiags); + CXXScopeSpec SS; + SS.Adopt(E->getNestedNameSpecifierLoc()); + return ConceptSpecializationExpr::CreateSubstitutionFailure( + SemaRef.Context, + SS.isSet() ? SS.getWithLocInContext(SemaRef.Context) + : NestedNameSpecifierLoc{}, + E->getTemplateKWLoc(), E->getConceptNameInfo(), E->getFoundDecl(), + E->getNamedConcept(), &Satisfaction); + } + /// \brief Build a new requires expression. /// /// By default, performs semantic analysis to build the new expression. @@ -12611,15 +12630,42 @@ E->getEndLoc()); } -template -ExprResult -TreeTransform::TransformConceptSpecializationExpr( - ConceptSpecializationExpr *E) { +namespace { +// TODO: Use SubstitutionDiagnostic instead of pair to propagate +// SubstitutedEntity. +// TODO: Refactor to use CreateSubstDiag from SemaTemplateInstantiate.cpp. +inline ConstraintSatisfaction::SubstitutionDiagnostic * +CreateSubstDiag(Sema &S, sema::TemplateDeductionInfo &Info) { + SmallString<128> Message; + SourceLocation ErrorLoc; + + PartialDiagnosticAt PDA(SourceLocation(), + PartialDiagnostic::NullDiagnostic{}); + Info.takeSFINAEDiagnostic(PDA); + PDA.second.EmitToString(S.getDiagnostics(), Message); + ErrorLoc = PDA.first; + char *MessageBuf = new (S.Context) char[Message.size()]; + std::copy(Message.begin(), Message.end(), MessageBuf); + return new (S.Context) ConstraintSatisfaction::SubstitutionDiagnostic{ + ErrorLoc, StringRef(MessageBuf, Message.size())}; +} +} // namespace + +template +ExprResult TreeTransform::TransformConceptSpecializationExpr( + ConceptSpecializationExpr *E) { const ASTTemplateArgumentListInfo *Old = E->getTemplateArgsAsWritten(); TemplateArgumentListInfo TransArgs(Old->LAngleLoc, Old->RAngleLoc); - if (getDerived().TransformTemplateArguments(Old->getTemplateArgs(), - Old->NumTemplateArgs, TransArgs)) - return ExprError(); + { + Sema::SFINAETrap Trap(getSema()); + if (getDerived().TransformTemplateArguments( + Old->getTemplateArgs(), Old->NumTemplateArgs, TransArgs)) { + if (!E->isInstantiationDependent() || !Trap.hasErrorOccurred()) + return ExprError(); + return getDerived().RebuildConceptSpecializationSubstitutionFailure( + E, CreateSubstDiag(SemaRef, **SemaRef.isSFINAEContext())); + } + } return getDerived().RebuildConceptSpecializationExpr( E->getNestedNameSpecifierLoc(), E->getTemplateKWLoc(), 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,80 @@ X.next(); }; + +namespace SubstitutionFailureNestedRequires { +template concept True = true; + +struct S { double value; }; + +template +concept Pipes = requires (T x) { + requires True || True; + requires True || True; +}; + +template +concept Amps1 = requires (T x) { + requires True && True; + // expected-note@-1{{because 'member reference base type 'int' is not a structure or union'}} +}; +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) { // expected-note {{candidate template ignored: constraints not satisfied [with T = int]}} + requires + True // expected-note {{because 'member reference base type 'int' is not a structure or union'}} + && True; +} {} +template void fooPipes() requires Pipes {} +template void fooAmps1() requires Amps1 {} +// expected-note@-1 {{candidate template ignored: constraints not satisfied [with T = int]}} \ +// expected-note@-1 {{because 'int' does not satisfy 'Amps1'}} + +void foo() { + foo1(); + foo1(); // expected-error {{no matching function for call to 'foo1'}} + fooPipes(); + fooPipes(); + fooAmps1(); + fooAmps1(); // expected-error {{no matching function for call to 'fooAmps1'}} +} + +template +concept HasNoValue = requires (T x) { + requires !True && True; +}; +static_assert(HasNoValue); +static_assert(!HasNoValue); + +template constexpr bool NotAConceptTrue = true; +template +concept SFinNestedRequires = requires (T x) { + // Only SF in a concept specialisation should be evaluated to false. + // Rest is still a SF. + requires NotAConceptTrue || NotAConceptTrue; + // expected-note@-1 {{because 'NotAConceptTrue || NotAConceptTrue' would be invalid: member reference base type 'int' is not a structure or union}} +}; +static_assert(!SFinNestedRequires); // Unsatisfied but not a hard error. +static_assert(SFinNestedRequires); +template +void foo() requires SFinNestedRequires {} +// expected-note@-1 {{candidate template ignored: constraints not satisfied [with T = int]}} +// expected-note@-2 {{because 'int' does not satisfy 'SFinNestedRequires'}} +void bar() { + foo(); // expected-error {{no matching function for call to 'foo'}} + foo(); +} +}