mboehme updated this revision to Diff 428016.
mboehme edited the summary of this revision.
mboehme added a comment.
Herald added a subscriber: jdoerfert.
- Added warnings for "legacy" type attributes
- Added documentation
- Fixed TODOs in the code


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124919/new/

https://reviews.llvm.org/D124919

Files:
  clang/include/clang/Basic/AttributeCommonInfo.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Parse/RAIIObjectsForParser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/Attributes.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/ParsedAttr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/annotate-type.c
  clang/test/SemaCXX/address-space-placement.cpp
  clang/test/SemaCXX/annotate-type.cpp
  clang/test/SemaOpenCL/address-spaces.cl
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===================================================================
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3734,7 +3734,7 @@
     if (!StmtSubjects.empty()) {
       OS << "bool diagAppertainsToDecl(Sema &S, const ParsedAttr &AL, ";
       OS << "const Decl *D) const override {\n";
-      OS << "  S.Diag(AL.getLoc(), diag::err_stmt_attribute_invalid_on_decl)\n";
+      OS << "  S.Diag(AL.getLoc(), diag::err_attribute_invalid_on_decl)\n";
       OS << "    << AL << D->getLocation();\n";
       OS << "  return false;\n";
       OS << "}\n\n";
Index: clang/test/SemaOpenCL/address-spaces.cl
===================================================================
--- clang/test/SemaOpenCL/address-spaces.cl
+++ clang/test/SemaOpenCL/address-spaces.cl
@@ -266,9 +266,9 @@
   __attribute__((opencl_private)) private_int_t var5;  // expected-warning {{multiple identical address spaces specified for type}}
   __attribute__((opencl_private)) private_int_t *var6; // expected-warning {{multiple identical address spaces specified for type}}
 #if __OPENCL_CPP_VERSION__
-  [[clang::opencl_private]] __global int var7;         // expected-error {{multiple address spaces specified for type}}
-  [[clang::opencl_private]] __global int *var8;        // expected-error {{multiple address spaces specified for type}}
-  [[clang::opencl_private]] private_int_t var9;        // expected-warning {{multiple identical address spaces specified for type}}
-  [[clang::opencl_private]] private_int_t *var10;      // expected-warning {{multiple identical address spaces specified for type}}
+  __global int [[clang::opencl_private]] var7;         // expected-error {{multiple address spaces specified for type}}
+  __global int [[clang::opencl_private]] *var8;        // expected-error {{multiple address spaces specified for type}}
+  private_int_t [[clang::opencl_private]] var9;        // expected-warning {{multiple identical address spaces specified for type}}
+  private_int_t [[clang::opencl_private]] *var10;      // expected-warning {{multiple identical address spaces specified for type}}
 #endif // !__OPENCL_CPP_VERSION__
 }
