[Clang][Parser] Don't evaluate concept when its definition is invalid

[zyn0217]

Since #103867, the nullness of the concept declaration has been turned to represent a state in which the concept definition is being parsed and used for self-reference checking.

However, PR missed a case where such a definition could be invalid, and we shall inhibit them from making it into evaluation.

Fixes #109780

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

Since #103867, the nullness of the concept declaration has been turned to represent a state in which the concept definition is being parsed and used for self-reference checking.

However, that PR missed a case where such a definition could be invalid and thus, the definition should not be found. Otherwise, the assumption in the evaluation would fail. This PR fixes it by removing the definition from the DeclContext in such situations.

Fixes #109780

---

Full diff: https://github.com/llvm/llvm-project/pull/111179.diff

4 Files Affected:

• (modified) clang/include/clang/Sema/Sema.h (+1-1)
• (modified) clang/lib/Parse/ParseTemplate.cpp (+9-1)
• (modified) clang/lib/Sema/SemaTemplate.cpp (+4-2)
• (modified) clang/test/SemaTemplate/concepts.cpp (+14)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index bede971ce0191b..eea3cf4e43cde8 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11995,7 +11995,7 @@ class Sema final : public SemaBase {
 
   ConceptDecl *ActOnStartConceptDefinition(
       Scope *S, MultiTemplateParamsArg TemplateParameterLists,
-      const IdentifierInfo *Name, SourceLocation NameLoc);
+      const IdentifierInfo *Name, SourceLocation NameLoc, bool &AddedToScope);
 
   ConceptDecl *ActOnFinishConceptDefinition(Scope *S, ConceptDecl *C,
                                             Expr *ConstraintExpr,
diff --git a/clang/lib/Parse/ParseTemplate.cpp b/clang/lib/Parse/ParseTemplate.cpp
index de29652abbfd95..9ddabdf5172e41 100644
--- a/clang/lib/Parse/ParseTemplate.cpp
+++ b/clang/lib/Parse/ParseTemplate.cpp
@@ -322,13 +322,19 @@ Parser::ParseConceptDefinition(const ParsedTemplateInfo &TemplateInfo,
 
   // [C++26][basic.scope.pdecl]/p13
   // The locus of a concept-definition is immediately after its concept-name.
+  bool AddedToScope = false;
   ConceptDecl *D = Actions.ActOnStartConceptDefinition(
-      getCurScope(), *TemplateInfo.TemplateParams, Id, IdLoc);
+      getCurScope(), *TemplateInfo.TemplateParams, Id, IdLoc, AddedToScope);
 
   ParsedAttributes Attrs(AttrFactory);
   MaybeParseAttributes(PAKM_GNU | PAKM_CXX11, Attrs);
 
   if (!TryConsumeToken(tok::equal)) {
+    // The expression is unset until ActOnFinishConceptDefinition(), so remove
+    // the invalid declaration from the future lookup such that the evaluation
+    // wouldn't have to handle empty expressions.
+    if (AddedToScope)
+      Actions.CurContext->removeDecl(D);
     Diag(Tok.getLocation(), diag::err_expected) << tok::equal;
     SkipUntil(tok::semi);
     return nullptr;
@@ -337,6 +343,8 @@ Parser::ParseConceptDefinition(const ParsedTemplateInfo &TemplateInfo,
   ExprResult ConstraintExprResult =
       Actions.CorrectDelayedTyposInExpr(ParseConstraintExpression());
   if (ConstraintExprResult.isInvalid()) {
+    if (AddedToScope)
+      Actions.CurContext->removeDecl(D);
     SkipUntil(tok::semi);
     return nullptr;
   }
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index eeaa1ebd7ba83a..2d8b47ea2474be 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -8632,7 +8632,7 @@ Decl *Sema::ActOnTemplateDeclarator(Scope *S,
 
 ConceptDecl *Sema::ActOnStartConceptDefinition(
     Scope *S, MultiTemplateParamsArg TemplateParameterLists,
-    const IdentifierInfo *Name, SourceLocation NameLoc) {
+    const IdentifierInfo *Name, SourceLocation NameLoc, bool &AddedToScope) {
   DeclContext *DC = CurContext;
 
   if (!DC->getRedeclContext()->isFileContext()) {
@@ -8688,8 +8688,10 @@ ConceptDecl *Sema::ActOnStartConceptDefinition(
   // 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())
+  if (Previous.empty()) {
     PushOnScopeChains(NewDecl, S, true);
+    AddedToScope = true;
+  }
 
   return NewDecl;
 }
diff --git a/clang/test/SemaTemplate/concepts.cpp b/clang/test/SemaTemplate/concepts.cpp
index a98ca3939222bd..9d29cc59d3ab96 100644
--- a/clang/test/SemaTemplate/concepts.cpp
+++ b/clang/test/SemaTemplate/concepts.cpp
@@ -1151,3 +1151,17 @@ int test() {
 }
 
 }
+
+namespace GH109780 {
+
+template <typename T>
+concept Concept; // expected-error {{expected '='}}
+
+bool val = Concept<int>; // expected-error {{use of undeclared identifier 'Concept'}}
+
+template <typename T>
+concept C = invalid; // expected-error {{use of undeclared identifier 'invalid'}}
+
+bool val2 = C<int>; // expected-error {{use of undeclared identifier 'C'}}
+
+} // namespace GH109780

[cor3ntin]

LGTM

[shafik]

This crash looks linked to this change: #115004

[shafik]

This crash looks linked to this change: #121980