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

Younan Zhang via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 7 05:48:15 PDT 2024


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

>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 1/3] [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

>From 3c2edaf8be00adf87808b24d1d1e66adbada446b Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Mon, 7 Oct 2024 20:42:12 +0800
Subject: [PATCH 2/3] Revert "[Clang][Parser] Remove the concept from the
 DeclContext if the definition is invalid"

This reverts commit fc3e60d2c1d038ae3f3ef2e4ec3e86605cfb0f69.
---
 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, 4 insertions(+), 28 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index eea3cf4e43cde8..bede971ce0191b 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, bool &AddedToScope);
+      const IdentifierInfo *Name, SourceLocation NameLoc);
 
   ConceptDecl *ActOnFinishConceptDefinition(Scope *S, ConceptDecl *C,
                                             Expr *ConstraintExpr,
diff --git a/clang/lib/Parse/ParseTemplate.cpp b/clang/lib/Parse/ParseTemplate.cpp
index 9ddabdf5172e41..de29652abbfd95 100644
--- a/clang/lib/Parse/ParseTemplate.cpp
+++ b/clang/lib/Parse/ParseTemplate.cpp
@@ -322,19 +322,13 @@ 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, AddedToScope);
+      getCurScope(), *TemplateInfo.TemplateParams, Id, IdLoc);
 
   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;
@@ -343,8 +337,6 @@ 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 2d8b47ea2474be..eeaa1ebd7ba83a 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, bool &AddedToScope) {
+    const IdentifierInfo *Name, SourceLocation NameLoc) {
   DeclContext *DC = CurContext;
 
   if (!DC->getRedeclContext()->isFileContext()) {
@@ -8688,10 +8688,8 @@ 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 9d29cc59d3ab96..a98ca3939222bd 100644
--- a/clang/test/SemaTemplate/concepts.cpp
+++ b/clang/test/SemaTemplate/concepts.cpp
@@ -1151,17 +1151,3 @@ 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

>From 3db6376f7fa9f42ad8e2f2fc164c473eda13f382 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Mon, 7 Oct 2024 20:46:49 +0800
Subject: [PATCH 3/3] Invalidate declarations

---
 clang/lib/Parse/ParseTemplate.cpp    |  4 ++++
 clang/test/SemaTemplate/concepts.cpp | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/clang/lib/Parse/ParseTemplate.cpp b/clang/lib/Parse/ParseTemplate.cpp
index de29652abbfd95..0953cfc3c017ef 100644
--- a/clang/lib/Parse/ParseTemplate.cpp
+++ b/clang/lib/Parse/ParseTemplate.cpp
@@ -331,6 +331,8 @@ Parser::ParseConceptDefinition(const ParsedTemplateInfo &TemplateInfo,
   if (!TryConsumeToken(tok::equal)) {
     Diag(Tok.getLocation(), diag::err_expected) << tok::equal;
     SkipUntil(tok::semi);
+    if (D)
+      D->setInvalidDecl();
     return nullptr;
   }
 
@@ -338,6 +340,8 @@ Parser::ParseConceptDefinition(const ParsedTemplateInfo &TemplateInfo,
       Actions.CorrectDelayedTyposInExpr(ParseConstraintExpression());
   if (ConstraintExprResult.isInvalid()) {
     SkipUntil(tok::semi);
+    if (D)
+      D->setInvalidDecl();
     return nullptr;
   }
 
diff --git a/clang/test/SemaTemplate/concepts.cpp b/clang/test/SemaTemplate/concepts.cpp
index a98ca3939222bd..312469313fc535 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>;
+
+template <typename T>
+concept C = invalid; // expected-error {{use of undeclared identifier 'invalid'}}
+
+bool val2 = C<int>;
+
+} // namespace GH109780



More information about the cfe-commits mailing list