Index: clang/test/SemaCXX/annotate-type.cpp
===================================================================
--- clang/test/SemaCXX/annotate-type.cpp
+++ clang/test/SemaCXX/annotate-type.cpp
@@ -2,10 +2,7 @@
 
 struct S1 {
   void f() [[clang::annotate_type("foo")]];
-  // FIXME: We would want to prohibit the attribute in the following location.
-  // However, Clang currently generally doesn't prohibit type-only C++11
-  // attributes on declarations. This should be fixed more generally.
-  [[clang::annotate_type("foo")]] void g();
+  [[clang::annotate_type("foo")]] void g(); // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
 };
 
 template <typename T1, typename T2> struct is_same {
@@ -48,23 +45,21 @@
 
 // More error cases: Prohibit adding the attribute to declarations.
 // Different declarations hit different code paths, so they need separate tests.
-// FIXME: Clang currently generally doesn't prohibit type-only C++11
-// attributes on declarations.
-namespace [[clang::annotate_type("foo")]] my_namespace {}
-struct [[clang::annotate_type("foo")]] S3;
-struct [[clang::annotate_type("foo")]] S3{
-  [[clang::annotate_type("foo")]] int member;
+namespace [[clang::annotate_type("foo")]] my_namespace {} // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+struct [[clang::annotate_type("foo")]] S3; // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+struct [[clang::annotate_type("foo")]] S3{ // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+  [[clang::annotate_type("foo")]] int member; // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
 };
 void f4() {
-  for ([[clang::annotate_type("foo")]] int i = 0; i < 42; ++i) {}
-  for (; [[clang::annotate_type("foo")]] bool b = false;) {}
-  while ([[clang::annotate_type("foo")]] bool b = false) {}
-  if ([[clang::annotate_type("foo")]] bool b = false) {}
+  for ([[clang::annotate_type("foo")]] int i = 0; i < 42; ++i) {} // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+  for (; [[clang::annotate_type("foo")]] bool b = false;) {} // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+  while ([[clang::annotate_type("foo")]] bool b = false) {} // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+  if ([[clang::annotate_type("foo")]] bool b = false) {} // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
   try {
-  } catch ([[clang::annotate_type("foo")]] int i) {
+  } catch ([[clang::annotate_type("foo")]] int i) { // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
   }
 }
 template <class T>
-[[clang::annotate_type("foo")]] T var_template;
-[[clang::annotate_type("foo")]] extern "C" int extern_c_func();
-extern "C" [[clang::annotate_type("foo")]] int extern_c_func();
+[[clang::annotate_type("foo")]] T var_template; // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+[[clang::annotate_type("foo")]] extern "C" int extern_c_func(); // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+extern "C" [[clang::annotate_type("foo")]] int extern_c_func(); // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
Index: clang/test/SemaCXX/address-space-placement.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/address-space-placement.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -fsyntax-only -fcxx-exceptions -verify %s
+
+// Check that we emit the correct warnings in various situations where the C++11
+// spelling of the `address_space` attribute is applied to a declaration instead
+// of a type.
+
+void f([[clang::address_space(1)]] int* param) { // expected-warning {{applying attribute 'address_space' to a declaration is deprecated; apply it to the type instead}}
+  [[clang::address_space(1)]] int* local1; // expected-warning {{applying attribute 'address_space' to a declaration is deprecated; apply it to the type instead}}
+  int* local2 [[clang::address_space(1)]]; // expected-error {{automatic variable qualified with an address space}} expected-warning {{applying attribute 'address_space' to a declaration is deprecated; apply it to the type instead}}
+
+  for ([[clang::address_space(1)]] int* p = nullptr; p; ++p) {} // expected-warning {{applying attribute 'address_space' to a declaration is deprecated; apply it to the type instead}}
+  for (; [[clang::address_space(1)]] int* p = nullptr; ) {} // expected-warning {{applying attribute 'address_space' to a declaration is deprecated; apply it to the type instead}}
+  while([[clang::address_space(1)]] int* p = nullptr) {} // expected-warning {{applying attribute 'address_space' to a declaration is deprecated; apply it to the type instead}}
+  if ([[clang::address_space(1)]] int* p = nullptr) {} // expected-warning {{applying attribute 'address_space' to a declaration is deprecated; apply it to the type instead}}
+  try {
+  } catch([[clang::address_space(1)]] int& i) { // expected-warning {{applying attribute 'address_space' to a declaration is deprecated; apply it to the type instead}}
+  }
+}
+
+[[clang::address_space(1)]] int* return_value(); // expected-warning {{applying attribute 'address_space' to a declaration is deprecated; apply it to the type instead}}
+
+[[clang::address_space(1)]] int global1; // expected-warning {{applying attribute 'address_space' to a declaration is deprecated; apply it to the type instead}}
+int global2 [[clang::address_space(1)]]; // expected-warning {{applying attribute 'address_space' to a declaration is deprecated; apply it to the type instead}}
+
+struct [[clang::address_space(1)]] S { // expected-warning {{applying attribute 'address_space' to a declaration is deprecated; apply it to the type instead}}
+  [[clang::address_space(1)]] int* member_function(); // expected-warning {{applying attribute 'address_space' to a declaration is deprecated; apply it to the type instead}}
+};
+
+template <class T>
+[[clang::address_space(1)]] T var_template; // expected-warning {{applying attribute 'address_space' to a declaration is deprecated; apply it to the type instead}}
+
+using void_ptr [[clang::address_space(1)]] = void *; // expected-warning {{applying attribute 'address_space' to a declaration is deprecated; apply it to the type instead}}
Index: clang/test/Sema/annotate-type.c
===================================================================
--- clang/test/Sema/annotate-type.c
+++ clang/test/Sema/annotate-type.c
@@ -17,11 +17,8 @@
   int *__attribute__((annotate_type("bar"))) y2; // expected-warning {{unknown attribute 'annotate_type' ignored}}
 
   // Various error cases
-  // FIXME: We would want to prohibit the attribute on the following two lines.
-  // However, Clang currently generally doesn't prohibit type-only C++11
-  // attributes on declarations. This should be fixed more generally.
-  [[clang::annotate_type("bar")]] int *z1;
-  int *z2 [[clang::annotate_type("bar")]];
+  [[clang::annotate_type("bar")]] int *z1; // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+  int *z2 [[clang::annotate_type("bar")]]; // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
   [[clang::annotate_type("bar")]]; // expected-error {{'annotate_type' attribute cannot be applied to a statement}}
   int *[[clang::annotate_type(1)]] z3; // expected-error {{'annotate_type' attribute requires a string}}
   int *[[clang::annotate_type()]] z4; // expected-error {{'annotate_type' attribute takes at least 1 argument}}
@@ -33,15 +30,13 @@
 }
 // More error cases: Prohibit adding the attribute to declarations.
 // Different declarations hit different code paths, so they need separate tests.
