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

via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 21 04:08:29 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Mathys Gasnier (Mathys-Gasnier)

<details>
<summary>Changes</summary>

Not all callers of `ParseExternalDeclaration` were parsing gnu attributes, this caused issues with namespaces declarations attributes needing to be in a specific order (See #<!-- -->73890).
This PR fixes this in namespaces, exports, and three other places. It also adds a test to ensure a non-regression of this behavior.

Fixes: #<!-- -->73890 

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


5 Files Affected:

- (modified) clang/lib/Parse/ParseDeclCXX.cpp (+15-9) 
- (modified) clang/lib/Parse/ParseObjc.cpp (+5-3) 
- (modified) clang/lib/Parse/ParseOpenMP.cpp (+5-3) 
- (modified) clang/lib/Parse/Parser.cpp (+5-3) 
- (modified) clang/test/Parser/cxx-attributes.cpp (+3) 


``````````diff
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}}

``````````

</details>


https://github.com/llvm/llvm-project/pull/117148


More information about the cfe-commits mailing list