Merge pull request #5655 from compnerd/attributes

parse: process GNU and standard attributes on top-level decls

Author: compnerd

clang/docs/ReleaseNotes.rst

@@ -248,6 +248,9 @@ Bug Fixes
   not satisfied in the event of an instantiation failures in a requires expression's
   parameter list. We previously handled this correctly in a constraint evaluation
   context, but not in a requires clause evaluated as a boolean.
+- GNU attributes being applied prior to standard attributes would be handled
+  improperly, which was corrected to match the behaviour exhibited by GCC.
+  `Issue 58229 <https://github.com/llvm/llvm-project/issues/58229>`_
 
 Improvements to Clang's diagnostics
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

clang/include/clang/Parse/Parser.h

@@ -1601,15 +1601,16 @@ class Parser : public CodeCompletionHandler {
 
   //===--------------------------------------------------------------------===//
   // C99 6.9: External Definitions.
-  DeclGroupPtrTy ParseExternalDeclaration(ParsedAttributes &Attrs,
+  DeclGroupPtrTy ParseExternalDeclaration(ParsedAttributes &DeclAttrs,
+                                          ParsedAttributes &DeclSpecAttrs,
                                           ParsingDeclSpec *DS = nullptr);
   bool isDeclarationAfterDeclarator();
   bool isStartOfFunctionDefinition(const ParsingDeclarator &Declarator);
-  DeclGroupPtrTy
-  ParseDeclarationOrFunctionDefinition(ParsedAttributes &Attrs,
-                                       ParsingDeclSpec *DS = nullptr,
-                                       AccessSpecifier AS = AS_none);
+  DeclGroupPtrTy ParseDeclarationOrFunctionDefinition(
+      ParsedAttributes &DeclAttrs, ParsedAttributes &DeclSpecAttrs,
+      ParsingDeclSpec *DS = nullptr, AccessSpecifier AS = AS_none);
   DeclGroupPtrTy ParseDeclOrFunctionDefInternal(ParsedAttributes &Attrs,
+                                                ParsedAttributes &DeclSpecAttrs,
                                                 ParsingDeclSpec &DS,
                                                 AccessSpecifier AS);
 
@@ -1624,7 +1625,8 @@ class Parser : public CodeCompletionHandler {
 
   // Objective-C External Declarations
   void MaybeSkipAttributes(tok::ObjCKeywordKind Kind);
-  DeclGroupPtrTy ParseObjCAtDirectives(ParsedAttributes &Attrs);
+  DeclGroupPtrTy ParseObjCAtDirectives(ParsedAttributes &DeclAttrs,
+                                       ParsedAttributes &DeclSpecAttrs);
   DeclGroupPtrTy ParseObjCAtClassDeclaration(SourceLocation atLoc);
   Decl *ParseObjCAtInterfaceDeclaration(SourceLocation AtLoc,
                                         ParsedAttributes &prefixAttrs);

clang/lib/Parse/ParseDeclCXX.cpp

@@ -254,9 +254,10 @@ void Parser::ParseInnerNamespace(const InnerNamespaceInfoList &InnerNSs,
   if (index == InnerNSs.size()) {
     while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) &&
            Tok.isNot(tok::eof)) {
-      ParsedAttributes Attrs(AttrFactory);
-      MaybeParseCXX11Attributes(Attrs);
-      ParseExternalDeclaration(Attrs);
+      ParsedAttributes DeclAttrs(AttrFactory);
+      MaybeParseCXX11Attributes(DeclAttrs);
+      ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
+      ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs);
     }
 
     // The caller is what called check -- we are simply calling
@@ -359,6 +360,7 @@ Decl *Parser::ParseLinkage(ParsingDeclSpec &DS, DeclaratorContext Context) {
 
   ParsedAttributes DeclAttrs(AttrFactory);
   MaybeParseCXX11Attributes(DeclAttrs);
+  ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
 
   if (Tok.isNot(tok::l_brace)) {
     // Reset the source range in DS, as the leading "extern"
@@ -367,7 +369,7 @@ Decl *Parser::ParseLinkage(ParsingDeclSpec &DS, DeclaratorContext Context) {
     DS.SetRangeEnd(SourceLocation());
     // ... but anyway remember that such an "extern" was seen.
     DS.setExternInLinkageSpec(true);
-    ParseExternalDeclaration(DeclAttrs, &DS);
+    ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs, &DS);
     return LinkageSpec ? Actions.ActOnFinishLinkageSpecification(
                              getCurScope(), LinkageSpec, SourceLocation())
                        : nullptr;
@@ -407,9 +409,9 @@ Decl *Parser::ParseLinkage(ParsingDeclSpec &DS, DeclaratorContext Context) {
         break;
       [[fallthrough]];
     default:
-      ParsedAttributes Attrs(AttrFactory);
-      MaybeParseCXX11Attributes(Attrs);
-      ParseExternalDeclaration(Attrs);
+      ParsedAttributes DeclAttrs(AttrFactory);
+      MaybeParseCXX11Attributes(DeclAttrs);
+      ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs);
       continue;
     }
 
@@ -439,9 +441,10 @@ Decl *Parser::ParseExportDeclaration() {
 
   if (Tok.isNot(tok::l_brace)) {
     // FIXME: Factor out a ParseExternalDeclarationWithAttrs.
-    ParsedAttributes Attrs(AttrFactory);
-    MaybeParseCXX11Attributes(Attrs);
-    ParseExternalDeclaration(Attrs);
+    ParsedAttributes DeclAttrs(AttrFactory);
+    MaybeParseCXX11Attributes(DeclAttrs);
+    ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
+    ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs);
     return Actions.ActOnFinishExportDecl(getCurScope(), ExportDecl,
                                          SourceLocation());
   }
@@ -458,9 +461,10 @@ Decl *Parser::ParseExportDeclaration() {
 
   while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) &&
          Tok.isNot(tok::eof)) {
-    ParsedAttributes Attrs(AttrFactory);
-    MaybeParseCXX11Attributes(Attrs);
-    ParseExternalDeclaration(Attrs);
+    ParsedAttributes DeclAttrs(AttrFactory);
+    MaybeParseCXX11Attributes(DeclAttrs);
+    ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
+    ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs);
   }
 
   T.consumeClose();

clang/lib/Parse/ParseHLSL.cpp

@@ -77,9 +77,11 @@ Decl *Parser::ParseHLSLBuffer(SourceLocation &DeclEnd) {
 
   while (Tok.isNot(tok::r_brace) && Tok.isNot(tok::eof)) {
     // FIXME: support attribute on constants inside cbuffer/tbuffer.
-    ParsedAttributes Attrs(AttrFactory);
+    ParsedAttributes DeclAttrs(AttrFactory);
+    ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
 
-    DeclGroupPtrTy Result = ParseExternalDeclaration(Attrs);
+    DeclGroupPtrTy Result =
+        ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs);
     if (!validateDeclsInsideHLSLBuffer(Result, IdentifierLoc, IsCBuffer,
                                        *this)) {
       T.skipToEnd();

clang/lib/Parse/ParseObjc.cpp

@@ -45,7 +45,11 @@ void Parser::MaybeSkipAttributes(tok::ObjCKeywordKind Kind) {
 /// [OBJC]  objc-protocol-definition
 /// [OBJC]  objc-method-definition
 /// [OBJC]  '@' 'end'
-Parser::DeclGroupPtrTy Parser::ParseObjCAtDirectives(ParsedAttributes &Attrs) {
+Parser::DeclGroupPtrTy
+Parser::ParseObjCAtDirectives(ParsedAttributes &DeclAttrs,
+                              ParsedAttributes &DeclSpecAttrs) {
+  DeclAttrs.takeAllFrom(DeclSpecAttrs);
+
   SourceLocation AtLoc = ConsumeToken(); // the "@"
 
   if (Tok.is(tok::code_completion)) {
@@ -54,17 +58,29 @@ Parser::DeclGroupPtrTy Parser::ParseObjCAtDirectives(ParsedAttributes &Attrs) {
     return nullptr;
   }
 
+  switch (Tok.getObjCKeywordID()) {
+  case tok::objc_interface:
+  case tok::objc_protocol:
+  case tok::objc_implementation:
+    break;
+  default:
+    llvm::for_each(DeclAttrs, [this](const auto &Attr) {
+      if (Attr.isGNUAttribute())
+        Diag(Tok.getLocation(), diag::err_objc_unexpected_attr);
+    });
+  }
+
   Decl *SingleDecl = nullptr;
   switch (Tok.getObjCKeywordID()) {
   case tok::objc_class:
     return ParseObjCAtClassDeclaration(AtLoc);
   case tok::objc_interface:
-    SingleDecl = ParseObjCAtInterfaceDeclaration(AtLoc, Attrs);
+    SingleDecl = ParseObjCAtInterfaceDeclaration(AtLoc, DeclAttrs);
     break;
   case tok::objc_protocol:
-    return ParseObjCAtProtocolDeclaration(AtLoc, Attrs);
+    return ParseObjCAtProtocolDeclaration(AtLoc, DeclAttrs);
   case tok::objc_implementation:
-    return ParseObjCAtImplementationDeclaration(AtLoc, Attrs);
+    return ParseObjCAtImplementationDeclaration(AtLoc, DeclAttrs);
   case tok::objc_end:
     return ParseObjCAtEndDeclaration(AtLoc);
   case tok::objc_compatibility_alias:
@@ -652,21 +668,23 @@ void Parser::ParseObjCInterfaceDeclList(tok::ObjCKeywordKind contextKey,
       if (Tok.is(tok::r_brace))
         break;
 
-      ParsedAttributes EmptyAttrs(AttrFactory);
+      ParsedAttributes EmptyDeclAttrs(AttrFactory);
+      ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
 
       // Since we call ParseDeclarationOrFunctionDefinition() instead of
       // ParseExternalDeclaration() below (so that this doesn't parse nested
       // @interfaces), this needs to duplicate some code from the latter.
       if (Tok.isOneOf(tok::kw_static_assert, tok::kw__Static_assert)) {
         SourceLocation DeclEnd;
         ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
-        allTUVariables.push_back(ParseDeclaration(
-            DeclaratorContext::File, DeclEnd, EmptyAttrs, EmptyDeclSpecAttrs));
+        allTUVariables.push_back(ParseDeclaration(DeclaratorContext::File,
+                                                  DeclEnd, EmptyDeclAttrs,
+                                                  EmptyDeclSpecAttrs));
         continue;
       }
 
-      allTUVariables.push_back(
-          ParseDeclarationOrFunctionDefinition(EmptyAttrs));
+      allTUVariables.push_back(ParseDeclarationOrFunctionDefinition(
+          EmptyDeclAttrs, EmptyDeclSpecAttrs));
       continue;
     }
 
@@ -2225,9 +2243,11 @@ Parser::ParseObjCAtImplementationDeclaration(SourceLocation AtLoc,
   {
     ObjCImplParsingDataRAII ObjCImplParsing(*this, ObjCImpDecl);
     while (!ObjCImplParsing.isFinished() && !isEofOrEom()) {
-      ParsedAttributes attrs(AttrFactory);
-      MaybeParseCXX11Attributes(attrs);
-      if (DeclGroupPtrTy DGP = ParseExternalDeclaration(attrs)) {
+      ParsedAttributes DeclAttrs(AttrFactory);
+      MaybeParseCXX11Attributes(DeclAttrs);
+      ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
+      if (DeclGroupPtrTy DGP =
+              ParseExternalDeclaration(DeclAttrs, EmptyDeclSpecAttrs)) {
         DeclGroupRef DG = DGP.get();
         DeclsInGroup.append(DG.begin(), DG.end());
       }

clang/lib/Parse/ParseOpenMP.cpp

@@ -2245,9 +2245,10 @@ 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);
         ParsingDeclSpec PDS(*this);
-        Ptr = ParseExternalDeclaration(Attrs, &PDS);
+        Ptr = ParseExternalDeclaration(Attrs, EmptyDeclSpecAttrs, &PDS);
       } else {
         Ptr =
             ParseCXXClassMemberDeclarationWithPragmas(AS, Attrs, TagType, Tag);

clang/lib/Parse/Parser.cpp

@@ -739,10 +739,17 @@ bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result,
     break;
   }
 
-  ParsedAttributes attrs(AttrFactory);
-  MaybeParseCXX11Attributes(attrs);
-
-  Result = ParseExternalDeclaration(attrs);
+  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);
   // An empty Result might mean a line with ';' or some parsing error, ignore
   // it.
   if (Result) {
@@ -785,8 +792,10 @@ bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result,
 ///
 /// [Modules-TS] module-import-declaration
 ///
-Parser::DeclGroupPtrTy Parser::ParseExternalDeclaration(ParsedAttributes &Attrs,
-                                                        ParsingDeclSpec *DS) {
+Parser::DeclGroupPtrTy
+Parser::ParseExternalDeclaration(ParsedAttributes &Attrs,
+                                 ParsedAttributes &DeclSpecAttrs,
+                                 ParsingDeclSpec *DS) {
   DestroyTemplateIdAnnotationsRAIIObj CleanupRAII(*this);
   ParenBraceBracketBalancer BalancerRAIIObj(*this);
 
@@ -874,7 +883,7 @@ Parser::DeclGroupPtrTy Parser::ParseExternalDeclaration(ParsedAttributes &Attrs,
     // __extension__ silences extension warnings in the subexpression.
     ExtensionRAIIObject O(Diags);  // Use RAII to do this.
     ConsumeToken();
-    return ParseExternalDeclaration(Attrs);
+    return ParseExternalDeclaration(Attrs, DeclSpecAttrs);
   }
   case tok::kw_asm: {
     ProhibitAttributes(Attrs);
@@ -902,7 +911,7 @@ Parser::DeclGroupPtrTy Parser::ParseExternalDeclaration(ParsedAttributes &Attrs,
     break;
   }
   case tok::at:
-    return ParseObjCAtDirectives(Attrs);
+    return ParseObjCAtDirectives(Attrs, DeclSpecAttrs);
   case tok::minus:
   case tok::plus:
     if (!getLangOpts().ObjC) {
@@ -950,18 +959,16 @@ Parser::DeclGroupPtrTy Parser::ParseExternalDeclaration(ParsedAttributes &Attrs,
     // A function definition cannot start with any of these keywords.
     {
       SourceLocation DeclEnd;
-      ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
       return ParseDeclaration(DeclaratorContext::File, DeclEnd, Attrs,
-                              EmptyDeclSpecAttrs);
+                              DeclSpecAttrs);
     }
 
   case tok::kw_cbuffer:
   case tok::kw_tbuffer:
     if (getLangOpts().HLSL) {
       SourceLocation DeclEnd;
-      ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
       return ParseDeclaration(DeclaratorContext::File, DeclEnd, Attrs,
-                              EmptyDeclSpecAttrs);
+                              DeclSpecAttrs);
     }
     goto dont_know;
 
@@ -972,9 +979,8 @@ Parser::DeclGroupPtrTy Parser::ParseExternalDeclaration(ParsedAttributes &Attrs,
       Diag(ConsumeToken(), diag::warn_static_inline_explicit_inst_ignored)
         << 0;
       SourceLocation DeclEnd;
-      ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
       return ParseDeclaration(DeclaratorContext::File, DeclEnd, Attrs,
-                              EmptyDeclSpecAttrs);
+                              DeclSpecAttrs);
     }
     goto dont_know;
 
@@ -985,9 +991,8 @@ Parser::DeclGroupPtrTy Parser::ParseExternalDeclaration(ParsedAttributes &Attrs,
       // Inline namespaces. Allowed as an extension even in C++03.
       if (NextKind == tok::kw_namespace) {
         SourceLocation DeclEnd;
-        ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
         return ParseDeclaration(DeclaratorContext::File, DeclEnd, Attrs,
-                                EmptyDeclSpecAttrs);
+                                DeclSpecAttrs);
       }
 
       // Parse (then ignore) 'inline' prior to a template instantiation. This is
@@ -996,9 +1001,8 @@ Parser::DeclGroupPtrTy Parser::ParseExternalDeclaration(ParsedAttributes &Attrs,
         Diag(ConsumeToken(), diag::warn_static_inline_explicit_inst_ignored)
           << 1;
         SourceLocation DeclEnd;
-        ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
         return ParseDeclaration(DeclaratorContext::File, DeclEnd, Attrs,
-                                EmptyDeclSpecAttrs);
+                                DeclSpecAttrs);
       }
     }
     goto dont_know;
@@ -1034,7 +1038,7 @@ Parser::DeclGroupPtrTy Parser::ParseExternalDeclaration(ParsedAttributes &Attrs,
       return nullptr;
     }
     // We can't tell whether this is a function-definition or declaration yet.
-    return ParseDeclarationOrFunctionDefinition(Attrs, DS);
+    return ParseDeclarationOrFunctionDefinition(Attrs, DeclSpecAttrs, DS);
   }
 
   // This routine returns a DeclGroup, if the thing we parsed only contains a
@@ -1099,7 +1103,17 @@ bool Parser::isStartOfFunctionDefinition(const ParsingDeclarator &Declarator) {
 /// [OMP]   allocate-directive                         [TODO]
 ///
 Parser::DeclGroupPtrTy Parser::ParseDeclOrFunctionDefInternal(
-    ParsedAttributes &Attrs, ParsingDeclSpec &DS, AccessSpecifier AS) {
+    ParsedAttributes &Attrs, ParsedAttributes &DeclSpecAttrs,
+    ParsingDeclSpec &DS, AccessSpecifier AS) {
+  // Because we assume that the DeclSpec has not yet been initialised, we simply
+  // overwrite the source range and attribute the provided leading declspec
+  // attributes.
+  assert(DS.getSourceRange().isInvalid() &&
+         "expected uninitialised source range");
+  DS.SetRangeStart(DeclSpecAttrs.Range.getBegin());
+  DS.SetRangeEnd(DeclSpecAttrs.Range.getEnd());
+  DS.takeAttributesFrom(DeclSpecAttrs);
+
   MaybeParseMicrosoftAttributes(DS.getAttributes());
   // Parse the common declaration-specifiers piece.
   ParseDeclarationSpecifiers(DS, ParsedTemplateInfo(), AS,
@@ -1198,17 +1212,18 @@ Parser::DeclGroupPtrTy Parser::ParseDeclOrFunctionDefInternal(
 }
 
 Parser::DeclGroupPtrTy Parser::ParseDeclarationOrFunctionDefinition(
-    ParsedAttributes &Attrs, ParsingDeclSpec *DS, AccessSpecifier AS) {
+    ParsedAttributes &Attrs, ParsedAttributes &DeclSpecAttrs,
+    ParsingDeclSpec *DS, AccessSpecifier AS) {
   if (DS) {
-    return ParseDeclOrFunctionDefInternal(Attrs, *DS, AS);
+    return ParseDeclOrFunctionDefInternal(Attrs, DeclSpecAttrs, *DS, AS);
   } else {
     ParsingDeclSpec PDS(*this);
     // Must temporarily exit the objective-c container scope for
     // parsing c constructs and re-enter objc container scope
     // afterwards.
     ObjCDeclContextSwitch ObjCDC(*this);
 
-    return ParseDeclOrFunctionDefInternal(Attrs, PDS, AS);
+    return ParseDeclOrFunctionDefInternal(Attrs, DeclSpecAttrs, PDS, AS);
   }
 }
 
@@ -2350,7 +2365,8 @@ void Parser::ParseMicrosoftIfExistsExternalDeclaration() {
   while (Tok.isNot(tok::r_brace) && !isEofOrEom()) {
     ParsedAttributes Attrs(AttrFactory);
     MaybeParseCXX11Attributes(Attrs);
-    DeclGroupPtrTy Result = ParseExternalDeclaration(Attrs);
+    ParsedAttributes EmptyDeclSpecAttrs(AttrFactory);
+    DeclGroupPtrTy Result = ParseExternalDeclaration(Attrs, EmptyDeclSpecAttrs);
     if (Result && !getCurScope()->getParent())
       Actions.getASTConsumer().HandleTopLevelDecl(Result.get());
   }