-// FIXME: Clang currently generally doesn't prohibit type-only C++11
-// attributes on declarations.
-[[clang::annotate_type("bar")]] int *global;
-void annotated_function([[clang::annotate_type("bar")]] int);
-void g([[clang::annotate_type("bar")]] int);
-struct [[clang::annotate_type("foo")]] S;
-struct [[clang::annotate_type("foo")]] S{
-  [[clang::annotate_type("foo")]] int member;
-  [[clang::annotate_type("foo")]] union {
+[[clang::annotate_type("bar")]] int *global; // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+[[clang::annotate_type("bar")]] void annotated_function(); // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+void g([[clang::annotate_type("bar")]] int); // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+struct [[clang::annotate_type("foo")]] S; // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+struct [[clang::annotate_type("foo")]] S{ // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+  [[clang::annotate_type("foo")]] int member; // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+  [[clang::annotate_type("foo")]] union { // expected-error {{an attribute list cannot appear here}}
     int i;
     float f;
   };
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -8312,15 +8312,15 @@
 /// ProcessDeclAttribute - Apply the specific attribute to the specified decl if
 /// the attribute applies to decls.  If the attribute is a type attribute, just
 /// silently ignore it if a GNU attribute.
-static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D,
-                                 const ParsedAttr &AL,
-                                 bool IncludeCXX11Attributes) {
+static void
+ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
+                     const Sema::ProcessDeclAttributeOptions &Options) {
   if (AL.isInvalid() || AL.getKind() == ParsedAttr::IgnoredAttribute)
     return;
 
   // Ignore C++11 attributes on declarator chunks: they appertain to the type
   // instead.
-  if (AL.isCXX11Attribute() && !IncludeCXX11Attributes)
+  if (AL.isCXX11Attribute() && !Options.IncludeCXX11Attributes)
     return;
 
   // Unknown attributes are automatically warned on. Target-specific attributes
@@ -8353,14 +8353,29 @@
     if (AL.getInfo().handleDeclAttribute(S, D, AL) != ParsedAttrInfo::NotHandled)
       break;
     if (!AL.isStmtAttr()) {
-      // Type attributes are handled elsewhere; silently move on.
       assert(AL.isTypeAttr() && "Non-type attribute not handled");
-      break;
+    }
+    if (AL.isTypeAttr()) {
+      if (Options.IgnoreTypeAttributes)
+        break;
+      if (AL.slidesFromDeclToDeclSpec()) {
+        if (AL.isStandardAttributeSyntax() && AL.isClangScope()) {
+          // For standard syntax attributes, which would normally appertain to
+          // the declaration here, suggest moving them to the type instead. But
+          // only do this for our own vendor attributes; moving other vendors'
+          // attributes might hurt portability.
+          S.Diag(AL.getLoc(), diag::warn_type_attribute_deprecated_on_decl)
+              << AL << D->getLocation();
+        }
+        // GNU type attributes are handled elsewhere; silently move on.
+        break;
+      }
     }
     // N.B., ClangAttrEmitter.cpp emits a diagnostic helper that ensures a
     // statement attribute is not written on a declaration, but this code is
-    // needed for attributes in Attr.td that do not list any subjects.
-    S.Diag(AL.getLoc(), diag::err_stmt_attribute_invalid_on_decl)
+    // needed for type attributes as well as statement attributes in Attr.td
+    // that do not list any subjects.
+    S.Diag(AL.getLoc(), diag::err_attribute_invalid_on_decl)
         << AL << D->getLocation();
     break;
   case ParsedAttr::AT_Interrupt:
@@ -9004,14 +9019,14 @@
 
 /// ProcessDeclAttributeList - Apply all the decl attributes in the specified
 /// attribute list to the specified decl, ignoring any type attributes.
-void Sema::ProcessDeclAttributeList(Scope *S, Decl *D,
-                                    const ParsedAttributesView &AttrList,
-                                    bool IncludeCXX11Attributes) {
+void Sema::ProcessDeclAttributeList(
+    Scope *S, Decl *D, const ParsedAttributesView &AttrList,
+    const ProcessDeclAttributeOptions &Options) {
   if (AttrList.empty())
     return;
 
   for (const ParsedAttr &AL : AttrList)
-    ProcessDeclAttribute(*this, S, D, AL, IncludeCXX11Attributes);
+    ProcessDeclAttribute(*this, S, D, AL, Options);
 
   // FIXME: We should be able to handle these cases in TableGen.
   // GCC accepts
@@ -9099,7 +9114,9 @@
     AccessSpecDecl *ASDecl, const ParsedAttributesView &AttrList) {
   for (const ParsedAttr &AL : AttrList) {
     if (AL.getKind() == ParsedAttr::AT_Annotate) {
-      ProcessDeclAttribute(*this, nullptr, ASDecl, AL, AL.isCXX11Attribute());
+      ProcessDeclAttributeOptions Options;
+      Options.IncludeCXX11Attributes = AL.isCXX11Attribute();
+      ProcessDeclAttribute(*this, nullptr, ASDecl, AL, Options);
     } else {
       Diag(AL.getLoc(), diag::err_only_annotate_after_access_spec);
       return true;
@@ -9241,16 +9258,22 @@
 /// specified in many different places, and we need to find and apply them all.
 void Sema::ProcessDeclAttributes(Scope *S, Decl *D, const Declarator &PD) {
   // Apply decl attributes from the DeclSpec if present.
-  if (!PD.getDeclSpec().getAttributes().empty())
-    ProcessDeclAttributeList(S, D, PD.getDeclSpec().getAttributes());
+  if (!PD.getDeclSpec().getAttributes().empty()) {
+    ProcessDeclAttributeList(
+        S, D, PD.getDeclSpec().getAttributes(),
+        ProcessDeclAttributeOptions().WithIgnoreTypeAttributes(true));
+  }
 
   // Walk the declarator structure, applying decl attributes that were in a type
   // position to the decl itself.  This handles cases like:
   //   int *__attr__(x)** D;
   // when X is a decl attribute.
-  for (unsigned i = 0, e = PD.getNumTypeObjects(); i != e; ++i)
+  for (unsigned i = 0, e = PD.getNumTypeObjects(); i != e; ++i) {
     ProcessDeclAttributeList(S, D, PD.getTypeObject(i).getAttrs(),
-                             /*IncludeCXX11Attributes=*/false);
+                             ProcessDeclAttributeOptions()
+                                 .WithIncludeCXX11Attributes(false)
+                                 .WithIgnoreTypeAttributes(true));
+  }
 
   // Finally, apply any attributes on the decl itself.
   ProcessDeclAttributeList(S, D, PD.getAttributes());
Index: clang/lib/Sema/ParsedAttr.cpp
===================================================================
--- clang/lib/Sema/ParsedAttr.cpp
+++ clang/lib/Sema/ParsedAttr.cpp
@@ -212,6 +212,38 @@
   return getInfo().IsSupportedByPragmaAttribute;
 }
 
+bool ParsedAttr::slidesFromDeclToDeclSpec() const {
+  if (!isStandardAttributeSyntax())
+    return true;
+
+  // We have historically allowed some attributes with standard attribute syntax
+  // to slide to the decl-specifier-seq, so we have to keep supporting it. This
+  // property is consciously not defined as a flag in Attr.td because we don't
+  // want new attributes to specify it.
+  switch (getParsedKind()) {
+  case AT_AddressSpace:
+  case AT_OpenCLPrivateAddressSpace:
+  case AT_OpenCLGlobalAddressSpace:
+  case AT_OpenCLGlobalDeviceAddressSpace:
+  case AT_OpenCLGlobalHostAddressSpace:
+  case AT_OpenCLLocalAddressSpace:
+  case AT_OpenCLConstantAddressSpace:
+  case AT_OpenCLGenericAddressSpace:
+  case AT_NeonPolyVectorType:
+  case AT_NeonVectorType:
+  case AT_ArmMveStrictPolymorphism:
+  case AT_BTFTypeTag:
+  case AT_Regparm:
+  case AT_NoDeref:
+  case AT_ObjCGC:
+  case AT_VectorSize:
+  case AT_MatrixType:
+    return true;
+  default:
+    return false;
+  }
+}
+
 bool ParsedAttr::acceptsExprPack() const { return getInfo().AcceptsExprPack; }
 
 unsigned ParsedAttr::getSemanticSpelling() const {
Index: clang/lib/Parse/Parser.cpp
===================================================================
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -1112,8 +1112,6 @@
     return Actions.ConvertDeclToDeclGroup(TheDecl);
   }
 
-  DS.takeAttributesFrom(Attrs);
-
   // ObjC2 allows prefix attributes on class interfaces and protocols.
   // FIXME: This still needs better diagnostics. We should only accept
   // attributes here, no types, etc.
@@ -1128,6 +1126,7 @@
     }
 
     DS.abort();
+    DS.takeAttributesFrom(Attrs);
 
     const char *PrevSpec = nullptr;
     unsigned DiagID;
@@ -1145,17 +1144,21 @@
             ParseObjCAtInterfaceDeclaration(AtLoc, DS.getAttributes()));
   }
 
+  ParsedAttributes DeclAttrs(AttrFactory);
+  ExtractDefiniteDeclAttrs(Attrs, DeclAttrs);
+  DS.takeAttributesFrom(Attrs);
+
   // If the declspec consisted only of 'extern' and we have a string
   // literal following it, this must be a C++ linkage specifier like
   // 'extern "C"'.
   if (getLangOpts().CPlusPlus && isTokenStringLiteral() &&
       DS.getStorageClassSpec() == DeclSpec::SCS_extern &&
       DS.getParsedSpecifiers() == DeclSpec::PQ_StorageClassSpecifier) {
-    Decl *TheDecl = ParseLinkage(DS, DeclaratorContext::File);
+    Decl *TheDecl = ParseLinkage(DS, DeclaratorContext::File, DeclAttrs);
     return Actions.ConvertDeclToDeclGroup(TheDecl);
   }
 
-  return ParseDeclGroup(DS, DeclaratorContext::File);
+  return ParseDeclGroup(DS, DeclaratorContext::File, DeclAttrs);
 }
 
 Parser::DeclGroupPtrTy Parser::ParseDeclarationOrFunctionDefinition(
Index: clang/lib/Parse/ParseTemplate.cpp
===================================================================
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -239,6 +239,9 @@
     return Decl;
   }
 
