diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 39e1b0fcb09bbd5..1786a70c82cd206 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -217,8 +217,10 @@ Bug Fixes to C++ Support - Clang now preserves the unexpanded flag in a lambda transform used for pack expansion. (#GH56852), (#GH85667), (#GH99877). - Fixed a bug when diagnosing ambiguous explicit specializations of constrained member functions. -- Fixed an assertion failure when selecting a function from an overload set that includes a +- Fixed an assertion failure when selecting a function from an overload set that includes a specialization of a conversion function template. +- Correctly diagnose attempts to use a concept name in its own definition; + A concept name is introduced to its scope sooner to match the C++ standard. (#GH55875) Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h index 5b6a6b40b28ef8f..687715a22e9fd31 100644 --- a/clang/include/clang/AST/DeclTemplate.h +++ b/clang/include/clang/AST/DeclTemplate.h @@ -3146,19 +3146,24 @@ class ConceptDecl : public TemplateDecl, public Mergeable { : TemplateDecl(Concept, DC, L, Name, Params), ConstraintExpr(ConstraintExpr) {}; public: - static ConceptDecl *Create(ASTContext &C, DeclContext *DC, - SourceLocation L, DeclarationName Name, + static ConceptDecl *Create(ASTContext &C, DeclContext *DC, SourceLocation L, + DeclarationName Name, TemplateParameterList *Params, - Expr *ConstraintExpr); + Expr *ConstraintExpr = nullptr); static ConceptDecl *CreateDeserialized(ASTContext &C, GlobalDeclID ID); Expr *getConstraintExpr() const { return ConstraintExpr; } + bool hasDefinition() const { return ConstraintExpr != nullptr; } + + void setDefinition(Expr *E) { ConstraintExpr = E; } + SourceRange getSourceRange() const override LLVM_READONLY { return SourceRange(getTemplateParameters()->getTemplateLoc(), - ConstraintExpr->getEndLoc()); + ConstraintExpr ? ConstraintExpr->getEndLoc() + : SourceLocation()); } bool isTypeConcept() const { diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 554dbaff2ce0d87..28fc3a69865b111 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3012,6 +3012,8 @@ def err_concept_no_parameters : Error< "specialization of concepts is not allowed">; def err_concept_extra_headers : Error< "extraneous template parameter list in concept definition">; +def err_recursive_concept : Error< + "a concept definition cannot refer to itself">; def err_concept_no_associated_constraints : Error< "concept cannot have associated constraints">; def err_non_constant_constraint_expression : Error< diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 25cb6c8fbf61041..a025ff6fc13f360 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -12033,14 +12033,17 @@ class Sema final : public SemaBase { void CheckDeductionGuideTemplate(FunctionTemplateDecl *TD); - Decl *ActOnConceptDefinition(Scope *S, - MultiTemplateParamsArg TemplateParameterLists, - const IdentifierInfo *Name, - SourceLocation NameLoc, Expr *ConstraintExpr, - const ParsedAttributesView &Attrs); + ConceptDecl *ActOnStartConceptDefinition( + Scope *S, MultiTemplateParamsArg TemplateParameterLists, + const IdentifierInfo *Name, SourceLocation NameLoc); + + ConceptDecl *ActOnFinishConceptDefinition(Scope *S, ConceptDecl *C, + Expr *ConstraintExpr, + const ParsedAttributesView &Attrs); void CheckConceptRedefinition(ConceptDecl *NewDecl, LookupResult &Previous, bool &AddToScope); + bool CheckConceptUseInDefinition(ConceptDecl *Concept, SourceLocation Loc); TypeResult ActOnDependentTag(Scope *S, unsigned TagSpec, TagUseKind TUK, const CXXScopeSpec &SS, diff --git a/clang/lib/Parse/ParseTemplate.cpp b/clang/lib/Parse/ParseTemplate.cpp index a5130f56600e54b..6ecfc15757f3d42 100644 --- a/clang/lib/Parse/ParseTemplate.cpp +++ b/clang/lib/Parse/ParseTemplate.cpp @@ -320,6 +320,11 @@ Parser::ParseConceptDefinition(const ParsedTemplateInfo &TemplateInfo, const IdentifierInfo *Id = Result.Identifier; SourceLocation IdLoc = Result.getBeginLoc(); + // [C++26][basic.scope.pdecl]/p13 + // The locus of a concept-definition is immediately after its concept-name. + ConceptDecl *D = Actions.ActOnStartConceptDefinition( + getCurScope(), *TemplateInfo.TemplateParams, Id, IdLoc); + ParsedAttributes Attrs(AttrFactory); MaybeParseAttributes(PAKM_GNU | PAKM_CXX11, Attrs); @@ -339,9 +344,12 @@ Parser::ParseConceptDefinition(const ParsedTemplateInfo &TemplateInfo, DeclEnd = Tok.getLocation(); ExpectAndConsumeSemi(diag::err_expected_semi_declaration); Expr *ConstraintExpr = ConstraintExprResult.get(); - return Actions.ActOnConceptDefinition(getCurScope(), - *TemplateInfo.TemplateParams, Id, IdLoc, - ConstraintExpr, Attrs); + + if (!D) + return nullptr; + + return Actions.ActOnFinishConceptDefinition(getCurScope(), D, ConstraintExpr, + Attrs); } /// ParseTemplateParameters - Parses a template-parameter-list enclosed in diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 8defc8e1c185c05..0f58eb2840211de 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -306,6 +306,10 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef Locs, } + if (auto *Concept = dyn_cast(D); + Concept && CheckConceptUseInDefinition(Concept, Loc)) + return true; + if (auto *MD = dyn_cast(D)) { // Lambdas are only default-constructible or assignable in C++2a onwards. if (MD->getParent()->isLambda() && diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index 876921a6b311d4d..25585f683752ac4 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -1079,6 +1079,9 @@ bool Sema::CheckTypeConstraint(TemplateIdAnnotation *TypeConstr) { return true; } + if (CheckConceptUseInDefinition(CD, TypeConstr->TemplateNameLoc)) + return true; + bool WereArgsSpecified = TypeConstr->LAngleLoc.isValid(); if (!WereArgsSpecified && @@ -8447,10 +8450,9 @@ Decl *Sema::ActOnTemplateDeclarator(Scope *S, return NewDecl; } -Decl *Sema::ActOnConceptDefinition( +ConceptDecl *Sema::ActOnStartConceptDefinition( Scope *S, MultiTemplateParamsArg TemplateParameterLists, - const IdentifierInfo *Name, SourceLocation NameLoc, Expr *ConstraintExpr, - const ParsedAttributesView &Attrs) { + const IdentifierInfo *Name, SourceLocation NameLoc) { DeclContext *DC = CurContext; if (!DC->getRedeclContext()->isFileContext()) { @@ -8486,11 +8488,8 @@ Decl *Sema::ActOnConceptDefinition( } } - if (DiagnoseUnexpandedParameterPack(ConstraintExpr)) - return nullptr; - ConceptDecl *NewDecl = - ConceptDecl::Create(Context, DC, NameLoc, Name, Params, ConstraintExpr); + ConceptDecl::Create(Context, DC, NameLoc, Name, Params); if (NewDecl->hasAssociatedConstraints()) { // C++2a [temp.concept]p4: @@ -8499,23 +8498,63 @@ Decl *Sema::ActOnConceptDefinition( NewDecl->setInvalidDecl(); } + DeclarationNameInfo NameInfo(NewDecl->getDeclName(), NewDecl->getBeginLoc()); + LookupResult Previous(*this, NameInfo, LookupOrdinaryName, + forRedeclarationInCurContext()); + LookupName(Previous, S); + FilterLookupForScope(Previous, CurContext, S, /*ConsiderLinkage=*/false, + /*AllowInlineNamespace*/ false); + + // We cannot properly handle redeclarations until we parse the constraint + // expression, so only inject the name if we are sure we are not redeclaring a + // symbol + if (Previous.empty()) + PushOnScopeChains(NewDecl, S, true); + + return NewDecl; +} + +static bool RemoveLookupResult(LookupResult &R, NamedDecl *C) { + bool Found = false; + LookupResult::Filter F = R.makeFilter(); + while (F.hasNext()) { + NamedDecl *D = F.next(); + if (D == C) { + F.erase(); + Found = true; + break; + } + } + F.done(); + return Found; +} + +ConceptDecl * +Sema::ActOnFinishConceptDefinition(Scope *S, ConceptDecl *C, + Expr *ConstraintExpr, + const ParsedAttributesView &Attrs) { + assert(!C->hasDefinition() && "Concept already defined"); + if (DiagnoseUnexpandedParameterPack(ConstraintExpr)) + return nullptr; + C->setDefinition(ConstraintExpr); + ProcessDeclAttributeList(S, C, Attrs); + // Check for conflicting previous declaration. - DeclarationNameInfo NameInfo(NewDecl->getDeclName(), NameLoc); + DeclarationNameInfo NameInfo(C->getDeclName(), C->getBeginLoc()); LookupResult Previous(*this, NameInfo, LookupOrdinaryName, forRedeclarationInCurContext()); LookupName(Previous, S); - FilterLookupForScope(Previous, DC, S, /*ConsiderLinkage=*/false, - /*AllowInlineNamespace*/false); + FilterLookupForScope(Previous, CurContext, S, /*ConsiderLinkage=*/false, + /*AllowInlineNamespace*/ false); + bool WasAlreadyAdded = RemoveLookupResult(Previous, C); bool AddToScope = true; - CheckConceptRedefinition(NewDecl, Previous, AddToScope); + CheckConceptRedefinition(C, Previous, AddToScope); - ActOnDocumentableDecl(NewDecl); - if (AddToScope) - PushOnScopeChains(NewDecl, S); - - ProcessDeclAttributeList(S, NewDecl, Attrs); + ActOnDocumentableDecl(C); + if (!WasAlreadyAdded && AddToScope) + PushOnScopeChains(C, S); - return NewDecl; + return C; } void Sema::CheckConceptRedefinition(ConceptDecl *NewDecl, @@ -8560,6 +8599,16 @@ void Sema::CheckConceptRedefinition(ConceptDecl *NewDecl, Context.setPrimaryMergedDecl(NewDecl, OldConcept->getCanonicalDecl()); } +bool Sema::CheckConceptUseInDefinition(ConceptDecl *Concept, + SourceLocation Loc) { + if (!Concept->isInvalidDecl() && !Concept->hasDefinition()) { + Diag(Loc, diag::err_recursive_concept) << Concept; + Diag(Concept->getLocation(), diag::note_declared_at); + return true; + } + return false; +} + /// \brief Strips various properties off an implicit instantiation /// that has just been explicitly specialized. static void StripImplicitInstantiation(NamedDecl *D, bool MinGW) { diff --git a/clang/test/CXX/drs/cwg25xx.cpp b/clang/test/CXX/drs/cwg25xx.cpp index 1c0d32fe3fdfce9..0d9f5eac23866a2 100644 --- a/clang/test/CXX/drs/cwg25xx.cpp +++ b/clang/test/CXX/drs/cwg25xx.cpp @@ -201,7 +201,9 @@ namespace cwg2565 { // cwg2565: 16 open 2023-06-07 template concept ErrorRequires = requires (ErrorRequires auto x) { - // since-cxx20-error@-1 {{unknown type name 'ErrorRequires'}} + // since-cxx20-error@-1 {{a concept definition cannot refer to itself}} \ + // since-cxx20-error@-1 {{'auto' not allowed in requires expression parameter}} \ + // since-cxx20-note@-1 {{declared here}} x; }; static_assert(ErrorRequires); @@ -209,9 +211,11 @@ namespace cwg2565 { // cwg2565: 16 open 2023-06-07 // since-cxx20-note@-2 {{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}} template - concept NestedErrorInRequires = requires (T x) { + concept NestedErrorInRequires = requires (T x) { // + // since-cxx20-note@-1 {{declared here}} requires requires (NestedErrorInRequires auto y) { - // since-cxx20-error@-1 {{unknown type name 'NestedErrorInRequires'}} + // since-cxx20-error@-1 {{a concept definition cannot refer to itself}} \ + // since-cxx20-error@-1 {{'auto' not allowed in requires expression parameter}} y; }; }; diff --git a/clang/test/SemaTemplate/concepts.cpp b/clang/test/SemaTemplate/concepts.cpp index a4b42cad79abd49..a98ca3939222bda 100644 --- a/clang/test/SemaTemplate/concepts.cpp +++ b/clang/test/SemaTemplate/concepts.cpp @@ -1006,7 +1006,14 @@ template concept Irrelevant = false; template -concept ErrorRequires = requires(ErrorRequires auto x) { x; }; // expected-error {{unknown type name 'ErrorRequires'}} +concept ErrorRequires = requires(ErrorRequires auto x) { x; }; +// expected-error@-1 {{a concept definition cannot refer to itself}} \ +// expected-error@-1 {{'auto' not allowed in requires expression parameter}} \ +// expected-note@-1 {{declared here}} + +template concept C1 = C1 && [](C1 auto) -> C1 auto {}; +//expected-error@-1 4{{a concept definition cannot refer to itself}} \ +//expected-note@-1 4{{declared here}} template void aaa(T t) // expected-note {{candidate template ignored: constraints not satisfied}} requires (False || False) || False {} // expected-note 3 {{'int' does not satisfy 'False'}}