[clang] Reject non-declaration C++11 attributes on declarations

For backwards compatiblity, we emit only a warning instead of an error if the
attribute is one of the existing type attributes that we have historically
allowed to "slide" to the `DeclSpec` just as if it had been specified in GNU
syntax. (We will call these "legacy type attributes" below.)

The high-level changes that achieve this are:

- We introduce a new field `Declarator::DeclarationAttrs` (with appropriate
  accessors) to store C++11 attributes occurring in the attribute-specifier-seq
  at the beginning of a simple-declaration (and other similar declarations).
  Previously, these attributes were placed on the `DeclSpec`, which made it
  impossible to reconstruct later on whether the attributes had in fact been
  placed on the decl-specifier-seq or ahead of the declaration.

- In the parser, we propgate declaration attributes and decl-specifier-seq
  attributes separately until we can place them in
  `Declarator::DeclarationAttrs` or `DeclSpec::Attrs`, respectively.

- In `ProcessDeclAttributes()`, in addition to processing declarator attributes,
  we now also process the attributes from `Declarator::DeclarationAttrs` (except
  if they are legacy type attributes).

- In `ConvertDeclSpecToType()`, in addition to processing `DeclSpec` attributes,
  we also process any legacy type attributes that occur in
  `Declarator::DeclarationAttrs` (and emit a warning).

- We make `ProcessDeclAttribute` emit an error if it sees any non-declaration
  attributes in C++11 syntax, except in the following cases:
  - If it is being called for attributes on a `DeclSpec` or `DeclaratorChunk`
  - If the attribute is a legacy type attribute (in which case we only emit
    a warning)

The standard justifies treating attributes at the beginning of a
simple-declaration and attributes after a declarator-id the same. Here are some
relevant parts of the standard:

- The attribute-specifier-seq at the beginning of a simple-declaration
  "appertains to each of the entities declared by the declarators of the
  init-declarator-list" (https://eel.is/c++draft/dcl.dcl#dcl.pre-3)

- "In the declaration for an entity, attributes appertaining to that entity can
  appear at the start of the declaration and after the declarator-id for that
  declaration." (https://eel.is/c++draft/dcl.dcl#dcl.pre-note-2)