+  ParsedAttributes DeclAttrs(AttrFactory);
+  ExtractDefiniteDeclAttrs(prefixAttrs, DeclAttrs);
+
   // Move the attributes from the prefix into the DS.
   if (TemplateInfo.Kind == ParsedTemplateInfo::ExplicitInstantiation)
     ProhibitAttributes(prefixAttrs);
@@ -247,6 +250,7 @@
 
   // Parse the declarator.
   ParsingDeclarator DeclaratorInfo(*this, DS, (DeclaratorContext)Context);
+  DeclaratorInfo.takeAttributes(DeclAttrs);
   if (TemplateInfo.TemplateParams)
     DeclaratorInfo.setTemplateParameterLists(*TemplateInfo.TemplateParams);
 
Index: clang/lib/Parse/ParseStmt.cpp
===================================================================
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -2583,6 +2583,9 @@
     ParsedAttributes Attributes(AttrFactory);
     MaybeParseCXX11Attributes(Attributes);
 
+    ParsedAttributes DeclAttrs(AttrFactory);
+    ExtractDefiniteDeclAttrs(Attributes, DeclAttrs);
+
     DeclSpec DS(AttrFactory);
     DS.takeAttributesFrom(Attributes);
 
@@ -2590,6 +2593,7 @@
       return StmtError();
 
     Declarator ExDecl(DS, DeclaratorContext::CXXCatch);
