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

[Mathys Gasnier via cfe-commits]

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

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 

>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] [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}}