[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 06:01:20 PST 2024


================
@@ -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))
+        ;
----------------
Sirraide wrote:

> If we factor out `ParseExternalDeclaration` in a function that also parses the attributes, factoring out attributes parsing is probably not as usefull right ?

Yeah, I think just making a new function that both parses attributes and then calls `ParseExternalDeclaration` seems to be enough.

> Since it's quite a lot of changes, should I do it in two commits ?

The commit history of a pr isn’t really that important because we always squash on merge anyway, so use however many commits you need—I’d recommend avoiding force-pushing though because it can make reviews more complicated (I at least find it harder to figure out what’s changed since my last review if there was a force-push).

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


More information about the cfe-commits mailing list