Allow writing calling convention attributes on function types

[aaron.ballman]

Summary

Calling convention attributes notionally appertain to the function type -- they modify the mangling of the function, change the behavior of assignment operations, etc. However, they're not currently allowed to be written in the type position within a function declaration. This patch allows the calling convention attributes to be written in the type position as well as the declaration position. This also adds a new diagnostic to diagnose this as being incompatible with GCC despite the attribute being in the gnu namespace. I do not think this incompatibility is sufficient to warrant adding a clang namespace for the attributes, but that is another option available to us if there is concern over the incompatibility.

Eventually, I would like to extend this to other attributes that really should be type attributes, such as enable_if. The calling convention attributes just happen to be the lowest hanging fruit.

[aaron.ballman]

Ping

[aaron.ballman]

Ping.

[aaron.ballman]

Ping

[aaron.ballman]

Ping x4

[rsmith]

I'm really sad about this; C++11 attributes were supposed to fix the undisciplined "do what I mean" behavior of GNU attributes. But you're right, these really are type attributes, and really should be permitted to appertain to types. That we permit these to appertain to function declarations is the mistake, but it's a mistake we need to make to match GCC, so while I'm sad about it, I think this patch is the right thing.

[aaron.ballman]

In D43750#1085780, @rsmith wrote:

I'm really sad about this; C++11 attributes were supposed to fix the undisciplined "do what I mean" behavior of GNU attributes. But you're right, these really are type attributes, and really should be permitted to appertain to types. That we permit these to appertain to function declarations is the mistake, but it's a mistake we need to make to match GCC, so while I'm sad about it, I think this patch is the right thing.

Yeah, this situation doesn't make me overly happy but I think it's the best way for us to move forward. Hopefully, after enough time has passed, we can consider deprecating writing these attributes on declarations with the eventual goal of disallowing it.

Committed in r331459.

include/clang/Basic/Attr.td

 
 /// An inheritable attribute is inherited by later redeclarations.
 class InheritableAttr : Attr {
   // Set to true if this attribute can be duplicated on a subject when inheriting
   // attributes from prior declarations.
   bit InheritEvenIfAlreadyPresent = 0;
 }
 