- "The optional attribute-specifier-seq following a declarator-id appertains to
  the entity that is declared."
  (https://eel.is/c++draft/dcl.dcl#dcl.meaning.general-1)

The standard contains similar wording to that for a simple-declaration in other
similar types of declarations, for example:

- "The optional attribute-specifier-seq in a parameter-declaration appertains to
  the parameter." (https://eel.is/c++draft/dcl.fct#3)

- "The optional attribute-specifier-seq in an exception-declaration appertains
  to the parameter of the catch clause" (https://eel.is/c++draft/except.pre#1)

The new behavior is tested both on the newly added type attribute
`annotate_type`, for which we emit errors, and for the legacy type attribute
`address_space` (chosen somewhat randomly from the various legacy type
attributes), for which we emit warnings.

Depends On D111548

Reviewed By: aaron.ballman, rsmith

Differential Revision: https://reviews.llvm.org/D126061

1 files changed, 87 insertions, 49 deletions

diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 12af519..a90d5d9 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -57,7 +57,7 @@ TypeResult Parser::ParseTypeName(SourceRange *Range, DeclaratorContext Context,
     *OwnedType = DS.isTypeSpecOwned() ? DS.getRepAsDecl() : nullptr;
 
   // Parse the abstract-declarator, if present.
-  Declarator DeclaratorInfo(DS, Context);
+  Declarator DeclaratorInfo(DS, ParsedAttributesView::none(), Context);
   ParseDeclarator(DeclaratorInfo);
   if (Range)
     *Range = DeclaratorInfo.getSourceRange();
@@ -1760,7 +1760,8 @@ void Parser::stripTypeAttributesOffDeclSpec(ParsedAttributes &Attrs,
 ///
 Parser::DeclGroupPtrTy Parser::ParseDeclaration(DeclaratorContext Context,
                                                 SourceLocation &DeclEnd,
-                                                ParsedAttributes &Attrs,
+                                                ParsedAttributes &DeclAttrs,
+                                                ParsedAttributes &DeclSpecAttrs,
                                                 SourceLocation *DeclSpecStart) {
   ParenBraceBracketBalancer BalancerRAIIObj(*this);
   // Must temporarily exit the objective-c container scope for
@@ -1771,32 +1772,40 @@ Parser::DeclGroupPtrTy Parser::ParseDeclaration(DeclaratorContext Context,
   switch (Tok.getKind()) {
   case tok::kw_template:
   case tok::kw_export:
-    ProhibitAttributes(Attrs);
-    SingleDecl = ParseDeclarationStartingWithTemplate(Context, DeclEnd, Attrs);
+    ProhibitAttributes(DeclAttrs);
+    ProhibitAttributes(DeclSpecAttrs);
+    SingleDecl =
+        ParseDeclarationStartingWithTemplate(Context, DeclEnd, DeclAttrs);
     break;
   case tok::kw_inline:
     // Could be the start of an inline namespace. Allowed as an ext in C++03.
     if (getLangOpts().CPlusPlus && NextToken().is(tok::kw_namespace)) {
-      ProhibitAttributes(Attrs);
+      ProhibitAttributes(DeclAttrs);
+      ProhibitAttributes(DeclSpecAttrs);
       SourceLocation InlineLoc = ConsumeToken();
       return ParseNamespace(Context, DeclEnd, InlineLoc);
     }
-    return ParseSimpleDeclaration(Context, DeclEnd, Attrs, true, nullptr,
-                                  DeclSpecStart);
+    return ParseSimpleDeclaration(Context, DeclEnd, DeclAttrs, DeclSpecAttrs,
+                                  true, nullptr, DeclSpecStart);
   case tok::kw_namespace:
-    ProhibitAttributes(Attrs);
+    ProhibitAttributes(DeclAttrs);
+    ProhibitAttributes(DeclSpecAttrs);
     return ParseNamespace(Context, DeclEnd);
-  case tok::kw_using:
+  case tok::kw_using: {
+    ParsedAttributes Attrs(AttrFactory);
+    takeAndConcatenateAttrs(DeclAttrs, DeclSpecAttrs, Attrs);
     return ParseUsingDirectiveOrDeclaration(Context, ParsedTemplateInfo(),
                                             DeclEnd, Attrs);
+  }
   case tok::kw_static_assert:
   case tok::kw__Static_assert:
-    ProhibitAttributes(Attrs);
+    ProhibitAttributes(DeclAttrs);
+    ProhibitAttributes(DeclSpecAttrs);
     SingleDecl = ParseStaticAssertDeclaration(DeclEnd);
     break;
   default:
-    return ParseSimpleDeclaration(Context, DeclEnd, Attrs, true, nullptr,
-                                  DeclSpecStart);
+    return ParseSimpleDeclaration(Context, DeclEnd, DeclAttrs, DeclSpecAttrs,
+                                  true, nullptr, DeclSpecStart);
   }
 
   // This routine returns a DeclGroup, if the thing we parsed only contains a
@@ -1826,10 +1835,17 @@ Parser::DeclGroupPtrTy Parser::ParseDeclaration(DeclaratorContext Context,
 /// the Declaration. The SourceLocation for this Decl is set to
 /// DeclSpecStart if DeclSpecStart is non-null.
 Parser::DeclGroupPtrTy Parser::ParseSimpleDeclaration(
-    DeclaratorContext Context, SourceLocation &DeclEnd, ParsedAttributes &Attrs,
+    DeclaratorContext Context, SourceLocation &DeclEnd,
+    ParsedAttributes &DeclAttrs, ParsedAttributes &DeclSpecAttrs,
     bool RequireSemi, ForRangeInit *FRI, SourceLocation *DeclSpecStart) {
+  // Need to retain these for diagnostics before we add them to the DeclSepc.
+  ParsedAttributesView OriginalDeclSpecAttrs;
+  OriginalDeclSpecAttrs.addAll(DeclSpecAttrs.begin(), DeclSpecAttrs.end());
+  OriginalDeclSpecAttrs.Range = DeclSpecAttrs.Range;
+
   // Parse the common declaration-specifiers piece.
   ParsingDeclSpec DS(*this);
+  DS.takeAttributesFrom(DeclSpecAttrs);
 
   DeclSpecContext DSContext = getDeclSpecContextFromDeclaratorContext(Context);
   ParseDeclarationSpecifiers(DS, ParsedTemplateInfo(), AS_none, DSContext);
@@ -1843,7 +1859,7 @@ Parser::DeclGroupPtrTy Parser::ParseSimpleDeclaration(
   // C99 6.7.2.3p6: Handle "struct-or-union identifier;", "enum { X };"
   // declaration-specifiers init-declarator-list[opt] ';'
   if (Tok.is(tok::semi)) {
-    ProhibitAttributes(Attrs);
+    ProhibitAttributes(DeclAttrs);
     DeclEnd = Tok.getLocation();
     if (RequireSemi) ConsumeToken();
     RecordDecl *AnonRecord = nullptr;
@@ -1860,8 +1876,7 @@ Parser::DeclGroupPtrTy Parser::ParseSimpleDeclaration(
   if (DeclSpecStart)
     DS.SetRangeStart(*DeclSpecStart);
 
-  DS.takeAttributesFrom(Attrs);
-  return ParseDeclGroup(DS, Context, &DeclEnd, FRI);
+  return ParseDeclGroup(DS, Context, DeclAttrs, &DeclEnd, FRI);
 }
 
 /// Returns true if this might be the start of a declarator, or a common typo
@@ -2016,10 +2031,16 @@ void Parser::SkipMalformedDecl() {
 /// result.
 Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
                                               DeclaratorContext Context,
+                                              ParsedAttributes &Attrs,
                                               SourceLocation *DeclEnd,
                                               ForRangeInit *FRI) {
   // Parse the first declarator.
-  ParsingDeclarator D(*this, DS, Context);
+  // Consume all of the attributes from `Attrs` by moving them to our own local
+  // list. This ensures that we will not attempt to interpret them as statement
+  // attributes higher up the callchain.
+  ParsedAttributes LocalAttrs(AttrFactory);
+  LocalAttrs.takeAllFrom(Attrs);
+  ParsingDeclarator D(*this, DS, LocalAttrs, Context);
   ParseDeclarator(D);
 
   // Bail out if the first declarator didn't seem well-formed.
@@ -3176,11 +3197,7 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec &DS,
       if (!AttrsLastTime)
         ProhibitAttributes(attrs);
       else {
-        // Reject most C++11 / C2x attributes on the decl-specifier-seq, but
-        // allow `annotate_type` as a special case.
-        // FIXME: We should more generally allow type attributes to be placed
-        // on the decl-specifier-seq; https://reviews.llvm.org/D126061 will
-        // make this change.
+        // Reject C++11 / C2x attributes that aren't type attributes.
         for (const ParsedAttr &PA : attrs) {
           if (!PA.isCXX11Attribute() && !PA.isC2xAttribute())
             continue;
@@ -3188,7 +3205,18 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec &DS,
             // We will warn about the unknown attribute elsewhere (in
             // SemaDeclAttr.cpp)
             continue;
-          if (PA.getKind() == ParsedAttr::AT_AnnotateType)
+          // GCC ignores this attribute when placed on the DeclSpec in [[]]
+          // syntax, so we do the same.
+          if (PA.getKind() == ParsedAttr::AT_VectorSize) {
+            Diag(PA.getLoc(), diag::warn_attribute_ignored) << PA;
+            PA.setInvalid();
+            continue;
+          }
+          // We reject AT_LifetimeBound and AT_AnyX86NoCfCheck, even though they
+          // are type attributes, because we historically haven't allowed these
+          // to be used as type attributes in C++11 / C2x syntax.
+          if (PA.isTypeAttr() && PA.getKind() != ParsedAttr::AT_LifetimeBound &&
+              PA.getKind() != ParsedAttr::AT_AnyX86NoCfCheck)
             continue;
           Diag(PA.getLoc(), diag::err_attribute_not_type_attr) << PA;
           PA.setInvalid();
@@ -4317,7 +4345,6 @@ void Parser::ParseStructDeclaration(
   // Parse leading attributes.
   ParsedAttributes Attrs(AttrFactory);
   MaybeParseCXX11Attributes(Attrs);
-  DS.takeAttributesFrom(Attrs);
 
   // Parse the common specifier-qualifiers-list piece.
   ParseSpecifierQualifierList(DS);
@@ -4325,6 +4352,11 @@ void Parser::ParseStructDeclaration(
   // If there are no declarators, this is a free-standing declaration
   // specifier. Let the actions module cope with it.
   if (Tok.is(tok::semi)) {
+    // C2x 6.7.2.1p9 : "The optional attribute specifier sequence in a
+    // member declaration appertains to each of the members declared by the
+    // member declarator list; it shall not appear if the optional member
+    // declarator list is omitted."
+    ProhibitAttributes(Attrs);
     RecordDecl *AnonRecord = nullptr;
     Decl *TheDecl = Actions.ParsedFreeStandingDeclSpec(getCurScope(), AS_none,
                                                        DS, AnonRecord);
@@ -4337,7 +4369,7 @@ void Parser::ParseStructDeclaration(
   bool FirstDeclarator = true;
   SourceLocation CommaLoc;
   while (true) {
-    ParsingFieldDeclarator DeclaratorInfo(*this, DS);
+    ParsingFieldDeclarator DeclaratorInfo(*this, DS, Attrs);
     DeclaratorInfo.D.setCommaLoc(CommaLoc);
 
     // Attributes are only allowed here on successive declarators.
@@ -4696,7 +4728,8 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, DeclSpec &DS,
       // declares 'enum E : int; E *p;' not 'enum E : int*; E p;'.
       DeclSpec DS(AttrFactory);
       ParseSpecifierQualifierList(DS, AS, DeclSpecContext::DSC_type_specifier);
-      Declarator DeclaratorInfo(DS, DeclaratorContext::TypeName);
+      Declarator DeclaratorInfo(DS, ParsedAttributesView::none(),
+                                DeclaratorContext::TypeName);
       BaseType = Actions.ActOnTypeName(getCurScope(), DeclaratorInfo);
 
       BaseRange = SourceRange(ColonLoc, DeclaratorInfo.getSourceRange().getEnd());
@@ -6618,9 +6651,9 @@ void Parser::InitCXXThisScopeForDeclaratorIfRelevant(
 /// declarator D up to a paren, which indicates that we are parsing function
 /// arguments.
 ///
-/// If FirstArgAttrs is non-null, then the caller parsed those arguments
-/// immediately after the open paren - they should be considered to be the
-/// first argument of a parameter.
+/// If FirstArgAttrs is non-null, then the caller parsed those attributes
+/// immediately after the open paren - they will be applied to the DeclSpec
+/// of the first parameter.
 ///
 /// If RequiresArg is true, then the first argument of the function is required
 /// to be present and required to not be an identifier list.
@@ -6922,7 +6955,7 @@ void Parser::ParseFunctionDeclaratorIdentifierList(
 ///
 /// DeclContext is the context of the declarator being parsed.  If FirstArgAttrs
 /// is non-null, then the caller parsed those attributes immediately after the
-/// open paren - they should be considered to be part of the first parameter.
+/// open paren - they will be applied to the DeclSpec of the first parameter.
 ///
 /// After returning, ParamInfo will hold the parsed parameters. EllipsisLoc will
 /// be the location of the ellipsis, if any was parsed.
@@ -6974,33 +7007,37 @@ void Parser::ParseParameterDeclarationClause(
     // Just use the ParsingDeclaration "scope" of the declarator.
     DeclSpec DS(AttrFactory);
 
-    // Parse any C++11 attributes.
-    MaybeParseCXX11Attributes(DS.getAttributes());
+    ParsedAttributes ArgDeclAttrs(AttrFactory);
+    ParsedAttributes ArgDeclSpecAttrs(AttrFactory);
+
+    if (FirstArgAttrs.Range.isValid()) {
+      // If the caller parsed attributes for the first argument, add them now.
+      // Take them so that we only apply the attributes to the first parameter.
+      // We have already started parsing the decl-specifier sequence, so don't
+      // parse any parameter-declaration pieces that precede it.
+      ArgDeclSpecAttrs.takeAllFrom(FirstArgAttrs);
+    } else {
+      // Parse any C++11 attributes.
+      MaybeParseCXX11Attributes(ArgDeclAttrs);
 
-    // Skip any Microsoft attributes before a param.
-    MaybeParseMicrosoftAttributes(DS.getAttributes());
+      // Skip any Microsoft attributes before a param.
+      MaybeParseMicrosoftAttributes(ArgDeclSpecAttrs);
+    }
 
     SourceLocation DSStart = Tok.getLocation();
 
-    // If the caller parsed attributes for the first argument, add them now.
-    // Take them so that we only apply the attributes to the first parameter.
-    // FIXME: If we can leave the attributes in the token stream somehow, we can
-    // get rid of a parameter (FirstArgAttrs) and this statement. It might be
-    // too much hassle.
-    DS.takeAttributesFrom(FirstArgAttrs);
-
     ParseDeclarationSpecifiers(DS);
-
+    DS.takeAttributesFrom(ArgDeclSpecAttrs);
 
     // Parse the declarator.  This is "PrototypeContext" or
     // "LambdaExprParameterContext", because we must accept either
     // 'declarator' or 'abstract-declarator' here.
-    Declarator ParmDeclarator(
-        DS, DeclaratorCtx == DeclaratorContext::RequiresExpr
-                ? DeclaratorContext::RequiresExpr
-                : DeclaratorCtx == DeclaratorContext::LambdaExpr
-                      ? DeclaratorContext::LambdaExprParameter
-                      : DeclaratorContext::Prototype);
+    Declarator ParmDeclarator(DS, ArgDeclAttrs,
+                              DeclaratorCtx == DeclaratorContext::RequiresExpr
+                                  ? DeclaratorContext::RequiresExpr
+                              : DeclaratorCtx == DeclaratorContext::LambdaExpr
+                                  ? DeclaratorContext::LambdaExprParameter
+                                  : DeclaratorContext::Prototype);
     ParseDeclarator(ParmDeclarator);
 
     // Parse GNU attributes, if present.
@@ -7299,7 +7336,8 @@ void Parser::ParseMisplacedBracketDeclarator(Declarator &D) {
   assert(!D.mayOmitIdentifier() && "Declarator cannot omit identifier");
 
   SourceLocation StartBracketLoc = Tok.getLocation();
-  Declarator TempDeclarator(D.getDeclSpec(), D.getContext());
+  Declarator TempDeclarator(D.getDeclSpec(), ParsedAttributesView::none(),
+                            D.getContext());
 
   while (Tok.is(tok::l_square)) {
     ParseBracketDeclarator(TempDeclarator);