https://github.com/cor3ntin created https://github.com/llvm/llvm-project/pull/103867
Per [basic.scope], the locus of a concept is immediately after the introduction of its name. This let us provide better diagnostics for attempt to define recursive concepts. Note that recursive concepts are not supported per https://eel.is/c++draft/basic#scope.pdecl-note-3, but there is no normative wording for that restriction. This is a known defect introduced by [p1787r6](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p1787r6.html). Fixes #55875 >From 021ffdd7597c6ca480de273db5604cb482306d50 Mon Sep 17 00:00:00 2001 From: Corentin Jabot <corentinja...@gmail.com> Date: Wed, 14 Aug 2024 12:06:03 +0200 Subject: [PATCH] [Clang] Adjust concept definition locus Per [basic.scope], the locus of a concept is immediately adfter the introduction of its name. This let us provide better diagnostics for attempt to define recursive concepts. Note that recursive concepts are not supported per https://eel.is/c++draft/basic#scope.pdecl-note-3, but there is no normative wording for that restriction. This is a known defect introduced by [p1787r6](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p1787r6.html). Fixes #55875 --- clang/docs/ReleaseNotes.rst | 4 +- clang/include/clang/AST/DeclTemplate.h | 13 ++- .../clang/Basic/DiagnosticSemaKinds.td | 2 + clang/include/clang/Sema/Sema.h | 13 +-- clang/lib/Parse/ParseTemplate.cpp | 14 +++- clang/lib/Sema/SemaExpr.cpp | 4 + clang/lib/Sema/SemaTemplate.cpp | 83 +++++++++++++++---- clang/test/CXX/drs/cwg25xx.cpp | 10 ++- clang/test/SemaTemplate/concepts.cpp | 9 +- 9 files changed, 118 insertions(+), 34 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 39e1b0fcb09bbd..1786a70c82cd20 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 5b6a6b40b28ef8..687715a22e9fd3 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<ConceptDecl> { : 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 554dbaff2ce0d8..28fc3a69865b11 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 25cb6c8fbf6104..aace4ce351eecb 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 a5130f56600e54..6ecfc15757f3d4 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 8defc8e1c185c0..a084708918fdb3 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -306,6 +306,10 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl *D, ArrayRef<SourceLocation> Locs, } + if (auto *Concept = dyn_cast<ConceptDecl>(D); + Concept && CheckConceptUseIndefinition(Concept, Loc)) + return true; + if (auto *MD = dyn_cast<CXXMethodDecl>(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 876921a6b311d4..496ddf07b0e686 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 1c0d32fe3fdfce..0d9f5eac23866a 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<typename T> 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<int>); @@ -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<typename T> - 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 a4b42cad79abd4..a98ca3939222bd 100644 --- a/clang/test/SemaTemplate/concepts.cpp +++ b/clang/test/SemaTemplate/concepts.cpp @@ -1006,7 +1006,14 @@ template<class> concept Irrelevant = false; template <typename T> -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<typename T> concept C1 = C1<T> && []<C1>(C1 auto) -> C1 auto {}; +//expected-error@-1 4{{a concept definition cannot refer to itself}} \ +//expected-note@-1 4{{declared here}} template<class T> void aaa(T t) // expected-note {{candidate template ignored: constraints not satisfied}} requires (False<T> || False<T>) || False<T> {} // expected-note 3 {{'int' does not satisfy 'False'}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