+    ExDecl.takeAttributes(DeclAttrs);
     ParseDeclarator(ExDecl);
     ExceptionDecl = Actions.ActOnExceptionDeclarator(getCurScope(), ExDecl);
   } else
Index: clang/lib/Parse/ParseExprCXX.cpp
===================================================================
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -2077,6 +2077,9 @@
   // If this is a for loop, we're entering its condition.
   ForConditionScope.enter(/*IsConditionVariable=*/true);
 
+  ParsedAttributes DeclAttrs(AttrFactory);
+  ExtractDefiniteDeclAttrs(attrs, DeclAttrs);
+
   // type-specifier-seq
   DeclSpec DS(AttrFactory);
   DS.takeAttributesFrom(attrs);
@@ -2084,6 +2087,7 @@
 
   // declarator
   Declarator DeclaratorInfo(DS, DeclaratorContext::Condition);
+  DeclaratorInfo.takeAttributes(DeclAttrs);
   ParseDeclarator(DeclaratorInfo);
 
   // simple-asm-expr[opt]
Index: clang/lib/Parse/ParseDeclCXX.cpp
===================================================================
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -345,7 +345,8 @@
 ///         'extern' string-literal '{' declaration-seq[opt] '}'
 ///         'extern' string-literal declaration
 ///
-Decl *Parser::ParseLinkage(ParsingDeclSpec &DS, DeclaratorContext Context) {
+Decl *Parser::ParseLinkage(ParsingDeclSpec &DS, DeclaratorContext Context,
+                           ParsedAttributes &DeclAttrs) {
   assert(isTokenStringLiteral() && "Not a string literal!");
   ExprResult Lang = ParseStringLiteralExpression(false);
 
@@ -357,8 +358,7 @@
                 getCurScope(), DS.getSourceRange().getBegin(), Lang.get(),
                 Tok.is(tok::l_brace) ? Tok.getLocation() : SourceLocation());
 
-  ParsedAttributes attrs(AttrFactory);
-  MaybeParseCXX11Attributes(attrs);
+  MaybeParseCXX11Attributes(DeclAttrs);
 
   if (Tok.isNot(tok::l_brace)) {
     // Reset the source range in DS, as the leading "extern"
@@ -367,7 +367,7 @@
     DS.SetRangeEnd(SourceLocation());
     // ... but anyway remember that such an "extern" was seen.
     DS.setExternInLinkageSpec(true);
-    ParseExternalDeclaration(attrs, &DS);
+    ParseExternalDeclaration(DeclAttrs, &DS);
     return LinkageSpec ? Actions.ActOnFinishLinkageSpecification(
                              getCurScope(), LinkageSpec, SourceLocation())
                        : nullptr;
@@ -375,7 +375,7 @@
 
   DS.abort();
 
-  ProhibitAttributes(attrs);
+  ProhibitAttributes(DeclAttrs);
 
   BalancedDelimiterTracker T(*this, tok::l_brace);
   T.consumeOpen();
@@ -2690,11 +2690,6 @@
   if (Tok.is(tok::annot_attr_openmp))
     return ParseOpenMPDeclarativeDirectiveWithExtDecl(AS, attrs);
 
-  // We need to keep these attributes for future diagnostic
-  // before they are taken over by declaration specifier.
-  FnAttrs.addAll(attrs.begin(), attrs.end());
-  FnAttrs.Range = attrs.Range;
-
   MaybeParseMicrosoftAttributes(attrs);
 
   if (Tok.is(tok::kw_using)) {
@@ -2725,6 +2720,27 @@
   // decl-specifier-seq:
   // Parse the common declaration-specifiers piece.
   ParsingDeclSpec DS(*this, TemplateDiags);
+  ParsedAttributes DeclAttrs(AttrFactory);
+  ExtractDefiniteDeclAttrs(attrs, DeclAttrs);
+
+  // We need to keep these attributes for future diagnostic
+  // before they are taken over by declaration specifier.
+  FnAttrs.addAll(attrs.begin(), attrs.end());
+  // Update the `SourceRange` for `FnAttrs` because we will need that below to
+  // decide whether any attributes were specified and whether we should
+  // therefore issue a diagnostic.
+  // There's an important special case here: If the code contained an empty
+  // C++11 attribute-specifier-seq (`[[]]`), `attrs` will be empty, but
+  // `attrs.Range` will not, and we similarly want FnAttrs.Range to be
+  // non-empty. We no longer know whether `attrs` was initally empty, but if it
+  // was, `DeclAttrs` will definitely be empty, and it's always safe to use
+  // `attrs.Range` in this case.
+  if (DeclAttrs.empty()) {
+    FnAttrs.Range = attrs.Range;
+  } else {
+    FnAttrs.updateRange();
+  }
+
   DS.takeAttributesFrom(attrs);
   if (MalformedTypeSpec)
     DS.SetTypeSpecError();
@@ -2777,6 +2793,8 @@
   }
 
   ParsingDeclarator DeclaratorInfo(*this, DS, DeclaratorContext::Member);
+  AttributeLender AttrLender;
+  AttrLender.lend(DeclAttrs, DeclaratorInfo);
   if (TemplateInfo.TemplateParams)
     DeclaratorInfo.setTemplateParameterLists(TemplateParams);
   VirtSpecifiers VS;
@@ -3075,7 +3093,9 @@
     }
 
     // Parse the next declarator.
+    AttrLender.giveBack(DeclAttrs, DeclaratorInfo);
     DeclaratorInfo.clear();
+    AttrLender.lend(DeclAttrs, DeclaratorInfo);
     VS.clear();
     BitfieldSize = ExprResult(/*Invalid=*/false);
     EqualLoc = PureSpecLoc = SourceLocation();
Index: clang/lib/Parse/ParseDecl.cpp
===================================================================
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -1732,6 +1732,30 @@
   }
 }
 
