[clang] [Clang][Parser] Remove the concept from the DeclContext if the definition is invalid (PR #111179)

[Younan Zhang via cfe-commits]

https://github.com/zyn0217 created https://github.com/llvm/llvm-project/pull/111179

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 https://github.com/llvm/llvm-project/issues/109780

>From fc3e60d2c1d038ae3f3ef2e4ec3e86605cfb0f69 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Fri, 4 Oct 2024 23:42:30 +0800
Subject: [PATCH] [Clang][Parser] Remove the concept from the DeclContext if
 the definition is invalid

---
 clang/include/clang/Sema/Sema.h      |  2 +-
 clang/lib/Parse/ParseTemplate.cpp    | 10 +++++++++-
 clang/lib/Sema/SemaTemplate.cpp      |  6 ++++--
 clang/test/SemaTemplate/concepts.cpp | 14 ++++++++++++++
 4 files changed, 28 insertions(+), 4 deletions(-)

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