[clang] [Clang] [Parser] Fixing all callers of `ParseExternalDeclaration` that didn't parse gnu attributes (PR #117148)

Mathys Gasnier via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 21 08:28:28 PST 2024


https://github.com/Mathys-Gasnier updated https://github.com/llvm/llvm-project/pull/117148

>From 086b2e21da4febd31789fe717bae526af625627e Mon Sep 17 00:00:00 2001
From: Mathys-Gasnier <mathys35.gasnier at gmail.com>
Date: Thu, 21 Nov 2024 11:08:41 +0100
Subject: [PATCH 1/2] [Clang] [Parser] Fixing all callers of
 `ParseExternalDeclaration` that didn't parse gnu attributes

---
 clang/lib/Parse/ParseDeclCXX.cpp     | 24 +++++++++++++++---------
 clang/lib/Parse/ParseObjc.cpp        |  8 +++++---
 clang/lib/Parse/ParseOpenMP.cpp      |  8 +++++---
 clang/lib/Parse/Parser.cpp           |  8 +++++---
 clang/test/Parser/cxx-attributes.cpp |  3 +++
 5 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 010ac9c1a3e3a9..4f050d6632c356 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -267,9 +267,11 @@ void Parser::ParseInnerNamespace(const InnerNamespaceInfoList &InnerNSs,
     while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) &&
            Tok.isNot(tok::eof)) {
       ParsedAttributes DeclAttrs(AttrFactory);
-      MaybeParseCXX11Attributes(DeclAttrs);
-      ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
-      ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs);
+      ParsedAttributes DeclSpecAttrs(AttrFactory);
+      while (MaybeParseCXX11Attributes(DeclAttrs) ||
+              MaybeParseGNUAttributes(DeclSpecAttrs))
+        ;
+      ParseExternalDeclaration(DeclAttrs, DeclSpecAttrs);
     }
 
     // The caller is what called check -- we are simply calling
@@ -468,9 +470,11 @@ Decl *Parser::ParseExportDeclaration() {
   if (Tok.isNot(tok::l_brace)) {
     // FIXME: Factor out a ParseExternalDeclarationWithAttrs.
     ParsedAttributes DeclAttrs(AttrFactory);
-    MaybeParseCXX11Attributes(DeclAttrs);
-    ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
-    ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs);
+    ParsedAttributes DeclSpecAttrs(AttrFactory);
+    while (MaybeParseCXX11Attributes(DeclAttrs) ||
+            MaybeParseGNUAttributes(DeclSpecAttrs))
+      ;
+    ParseExternalDeclaration(DeclAttrs, DeclSpecAttrs);
     return Actions.ActOnFinishExportDecl(getCurScope(), ExportDecl,
                                          SourceLocation());
   }
@@ -481,9 +485,11 @@ Decl *Parser::ParseExportDeclaration() {
   while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) &&
          Tok.isNot(tok::eof)) {
     ParsedAttributes DeclAttrs(AttrFactory);
-    MaybeParseCXX11Attributes(DeclAttrs);
-    ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
-    ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs);
+    ParsedAttributes DeclSpecAttrs(AttrFactory);
+    while (MaybeParseCXX11Attributes(DeclAttrs) ||
+            MaybeParseGNUAttributes(DeclSpecAttrs))
+      ;
+    ParseExternalDeclaration(DeclAttrs, DeclSpecAttrs);
   }
 
   T.consumeClose();
diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp
index bcbf4dfbabafa8..ec0572d58b3552 100644
--- a/clang/lib/Parse/ParseObjc.cpp
+++ b/clang/lib/Parse/ParseObjc.cpp
@@ -2287,10 +2287,12 @@ Parser::ParseObjCAtImplementationDeclaration(SourceLocation AtLoc,
     ObjCImplParsingDataRAII ObjCImplParsing(*this, ObjCImpDecl);
     while (!ObjCImplParsing.isFinished() && !isEofOrEom()) {
       ParsedAttributes DeclAttrs(AttrFactory);
-      MaybeParseCXX11Attributes(DeclAttrs);
-      ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
+      ParsedAttributes DeclSpecAttrs(AttrFactory);
+      while (MaybeParseCXX11Attributes(DeclAttrs) ||
+              MaybeParseGNUAttributes(DeclSpecAttrs))
+        ;
       if (DeclGroupPtrTy DGP =
-              ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs)) {
+              ParseExternalDeclaration(DeclAttrs, DeclSpecAttrs)) {
         DeclGroupRef DG = DGP.get();
         DeclsInGroup.append(DG.begin(), DG.end());
       }
diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index b91928063169ee..debfc7bd898864 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2314,10 +2314,12 @@ Parser::DeclGroupPtrTy Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(
       // Here we expect to see some function declaration.
       if (AS == AS_none) {
         assert(TagType == DeclSpec::TST_unspecified);
-        ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
-        MaybeParseCXX11Attributes(Attrs);
+        ParsedAttributes DeclSpecAttrs(AttrFactory);
+        while (MaybeParseCXX11Attributes(Attrs) ||
+                MaybeParseGNUAttributes(DeclSpecAttrs))
+          ;
         ParsingDeclSpec PDS(*this);
-        Ptr = ParseExternalDeclaration(Attrs, EmptyDeclSpecAttrs, &PDS);
+        Ptr = ParseExternalDeclaration(Attrs, DeclSpecAttrs, &PDS);
       } else {
         Ptr =
             ParseCXXClassMemberDeclarationWithPragmas(AS, Attrs, TagType, Tag);
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index 36e56a92c3092e..54246510cc2453 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -2432,9 +2432,11 @@ void Parser::ParseMicrosoftIfExistsExternalDeclaration() {
   // FIXME: Support module import within __if_exists?
   while (Tok.isNot(tok::r_brace) && !isEofOrEom()) {
     ParsedAttributes Attrs(AttrFactory);
-    MaybeParseCXX11Attributes(Attrs);
-    ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
-    DeclGroupPtrTy Result = ParseExternalDeclaration(Attrs, EmptyDeclSpecAttrs);
+    ParsedAttributes DeclSpecAttrs(AttrFactory);
+    while (MaybeParseCXX11Attributes(Attrs) ||
+            MaybeParseGNUAttributes(DeclSpecAttrs))
+      ;
+    DeclGroupPtrTy Result = ParseExternalDeclaration(Attrs, DeclSpecAttrs);
     if (Result && !getCurScope()->getParent())
       Actions.getASTConsumer().HandleTopLevelDecl(Result.get());
   }
diff --git a/clang/test/Parser/cxx-attributes.cpp b/clang/test/Parser/cxx-attributes.cpp
index 51d1f9c8228121..efc4c562653140 100644
--- a/clang/test/Parser/cxx-attributes.cpp
+++ b/clang/test/Parser/cxx-attributes.cpp
@@ -25,6 +25,9 @@ namespace PR17666 {
 
   typedef int __attribute__((aligned(int(1)))) T1;
   typedef int __attribute__((aligned(int))) T2; // expected-error {{expected '(' for function-style cast}}
+
+  class C;
+  __attribute__((attr)) [[nodiscard]] C f(); // expected-warning{{unknown attribute 'attr' ignored}}
 }
 
 __attribute((typename)) int x; // expected-warning {{unknown attribute 'typename' ignored}}

>From 80f779477dcd4a37935da51662b29f8c7a10e151 Mon Sep 17 00:00:00 2001
From: Mathys-Gasnier <mathys35.gasnier at gmail.com>
Date: Thu, 21 Nov 2024 17:28:15 +0100
Subject: [PATCH 2/2] Factoring out attributes and external declaration parsing
 into a function

---
 clang/include/clang/Parse/Parser.h |  2 ++
 clang/lib/Parse/ParseDeclCXX.cpp   | 29 ++++-------------------------
 clang/lib/Parse/ParseObjc.cpp      |  8 +-------
 clang/lib/Parse/Parser.cpp         | 30 +++++++++++++-----------------
 4 files changed, 20 insertions(+), 49 deletions(-)

diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 045ee754a242b3..029322567b7ea9 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1687,6 +1687,8 @@ class Parser : public CodeCompletionHandler {
   DeclGroupPtrTy ParseExternalDeclaration(ParsedAttributes &DeclAttrs,
                                           ParsedAttributes &DeclSpecAttrs,
                                           ParsingDeclSpec *DS = nullptr);
+  DeclGroupPtrTy ParseExternalDeclarationWithAttrs(
+                                                 ParsingDeclSpec *DS = nullptr);
   bool isDeclarationAfterDeclarator();
   bool isStartOfFunctionDefinition(const ParsingDeclarator &Declarator);
   DeclGroupPtrTy ParseDeclarationOrFunctionDefinition(
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 4f050d6632c356..8a85331d14292b 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -266,12 +266,7 @@ void Parser::ParseInnerNamespace(const InnerNamespaceInfoList &InnerNSs,
   if (index == InnerNSs.size()) {
     while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) &&
            Tok.isNot(tok::eof)) {
-      ParsedAttributes DeclAttrs(AttrFactory);
-      ParsedAttributes DeclSpecAttrs(AttrFactory);
-      while (MaybeParseCXX11Attributes(DeclAttrs) ||
-              MaybeParseGNUAttributes(DeclSpecAttrs))
-        ;
-      ParseExternalDeclaration(DeclAttrs, DeclSpecAttrs);
+      ParseExternalDeclarationWithAttrs();
     }
 
     // The caller is what called check -- we are simply calling
@@ -426,12 +421,7 @@ Decl *Parser::ParseLinkage(ParsingDeclSpec &DS, DeclaratorContext Context) {
         break;
       [[fallthrough]];
     default:
-      ParsedAttributes DeclAttrs(AttrFactory);
-      ParsedAttributes DeclSpecAttrs(AttrFactory);
-      while (MaybeParseCXX11Attributes(DeclAttrs) ||
-             MaybeParseGNUAttributes(DeclSpecAttrs))
-        ;
-      ParseExternalDeclaration(DeclAttrs, DeclSpecAttrs);
+      ParseExternalDeclarationWithAttrs();
       continue;
     }
 
@@ -468,13 +458,7 @@ Decl *Parser::ParseExportDeclaration() {
       Tok.is(tok::l_brace) ? Tok.getLocation() : SourceLocation());
 
   if (Tok.isNot(tok::l_brace)) {
-    // FIXME: Factor out a ParseExternalDeclarationWithAttrs.
-    ParsedAttributes DeclAttrs(AttrFactory);
-    ParsedAttributes DeclSpecAttrs(AttrFactory);
-    while (MaybeParseCXX11Attributes(DeclAttrs) ||
-            MaybeParseGNUAttributes(DeclSpecAttrs))
-      ;
-    ParseExternalDeclaration(DeclAttrs, DeclSpecAttrs);
+    ParseExternalDeclarationWithAttrs();
     return Actions.ActOnFinishExportDecl(getCurScope(), ExportDecl,
                                          SourceLocation());
   }
@@ -484,12 +468,7 @@ Decl *Parser::ParseExportDeclaration() {
 
   while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) &&
          Tok.isNot(tok::eof)) {
-    ParsedAttributes DeclAttrs(AttrFactory);
-    ParsedAttributes DeclSpecAttrs(AttrFactory);
-    while (MaybeParseCXX11Attributes(DeclAttrs) ||
-            MaybeParseGNUAttributes(DeclSpecAttrs))
-      ;
-    ParseExternalDeclaration(DeclAttrs, DeclSpecAttrs);
+    ParseExternalDeclarationWithAttrs();
   }
 
   T.consumeClose();
diff --git a/clang/lib/Parse/ParseObjc.cpp b/clang/lib/Parse/ParseObjc.cpp
index ec0572d58b3552..5964573d70bbf8 100644
--- a/clang/lib/Parse/ParseObjc.cpp
+++ b/clang/lib/Parse/ParseObjc.cpp
@@ -2286,13 +2286,7 @@ Parser::ParseObjCAtImplementationDeclaration(SourceLocation AtLoc,
   {
     ObjCImplParsingDataRAII ObjCImplParsing(*this, ObjCImpDecl);
     while (!ObjCImplParsing.isFinished() && !isEofOrEom()) {
-      ParsedAttributes DeclAttrs(AttrFactory);
-      ParsedAttributes DeclSpecAttrs(AttrFactory);
-      while (MaybeParseCXX11Attributes(DeclAttrs) ||
-              MaybeParseGNUAttributes(DeclSpecAttrs))
-        ;
-      if (DeclGroupPtrTy DGP =
-              ParseExternalDeclaration(DeclAttrs, DeclSpecAttrs)) {
+      if (DeclGroupPtrTy DGP = ParseExternalDeclarationWithAttrs()) {
         DeclGroupRef DG = DGP.get();
         DeclsInGroup.append(DG.begin(), DG.end());
       }
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index 54246510cc2453..dc9e9c28154f93 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -745,17 +745,7 @@ bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result,
     break;
   }
 
-  ParsedAttributes DeclAttrs(AttrFactory);
-  ParsedAttributes DeclSpecAttrs(AttrFactory);
-  // GNU attributes are applied to the declaration specification while the
-  // standard attributes are applied to the declaration.  We parse the two
-  // attribute sets into different containters so we can apply them during
-  // the regular parsing process.
-  while (MaybeParseCXX11Attributes(DeclAttrs) ||
-         MaybeParseGNUAttributes(DeclSpecAttrs))
-    ;
-
-  Result = ParseExternalDeclaration(DeclAttrs, DeclSpecAttrs);
+  Result = ParseExternalDeclarationWithAttrs();
   // An empty Result might mean a line with ';' or some parsing error, ignore
   // it.
   if (Result) {
@@ -1074,6 +1064,17 @@ Parser::ParseExternalDeclaration(ParsedAttributes &Attrs,
   return Actions.ConvertDeclToDeclGroup(SingleDecl);
 }
 
+/// Parse CXX11 and GNU attributes and passes them to `ParseExternalDeclaration`
+Parser::DeclGroupPtrTy
+Parser::ParseExternalDeclarationWithAttrs(ParsingDeclSpec *DS) {
+  ParsedAttributes DeclAttrs(AttrFactory);
+  ParsedAttributes DeclSpecAttrs(AttrFactory);
+  while (MaybeParseCXX11Attributes(DeclAttrs) ||
+          MaybeParseGNUAttributes(DeclSpecAttrs))
+    ;
+  return ParseExternalDeclaration(DeclAttrs, DeclSpecAttrs, DS);
+}
+
 /// Determine whether the current token, if it occurs after a
 /// declarator, continues a declaration or declaration list.
 bool Parser::isDeclarationAfterDeclarator() {
@@ -2431,12 +2432,7 @@ void Parser::ParseMicrosoftIfExistsExternalDeclaration() {
   // Parse the declarations.
   // FIXME: Support module import within __if_exists?
   while (Tok.isNot(tok::r_brace) && !isEofOrEom()) {
-    ParsedAttributes Attrs(AttrFactory);
-    ParsedAttributes DeclSpecAttrs(AttrFactory);
-    while (MaybeParseCXX11Attributes(Attrs) ||
-            MaybeParseGNUAttributes(DeclSpecAttrs))
-      ;
-    DeclGroupPtrTy Result = ParseExternalDeclaration(Attrs, DeclSpecAttrs);
+    DeclGroupPtrTy Result = ParseExternalDeclarationWithAttrs();
     if (Result && !getCurScope()->getParent())
       Actions.getASTConsumer().HandleTopLevelDecl(Result.get());
   }



More information about the cfe-commits mailing list