+// Given attributes `Attrs` that were specified ahead of a declaration,
+// identifies those attributes that definitely apply to the declaration itself
+// (not to the decl-specifier-seq) and moves them to `DeclAttrs`.
+void Parser::ExtractDefiniteDeclAttrs(ParsedAttributes &Attrs,
+                                      ParsedAttributes &DeclAttrs) {
+  llvm::SmallVector<ParsedAttr *, 1> ToBeMoved;
+  for (ParsedAttr &AL : Attrs) {
+    if (AL.slidesFromDeclToDeclSpec()) {
+      // For standard syntax attributes, which would normally appertain to the
+      // declaration here, suggest moving them to the type instead. But only do
+      // this for our own vendor attributes; moving other vendors' attributes
+      // might hurt portability.
+      if (AL.isStandardAttributeSyntax() && AL.isClangScope()) {
+        Diag(AL.getLoc(), diag::warn_type_attribute_deprecated_on_decl) << AL;
+      }
+    } else {
+      ToBeMoved.push_back(&AL);
+    }
+  }
+  for (ParsedAttr *AL : ToBeMoved) {
+    DeclAttrs.takeOneFrom(Attrs, AL);
+  }
+}
+
 /// ParseDeclaration - Parse a full 'declaration', which consists of
 /// declaration-specifiers, some number of declarators, and a semicolon.
 /// 'Context' should be a DeclaratorContext value.  This returns the
@@ -1850,8 +1874,10 @@
   if (DeclSpecStart)
     DS.SetRangeStart(*DeclSpecStart);
 
+  ParsedAttributes DeclAttrs(AttrFactory);
+  ExtractDefiniteDeclAttrs(Attrs, DeclAttrs);
   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
@@ -2006,10 +2032,13 @@
 /// result.
 Parser::DeclGroupPtrTy Parser::ParseDeclGroup(ParsingDeclSpec &DS,
                                               DeclaratorContext Context,
+                                              ParsedAttributes &Attrs,
                                               SourceLocation *DeclEnd,
                                               ForRangeInit *FRI) {
   // Parse the first declarator.
   ParsingDeclarator D(*this, DS, Context);
+  AttributeLender AttrLender;
+  AttrLender.lend(Attrs, D);
   ParseDeclarator(D);
 
   // Bail out if the first declarator didn't seem well-formed.
@@ -2172,7 +2201,9 @@
     }
 
     // Parse the next declarator.
+    AttrLender.giveBack(Attrs, D);
     D.clear();
+    AttrLender.lend(Attrs, D);
     D.setCommaLoc(CommaLoc);
 
     // Accept attributes in an init-declarator.  In the first declarator in a
@@ -4300,6 +4331,8 @@
   // Parse leading attributes.
   ParsedAttributes Attrs(AttrFactory);
   MaybeParseCXX11Attributes(Attrs);
+  ParsedAttributes DeclAttrs(AttrFactory);
+  ExtractDefiniteDeclAttrs(Attrs, DeclAttrs);
   DS.takeAttributesFrom(Attrs);
 
   // Parse the common specifier-qualifiers-list piece.
@@ -4308,6 +4341,12 @@
   // 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 draft 6.7.2.1/9 : "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."
+    DeclAttrs.updateRange();
+    ProhibitAttributes(DeclAttrs);
     RecordDecl *AnonRecord = nullptr;
     Decl *TheDecl = Actions.ParsedFreeStandingDeclSpec(getCurScope(), AS_none,
                                                        DS, AnonRecord);
@@ -4321,6 +4360,8 @@
   SourceLocation CommaLoc;
   while (true) {
     ParsingFieldDeclarator DeclaratorInfo(*this, DS);
+    AttributeLender AttrLender;
+    AttrLender.lend(DeclAttrs, DeclaratorInfo.D);
     DeclaratorInfo.D.setCommaLoc(CommaLoc);
 
     // Attributes are only allowed here on successive declarators.
@@ -4361,6 +4402,8 @@
       return;
 
     FirstDeclarator = false;
+
+    AttrLender.giveBack(DeclAttrs, DeclaratorInfo.D);
   }
 }
 
