[clang] [Clang] Adjust concept definition locus (PR #103867)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 14 05:25:35 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: cor3ntin (cor3ntin)
<details>
<summary>Changes</summary>
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
---
Full diff: https://github.com/llvm/llvm-project/pull/103867.diff
9 Files Affected:
- (modified) clang/docs/ReleaseNotes.rst (+3-1)
- (modified) clang/include/clang/AST/DeclTemplate.h (+9-4)
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2)
- (modified) clang/include/clang/Sema/Sema.h (+8-5)
- (modified) clang/lib/Parse/ParseTemplate.cpp (+11-3)
- (modified) clang/lib/Sema/SemaExpr.cpp (+4)
- (modified) clang/lib/Sema/SemaTemplate.cpp (+66-17)
- (modified) clang/test/CXX/drs/cwg25xx.cpp (+7-3)
- (modified) clang/test/SemaTemplate/concepts.cpp (+8-1)
``````````diff
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 at -1 {{unknown type name 'ErrorRequires'}}
+ // since-cxx20-error at -1 {{a concept definition cannot refer to itself}} \
+ // since-cxx20-error at -1 {{'auto' not allowed in requires expression parameter}} \
+ // since-cxx20-note at -1 {{declared here}}
x;
};
static_assert(ErrorRequires<int>);
@@ -209,9 +211,11 @@ namespace cwg2565 { // cwg2565: 16 open 2023-06-07
// since-cxx20-note at -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 at -1 {{declared here}}
requires requires (NestedErrorInRequires auto y) {
- // since-cxx20-error at -1 {{unknown type name 'NestedErrorInRequires'}}
+ // since-cxx20-error at -1 {{a concept definition cannot refer to itself}} \
+ // since-cxx20-error at -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 at -1 {{a concept definition cannot refer to itself}} \
+// expected-error at -1 {{'auto' not allowed in requires expression parameter}} \
+// expected-note at -1 {{declared here}}
+
+template<typename T> concept C1 = C1<T> && []<C1>(C1 auto) -> C1 auto {};
+//expected-error at -1 4{{a concept definition cannot refer to itself}} \
+//expected-note at -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'}}
``````````
</details>
https://github.com/llvm/llvm-project/pull/103867
More information about the cfe-commits
mailing list