+/// Some attributes, like calling conventions, can appear in either the
+/// declaration or the type position. These attributes are morally type
+/// attributes, but have historically been written on declarations.
+class DeclOrTypeAttr : InheritableAttr;
+
 /// A target-specific attribute.  This class is meant to be used as a mixin
 /// with InheritableAttr or Attr depending on the attribute's needs.
 class TargetSpecificAttr<TargetSpec target> {
   TargetSpec Target = target;
   // Attributes are generally required to have unique spellings for their names
   // so that the parser can determine what kind of attribute it has parsed.
   // However, target-specific attributes are special in that the attribute only
   // "exists" for a given target. So two target-specific attributes can share

 
 def CarriesDependency : InheritableParamAttr {
   let Spellings = [GNU<"carries_dependency">,
                    CXX11<"","carries_dependency", 200809>];
   let Subjects = SubjectList<[ParmVar, ObjCMethod, Function], ErrorDiag>;
   let Documentation = [CarriesDependencyDocs];
 }
 
-def CDecl : InheritableAttr {
+def CDecl : DeclOrTypeAttr {
   let Spellings = [GCC<"cdecl">, Keyword<"__cdecl">, Keyword<"_cdecl">];
 //  let Subjects = [Function, ObjCMethod];
   let Documentation = [Undocumented];
 }
 
 // cf_audited_transfer indicates that the given function has been
 // audited and has been marked with the appropriate cf_consumed and
 // cf_returns_retained attributes.  It is generally applied by

 
 def FallThrough : StmtAttr {
   let Spellings = [CXX11<"", "fallthrough", 201603>, C2x<"", "fallthrough">,
                    CXX11<"clang", "fallthrough">];
 //  let Subjects = [NullStmt];
   let Documentation = [FallthroughDocs];
 }
 
-def FastCall : InheritableAttr {
+def FastCall : DeclOrTypeAttr {
   let Spellings = [GCC<"fastcall">, Keyword<"__fastcall">,
                    Keyword<"_fastcall">];
 //  let Subjects = [Function, ObjCMethod];
   let Documentation = [FastCallDocs];
 }
 
-def RegCall : InheritableAttr {
+def RegCall : DeclOrTypeAttr {
   let Spellings = [GCC<"regcall">, Keyword<"__regcall">];
   let Documentation = [RegCallDocs];
 }
 
 def Final : InheritableAttr {
   let Spellings = [Keyword<"final">, Keyword<"sealed">];
   let Accessors = [Accessor<"isSpelledAsSealed", [Keyword<"sealed">]>];
   let SemaHandler = 0;

 }
 
 def MayAlias : InheritableAttr {
   // FIXME: this is a type attribute in GCC, but a declaration attribute here.
   let Spellings = [GCC<"may_alias">];
   let Documentation = [Undocumented];
 }
 
-def MSABI : InheritableAttr {
+def MSABI : DeclOrTypeAttr {
   let Spellings = [GCC<"ms_abi">];
 //  let Subjects = [Function, ObjCMethod];
   let Documentation = [MSABIDocs];
 }
 
 def MSP430Interrupt : InheritableAttr, TargetSpecificAttr<TargetMSP430> {
   // NOTE: If you add any additional spellings, ARMInterrupt's, MipsInterrupt's
   // and AnyX86Interrupt's spellings must match.

 }
 
 def Packed : InheritableAttr {
   let Spellings = [GCC<"packed">];
 //  let Subjects = [Tag, Field];
   let Documentation = [Undocumented];
 }
 
-def IntelOclBicc : InheritableAttr {
+def IntelOclBicc : DeclOrTypeAttr {
   let Spellings = [Clang<"intel_ocl_bicc", 0>];
 //  let Subjects = [Function, ObjCMethod];
   let Documentation = [Undocumented];
 }
 
-def Pcs : InheritableAttr {
+def Pcs : DeclOrTypeAttr {
   let Spellings = [GCC<"pcs">];
   let Args = [EnumArgument<"PCS", "PCSType",
                            ["aapcs", "aapcs-vfp"],
                            ["AAPCS", "AAPCS_VFP"]>];
 //  let Subjects = [Function, ObjCMethod];
   let Documentation = [PcsDocs];
 }
 

 def Sentinel : InheritableAttr {
   let Spellings = [GCC<"sentinel">];
   let Args = [DefaultIntArgument<"Sentinel", 0>,
               DefaultIntArgument<"NullPos", 0>];
 //  let Subjects = SubjectList<[Function, ObjCMethod, Block, Var]>;
   let Documentation = [Undocumented];
 }
 
-def StdCall : InheritableAttr {
+def StdCall : DeclOrTypeAttr {
   let Spellings = [GCC<"stdcall">, Keyword<"__stdcall">, Keyword<"_stdcall">];
 //  let Subjects = [Function, ObjCMethod];
   let Documentation = [StdCallDocs];
 }
 
-def SwiftCall : InheritableAttr {
+def SwiftCall : DeclOrTypeAttr {
   let Spellings = [Clang<"swiftcall">];
 //  let Subjects = SubjectList<[Function]>;
   let Documentation = [SwiftCallDocs];
 }
 
 def SwiftContext : ParameterABIAttr {
   let Spellings = [Clang<"swift_context">];
   let Documentation = [SwiftContextDocs];

 }
 
 def Suppress : StmtAttr {
   let Spellings = [CXX11<"gsl", "suppress">];
   let Args = [VariadicStringArgument<"DiagnosticIdentifiers">];
   let Documentation = [SuppressDocs];
 }
 
-def SysVABI : InheritableAttr {
+def SysVABI : DeclOrTypeAttr {
   let Spellings = [GCC<"sysv_abi">];
 //  let Subjects = [Function, ObjCMethod];
   let Documentation = [Undocumented];
 }
 
-def ThisCall : InheritableAttr {
+def ThisCall : DeclOrTypeAttr {
   let Spellings = [GCC<"thiscall">, Keyword<"__thiscall">,
                    Keyword<"_thiscall">];
 //  let Subjects = [Function, ObjCMethod];
   let Documentation = [ThisCallDocs];
 }
 
-def VectorCall : InheritableAttr {
+def VectorCall : DeclOrTypeAttr {
   let Spellings = [Clang<"vectorcall">, Keyword<"__vectorcall">,
                    Keyword<"_vectorcall">];
 //  let Subjects = [Function, ObjCMethod];
   let Documentation = [VectorCallDocs];
 }
 
-def Pascal : InheritableAttr {
+def Pascal : DeclOrTypeAttr {
   let Spellings = [Clang<"pascal">, Keyword<"__pascal">, Keyword<"_pascal">];
 //  let Subjects = [Function, ObjCMethod];
   let Documentation = [Undocumented];
 }
 
-def PreserveMost : InheritableAttr {
+def PreserveMost : DeclOrTypeAttr {
   let Spellings = [Clang<"preserve_most">];
   let Documentation = [PreserveMostDocs];
 }
 
-def PreserveAll : InheritableAttr {
+def PreserveAll : DeclOrTypeAttr {
   let Spellings = [Clang<"preserve_all">];
   let Documentation = [PreserveAllDocs];
 }
 
 def Target : InheritableAttr {
   let Spellings = [GCC<"target">];
   let Args = [StringArgument<"featuresStr">];
   let Subjects = SubjectList<[Function], ErrorDiag>;

include/clang/Basic/DiagnosticSemaKinds.td

 def err_attribute_regparm_wrong_platform : Error<
   "'regparm' is not valid on this platform">;
 def err_attribute_regparm_invalid_number : Error<
   "'regparm' parameter must be between 0 and %0 inclusive">;
 def err_attribute_not_supported_in_lang : Error<
   "%0 attribute is not supported in %select{C|C++|Objective-C}1">;
 def err_attribute_not_supported_on_arch
     : Error<"%0 attribute is not supported on '%1'">;
+def warn_gcc_ignores_type_attr : Warning<
+  "GCC does not allow the %0 attribute to be written on a type">,
+  InGroup<GccCompat>;
 
 // Clang-Specific Attributes
 def warn_attribute_iboutlet : Warning<
   "%0 attribute can only be applied to instance variables or properties">,
   InGroup<IgnoredAttributes>;
 def err_iboutletcollection_type : Error<
   "invalid type %0 as argument of iboutletcollection attribute">;
 def err_iboutletcollection_builtintype : Error<

lib/Sema/SemaType.cpp

                             // stmts
 
     // Skip attributes that were marked to be invalid.
     if (attr.isInvalid())
       continue;
 
     if (attr.isCXX11Attribute()) {
       // [[gnu::...]] attributes are treated as declaration attributes, so may
-      // not appertain to a DeclaratorChunk, even if we handle them as type
-      // attributes.
+      // not appertain to a DeclaratorChunk. If we handle them as type
+      // attributes, accept them in that position and diagnose the GCC
+      // incompatibility.
       if (attr.getScopeName() && attr.getScopeName()->isStr("gnu")) {
+        bool IsTypeAttr = attr.isTypeAttr();
         if (TAL == TAL_DeclChunk) {
           state.getSema().Diag(attr.getLoc(),
-                               diag::warn_cxx11_gnu_attribute_on_type)
+                               IsTypeAttr
+                                   ? diag::warn_gcc_ignores_type_attr
+                                   : diag::warn_cxx11_gnu_attribute_on_type)
               << attr.getName();
-          continue;
+          if (!IsTypeAttr)
+            continue;
         }
       } else if (TAL != TAL_DeclChunk) {
         // Otherwise, only consider type processing for a C++11 attribute if
         // it's actually been applied to a type.
         continue;
       }
     }
 

test/SemaCXX/cxx11-gnu-attrs.cpp

-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++11 -verify %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++11 -Wno-gcc-compat -verify %s
 
 // Error cases.
 
 [[gnu::this_attribute_does_not_exist]] int unknown_attr;
 // expected-warning@-1 {{unknown attribute 'this_attribute_does_not_exist' ignored}}
 int [[gnu::unused]] attr_on_type;
 // expected-error@-1 {{'unused' attribute cannot be applied to types}}
 int *[[gnu::unused]] attr_on_ptr;
 // expected-warning@-1 {{attribute 'unused' ignored, because it cannot be applied to a type}}
 [[gnu::fastcall]] void pr17424_1();
 // expected-warning@-1 {{calling convention 'fastcall' ignored for this target}}
 [[gnu::fastcall]] [[gnu::stdcall]] void pr17424_2();
 // expected-warning@-1 {{calling convention 'fastcall' ignored for this target}}
 // expected-warning@-2 {{calling convention 'stdcall' ignored for this target}}
 [[gnu::fastcall]] __stdcall void pr17424_3();
 // expected-warning@-1 {{calling convention 'fastcall' ignored for this target}}
 // expected-warning@-2 {{calling convention '__stdcall' ignored for this target}}
 [[gnu::fastcall]] void pr17424_4() [[gnu::stdcall]];
 // expected-warning@-1 {{calling convention 'fastcall' ignored for this target}}
-// expected-warning@-2 {{attribute 'stdcall' ignored, because it cannot be applied to a type}}
-// expected-warning@-3 {{calling convention 'stdcall' ignored for this target}}
+// expected-warning@-2 {{calling convention 'stdcall' ignored for this target}}
 void pr17424_5 [[gnu::fastcall]]();
 // expected-warning@-1 {{calling convention 'fastcall' ignored for this target}}
 
 // Valid cases.
 
 void aliasb [[gnu::alias("_Z6alias1v")]] ();
 void alias1() {}
 void aliasa [[gnu::alias("_Z6alias1v")]] ();

test/SemaCXX/type-attrs.cpp

+// RUN: %clang_cc1 -triple x86_64-pc-win32 -Wgcc-compat -std=c++11 -verify %s
+
+void f() [[gnu::cdecl]] {}  // expected-warning {{GCC does not allow the 'cdecl' attribute to be written on a type}}
+void g() [[gnu::stdcall]] {}  // expected-warning {{GCC does not allow the 'stdcall' attribute to be written on a type}}
+void i() [[gnu::fastcall]] {}  // expected-warning {{GCC does not allow the 'fastcall' attribute to be written on a type}}

utils/TableGen/ClangAttrEmitter.cpp

       continue;
     
     ArrayRef<std::pair<Record *, SMRange>> Supers = R.getSuperClasses();
     assert(!Supers.empty() && "Forgot to specify a superclass for the attr");
     std::string SuperName;
     bool Inheritable = false;
     for (const auto &Super : llvm::reverse(Supers)) {
       const Record *R = Super.first;
-      if (R->getName() != "TargetSpecificAttr" && SuperName.empty())
+      if (R->getName() != "TargetSpecificAttr" &&
+          R->getName() != "DeclOrTypeAttr" && SuperName.empty())
         SuperName = R->getName();
       if (R->getName() == "InheritableAttr")
         Inheritable = true;
     }
 
     OS << "class " << R.getName() << "Attr : public " << SuperName << " {\n";
 
     std::vector<Record*> ArgRecords = R.getValueAsListOfDefs("Args");

     // the spellings are identical, and custom parsing rules match, etc.
 
     // We need to generate struct instances based off ParsedAttrInfo from
     // AttributeList.cpp.
     SS << "  { ";
     emitArgInfo(*I->second, SS);
     SS << ", " << I->second->getValueAsBit("HasCustomParsing");
     SS << ", " << I->second->isSubClassOf("TargetSpecificAttr");
-    SS << ", " << I->second->isSubClassOf("TypeAttr");
+    SS << ", "
+       << (I->second->isSubClassOf("TypeAttr") ||
+           I->second->isSubClassOf("DeclOrTypeAttr"));
     SS << ", " << I->second->isSubClassOf("StmtAttr");
     SS << ", " << IsKnownToGCC(*I->second);
     SS << ", " << PragmaAttributeSupport.isAttributedSupported(*I->second);
     SS << ", " << GenerateAppertainsTo(*I->second, OS);
     SS << ", " << GenerateLangOptRequirements(*I->second, OS);
     SS << ", " << GenerateTargetRequirements(*I->second, Dupes, OS);
     SS << ", " << GenerateSpellingIndexToSemanticSpelling(*I->second, OS);
     SS << ", "