@@ -6953,20 +6996,26 @@
     // Just use the ParsingDeclaration "scope" of the declarator.
     DeclSpec DS(AttrFactory);
 
-    // Parse any C++11 attributes.
-    MaybeParseCXX11Attributes(DS.getAttributes());
-
-    // Skip any Microsoft attributes before a param.
-    MaybeParseMicrosoftAttributes(DS.getAttributes());
-
-    SourceLocation DSStart = Tok.getLocation();
+    ParsedAttributes ArgAttrs(AttrFactory);
 
     // 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);
+    ArgAttrs.takeAllFrom(FirstArgAttrs);
+
+    // Parse any C++11 attributes.
+    MaybeParseCXX11Attributes(ArgAttrs);
+
+    // Skip any Microsoft attributes before a param.
+    MaybeParseMicrosoftAttributes(ArgAttrs);
+
+    ParsedAttributes DeclAttrs(AttrFactory);
+    ExtractDefiniteDeclAttrs(ArgAttrs, DeclAttrs);
+    DS.takeAttributesFrom(ArgAttrs);
+
+    SourceLocation DSStart = Tok.getLocation();
 
     ParseDeclarationSpecifiers(DS);
 
@@ -6980,6 +7029,7 @@
                 : DeclaratorCtx == DeclaratorContext::LambdaExpr
                       ? DeclaratorContext::LambdaExprParameter
                       : DeclaratorContext::Prototype);
+    ParmDeclarator.takeAttributes(DeclAttrs);
     ParseDeclarator(ParmDeclarator);
 
     // Parse GNU attributes, if present.
Index: clang/lib/Basic/Attributes.cpp
===================================================================
--- clang/lib/Basic/Attributes.cpp
+++ clang/lib/Basic/Attributes.cpp
@@ -85,6 +85,10 @@
   return ScopeName && (ScopeName->isStr("gnu") || ScopeName->isStr("__gnu__"));
 }
 
+bool AttributeCommonInfo::isClangScope() const {
+  return ScopeName && (ScopeName->isStr("clang") || ScopeName->isStr("_Clang"));
+}
+
 #include "clang/Sema/AttrParsedAttrKinds.inc"
 
 static SmallString<64> normalizeName(const IdentifierInfo *Name,
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -4423,8 +4423,38 @@
   // Helper for delayed processing of attributes.
   void ProcessDeclAttributeDelayed(Decl *D,
                                    const ParsedAttributesView &AttrList);
-  void ProcessDeclAttributeList(Scope *S, Decl *D, const ParsedAttributesView &AL,
-                             bool IncludeCXX11Attributes = true);
+
+  // Options for ProcessDeclAttributeList().
+  struct ProcessDeclAttributeOptions {
+    ProcessDeclAttributeOptions()
+        : IncludeCXX11Attributes(true), IgnoreTypeAttributes(false) {}
+
+    ProcessDeclAttributeOptions WithIncludeCXX11Attributes(bool Val) {
+      ProcessDeclAttributeOptions Result = *this;
+      Result.IncludeCXX11Attributes = Val;
+      return Result;
+    }
+
+    ProcessDeclAttributeOptions WithIgnoreTypeAttributes(bool Val) {
+      ProcessDeclAttributeOptions Result = *this;
+      Result.IgnoreTypeAttributes = Val;
+      return Result;
+    }
+
+    // Should C++11 attributes be processed?
+    bool IncludeCXX11Attributes;
+
+    // Should any type attributes encountered be ignored?
+    // If this option is false, a diagnostic will be emitted for any type
+    // attributes of a kind that does not "slide" from the declaration to
+    // the decl-specifier-seq.
+    bool IgnoreTypeAttributes;
+  };
+
+  void ProcessDeclAttributeList(Scope *S, Decl *D,
+                                const ParsedAttributesView &AttrList,
+                                const ProcessDeclAttributeOptions &Options =
+                                    ProcessDeclAttributeOptions());
   bool ProcessAccessDeclAttributeList(AccessSpecDecl *ASDecl,
                                    const ParsedAttributesView &AttrList);
 
Index: clang/include/clang/Sema/ParsedAttr.h
===================================================================
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -651,6 +651,16 @@
   bool isKnownToGCC() const;
   bool isSupportedByPragmaAttribute() const;
 
+  /// Returns whether the attribute, if specified ahead of a declaration,
+  /// should be applied to the decl-specifier-seq instead (i.e. whether it
+  /// "slides" to the decl-specifier-seq).
+  /// Attributes with GNU, __declspec or keyword syntax generally slide
+  /// to the decl-specifier-seq. C++11 attributes specified ahead of the
+  /// declaration always appertain to the declaration according to the standard,
+  /// but historically we have allowed some of these attributes to slide to
+  /// the decl-specifier-seq too, so we need to keep supporting this behavior.
+  bool slidesFromDeclToDeclSpec() const;
+
   /// If the parsed attribute has a semantic equivalent, and it would
   /// have a semantic Spelling enumeration (due to having semantically-distinct
   /// spelling variations), return the value of that semantic spelling. If the
@@ -906,6 +916,22 @@
   ParsedAttr &operator[](SizeType pos) { return *AttrList[pos]; }
   const ParsedAttr &operator[](SizeType pos) const { return *AttrList[pos]; }
 
+  void updateRange() {
+    Range = SourceRange();
+    for (const ParsedAttr &PA : *this) {
+      if (Range.isInvalid()) {
+        Range = PA.getRange();
+        continue;
+      }
+      if (PA.getRange().getBegin() < Range.getBegin()) {
+        Range.setBegin(PA.getRange().getBegin());
+      }
+      if (PA.getRange().getEnd() > Range.getEnd()) {
+        Range.setEnd(PA.getRange().getEnd());
+      }
+    }
+  }
+
   void addAtEnd(ParsedAttr *newAttr) {
     assert(newAttr);
     AttrList.push_back(newAttr);
Index: clang/include/clang/Parse/RAIIObjectsForParser.h
===================================================================
--- clang/include/clang/Parse/RAIIObjectsForParser.h
+++ clang/include/clang/Parse/RAIIObjectsForParser.h
@@ -459,6 +459,35 @@
     }
     void skipToEnd();
   };
+
+  // Helper class that allows "lending" a list of attributes to a Declarator,
+  // then taking it back later.
+  // This is not strictly an RAII class, as the destructor does not
+  // automatically return the attributes, but it is related in spirit to the
+  // other classes in this file.
+  class AttributeLender {
+  public:
+    // Transfers all attributes in `Attrs` to `D` and records which attributes
+    // were transferred.
+    void lend(ParsedAttributes &Attrs, Declarator &D) {
+      for (ParsedAttr &AL : Attrs) {
+        LentAttrs.push_back(&AL);
+      }
+      D.takeAttributes(Attrs);
+    }
+
+    // Takes the attributes that a previous call to `lend` transferred to `D`
+    // and transfers them back to `Attrs`.
+    void giveBack(ParsedAttributes &Attrs, Declarator &D) {
+      for (ParsedAttr *AL : LentAttrs) {
+        Attrs.takeOneFrom(D.getAttributes(), AL);
+      }
+      LentAttrs.clear();
+    }
+
+  private:
+    llvm::SmallVector<ParsedAttr *, 1> LentAttrs;
+  };
 } // end namespace clang
 
 #endif
Index: clang/include/clang/Parse/Parser.h
===================================================================
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -2316,6 +2316,7 @@
                          SourceLocation *DeclSpecStart = nullptr);
   bool MightBeDeclarator(DeclaratorContext Context);
   DeclGroupPtrTy ParseDeclGroup(ParsingDeclSpec &DS, DeclaratorContext Context,
+                                ParsedAttributes &Attrs,
                                 SourceLocation *DeclEnd = nullptr,
                                 ForRangeInit *FRI = nullptr);
   Decl *ParseDeclarationAfterDeclarator(Declarator &D,
@@ -2611,6 +2612,9 @@
   void stripTypeAttributesOffDeclSpec(ParsedAttributes &Attrs, DeclSpec &DS,
                                       Sema::TagUseKind TUK);
 
+  void ExtractDefiniteDeclAttrs(ParsedAttributes &Attrs,
+                                ParsedAttributes &DeclAttrs);
+
   // FixItLoc = possible correct location for the attributes
   void ProhibitAttributes(ParsedAttributes &Attrs,
                           SourceLocation FixItLoc = SourceLocation()) {
@@ -3015,7 +3019,8 @@
                            unsigned int index, SourceLocation &InlineLoc,
                            ParsedAttributes &attrs,
                            BalancedDelimiterTracker &Tracker);
-  Decl *ParseLinkage(ParsingDeclSpec &DS, DeclaratorContext Context);
+  Decl *ParseLinkage(ParsingDeclSpec &DS, DeclaratorContext Context,
+                     ParsedAttributes &DeclAttrs);
   Decl *ParseExportDeclaration();
   DeclGroupPtrTy ParseUsingDirectiveOrDeclaration(
       DeclaratorContext Context, const ParsedTemplateInfo &TemplateInfo,
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3381,8 +3381,11 @@
   "annotating the 'if %select{constexpr|consteval}0' statement here">;
 def err_decl_attribute_invalid_on_stmt : Error<
   "%0 attribute cannot be applied to a statement">;
-def err_stmt_attribute_invalid_on_decl : Error<
+def err_attribute_invalid_on_decl : Error<
   "%0 attribute cannot be applied to a declaration">;
+def warn_type_attribute_deprecated_on_decl : Warning<
+  "applying attribute %0 to a declaration is deprecated; apply it to the type instead">,
+  InGroup<DeprecatedAttributes>;
 def warn_declspec_attribute_ignored : Warning<
   "attribute %0 is ignored, place it after "
   "\"%select{class|struct|interface|union|enum}1\" to apply attribute to "
Index: clang/include/clang/Basic/AttributeCommonInfo.h
===================================================================
--- clang/include/clang/Basic/AttributeCommonInfo.h
+++ clang/include/clang/Basic/AttributeCommonInfo.h
@@ -146,6 +146,7 @@
   bool isMicrosoftAttribute() const { return SyntaxUsed == AS_Microsoft; }
 
   bool isGNUScope() const;
+  bool isClangScope() const;
 
   bool isAlignasAttribute() const {
     // FIXME: Use a better mechanism to determine this.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to