Support GCC [[gnu::attributes]] in C2x mode

[aaron.ballman]

Summary

GCC 10.1 introduced support for the [[]] style spelling of attributes in C mode. Similar to how GCC supports __attribute__((foo)) as [[gnu::foo]] in C++ mode, it now supports the same spelling in C mode as well. This patch makes a change in Clang so that when you use the GCC attribute spelling, the attribute is automatically available in all three spellings by default. However, like Clang, GCC has some attributes it only recognizes in C++ mode (specifically, abi_tag and init_priority), which this patch also honors.

[erichkeane]

1 comment, perhaps outside the scope of this patch.

clang/utils/TableGen/ClangAttrEmitter.cpp

85

I guess its a problem for all of these, but why is the last 'true'/'false' (KnownToGCC) not checked from the Spelling? It has: let KnownToGCC = 1;.

I would presume that line either is meaningless, or should be used for the true/false bits in here.

[dblaikie]

Sounds good to me (welcome to wait for other reviews if you're looking for something more nuanced/exuperienced in this area)

[erichkeane]

1 more question, how did you arrive at the 'not supported in C' list? Did you find some GCC docs for that (if so, please put in the commit message)? Or did you just try them all?

[dblaikie]

(sorry @erichkeane didn't mean to usurp your feedback by approving before it was answered - @aaron.ballman do treat my approval as contingent on that feedback being addressed as you/both see fit)

[rsmith]

clang/utils/TableGen/ClangAttrEmitter.cpp

85

The KnownToGCC flag affects whether we produce certain warnings, not which spellings we register. This does seem fishy: we have the same information represented and examined both by considering whether the attribute has a GNU vs GCC spelling and by considering whether the KnownToGCC flag is set. I imagine we could factor this better. (The two concerns are, I suppose, notionally orthogonal, but I can't imagine we would ever want them to differ.)

[erichkeane]

In D80836#2064182, @dblaikie wrote:

(sorry @erichkeane didn't mean to usurp your feedback by approving before it was answered - @aaron.ballman do treat my approval as contingent on that feedback being addressed as you/both see fit)

No problem :) The "KnownToGCC" issue is likely a separate issue that should be fixed in a separate patch.

The source-of-the-not-supported-by-C info is simply "if you got this elsewhere, please document the source for future reference", which can be handled without me double checking. If its self experimentation, that makes me sad, but I don't think there is further action that could be taken about that one.

clang/utils/TableGen/ClangAttrEmitter.cpp

85

Well, both are used to set the value in "FlattenedSpelling". Here we use true/false, but if you construct one via a record it uses the KnownToGCC flag. It seems to me that these values should be initialized consistently.

The "spelling" Variety themselves I can see being different, but for consistency's sake, these trues in the emplace_back should set KnownToGCC with the attribute. That, or we abandon the KnownToGCC 'Value' in the spelling and just always infer it based on the variety.

Like I said, its just weird that sometimes we set this via the tablegen file, sometimes we do it automagically.

[aaron.ballman]

Updated to remove the KnownToGCC tablegen bit.

[aaron.ballman]

clang/utils/TableGen/ClangAttrEmitter.cpp

85

Yeah, I agree that this is a bit weird. IIRC, this came from a time when GCC hadn't yet made all attribute spellings available as [[]] spellings and so there was a worry we'd have some attribute spellings that use GNU instead of GCC, despite being supported by GCC. I don't think that situation is plausible any longer.

I think a better approach is to keep the known to GCC bit on the ParsedAttr object so we don't have to check attribute vendor name when issuing a diagnostic, but we should infer that bit from the spelling instead of setting it in the Attr.td file.

[erichkeane]

clang/utils/TableGen/ClangAttrEmitter.cpp

65

Doesn't this result in 'K' being uninitialized now?

[aaron.ballman]

clang/utils/TableGen/ClangAttrEmitter.cpp

65

K is still being initialized by the ctor init list above on line 56.

[erichkeane]

clang/utils/TableGen/ClangAttrEmitter.cpp

65

Isn't that in a different constructor?

[aaron.ballman]

New and improved with proper member initialization in all constructors.

[aaron.ballman]

clang/utils/TableGen/ClangAttrEmitter.cpp

65

Yes, yes it is. :-D Thank you for helping to point that out. I've fixed in the latest patch.

[aaron.ballman]

Whoops, *now* I'm properly setting the bit from the record.

[aaron.ballman]

In D80836#2064181, @erichkeane wrote:

1 more question, how did you arrive at the 'not supported in C' list? Did you find some GCC docs for that (if so, please put in the commit message)? Or did you just try them all?

There's a documented list, and I'll add it to the commit message (though the format of the link makes me wonder how stable it will be): https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/C_002b_002b-Attributes.html#C_002b_002b-Attributes

[aaron.ballman]

Sorry for the back and forth, this Monday morning is not easy it seems.

[aaron.ballman]

Thank you for the review, I've commit in 522934da1f0c78c1de1a80d4ba14204a11f5afa8

clang/include/clang/Basic/Attr.td

class VariadicEnumArgument<string name, string type, list<string> values,
   list<string> Values = values;
   list<string> Enums = enums;
 }
 
 // This handles one spelling of an attribute.
 class Spelling<string name, string variety> {
   string Name = name;
   string Variety = variety;
-  bit KnownToGCC;
 }
 
 class GNU<string name> : Spelling<name, "GNU">;
 class Declspec<string name> : Spelling<name, "Declspec">;
 class Microsoft<string name> : Spelling<name, "Microsoft">;
 class CXX11<string namespace, string name, int version = 1>
     : Spelling<name, "CXX11"> {
   string Namespace = namespace;
   int Version = version;
 }
 class C2x<string namespace, string name> : Spelling<name, "C2x"> {
   string Namespace = namespace;
 }
 
 class Keyword<string name> : Spelling<name, "Keyword">;
 class Pragma<string namespace, string name> : Spelling<name, "Pragma"> {
   string Namespace = namespace;
 }
 
-// The GCC spelling implies GNU<name> and CXX11<"gnu", name> and also sets
-// KnownToGCC to 1. This spelling should be used for any GCC-compatible
+// The GCC spelling implies GNU<name>, CXX11<"gnu", name>, and optionally,
+// C2x<"gnu", name>. This spelling should be used for any GCC-compatible
 // attributes.
-class GCC<string name> : Spelling<name, "GCC"> {
-  let KnownToGCC = 1;
+class GCC<string name, bit allowInC = 1> : Spelling<name, "GCC"> {
+  bit AllowInC = allowInC;
 }
 
 // The Clang spelling implies GNU<name>, CXX11<"clang", name>, and optionally,
 // C2x<"clang", name>. This spelling should be used for any Clang-specific
 // attributes.
 class Clang<string name, bit allowInC = 1> : Spelling<name, "Clang"> {
   bit AllowInC = allowInC;
 }
class IgnoredAttr : Attr {
   let Documentation = [Undocumented];
 }
 
 //
 // Attributes begin here
 //
 
 def AbiTag : Attr {
-  let Spellings = [GCC<"abi_tag">];
+  let Spellings = [GCC<"abi_tag", /*AllowInC*/0>];
   let Args = [VariadicStringArgument<"Tags">];
   let Subjects = SubjectList<[Struct, Var, Function, Namespace], ErrorDiag>;
   let MeaningfulToClassTemplateDefinition = 1;
   let Documentation = [AbiTagsDocs];
 }
 
 def AddressSpace : TypeAttr {
   let Spellings = [Clang<"address_space">];
def WorkGroupSizeHint :  InheritableAttr {
   let Args = [UnsignedArgument<"XDim">,
               UnsignedArgument<"YDim">,
               UnsignedArgument<"ZDim">];
   let Subjects = SubjectList<[Function], ErrorDiag>;
   let Documentation = [Undocumented];
 }
 
 def InitPriority : InheritableAttr {
-  let Spellings = [GCC<"init_priority">];
+  let Spellings = [GCC<"init_priority", /*AllowInC*/0>];
   let Args = [UnsignedArgument<"Priority">];
   let Subjects = SubjectList<[Var], ErrorDiag>;
   let Documentation = [Undocumented];
 }
 
 def Section : InheritableAttr {
   let Spellings = [GCC<"section">, Declspec<"allocate">];
   let Args = [StringArgument<"Name">];

clang/test/Sema/attr-c2x.c

 
 void foo2(void) [[clang::unavailable("not available - replaced")]]; // expected-note {{'foo2' has been explicitly marked unavailable here}}
 void bar(void) {
   foo2(); // expected-error {{'foo2' is unavailable: not available - replaced}}
 }
 
 [[nodiscard]] int without_underscores(void);
 [[__nodiscard__]] int underscores(void);
+
+// Match GCC's behavior for C attributes as well.
+[[gnu::constructor]] void ctor_func(void);
+[[gnu::destructor]] void dtor_func(void);
+[[gnu::hot]] void hot_func(void);
+[[__gnu__::hot]] void hot_func2(void);
+[[gnu::__hot__]] void hot_func3(void);
+[[__gnu__::__hot__]] void hot_func4(void);
+
+// Note how not all GCC attributes are supported in C.
+[[gnu::abi_tag("")]] void abi_func(void); // expected-warning {{unknown attribute 'abi_tag' ignored}}
+struct S s [[gnu::init_priority(1)]]; // expected-warning {{unknown attribute 'init_priority' ignored}}

clang/utils/TableGen/ClangAttrEmitter.cpp

 #include <vector>
 
 using namespace llvm;
 
 namespace {
 
 class FlattenedSpelling {
   std::string V, N, NS;
-  bool K;
+  bool K = false;
 
 public:
   FlattenedSpelling(const std::string &Variety, const std::string &Name,
                     const std::string &Namespace, bool KnownToGCC) :
     V(Variety), N(Name), NS(Namespace), K(KnownToGCC) {}
   explicit FlattenedSpelling(const Record &Spelling)
       : V(std::string(Spelling.getValueAsString("Variety"))),
         N(std::string(Spelling.getValueAsString("Name"))) {
     assert(V != "GCC" && V != "Clang" &&
            "Given a GCC spelling, which means this hasn't been flattened!");
     if (V == "CXX11" || V == "C2x" || V == "Pragma")
       NS = std::string(Spelling.getValueAsString("Namespace"));
-    bool Unset;
-    K = Spelling.getValueAsBitOrUnset("KnownToGCC", Unset);
   }
 
   const std::string &variety() const { return V; }
   const std::string &name() const { return N; }
   const std::string &nameSpace() const { return NS; }
   bool knownToGCC() const { return K; }
 };
 
 } // end anonymous namespace
 
 static std::vector<FlattenedSpelling>
 GetFlattenedSpellings(const Record &Attr) {
   std::vector<Record *> Spellings = Attr.getValueAsListOfDefs("Spellings");
   std::vector<FlattenedSpelling> Ret;
 
   for (const auto &Spelling : Spellings) {
     StringRef Variety = Spelling->getValueAsString("Variety");
     StringRef Name = Spelling->getValueAsString("Name");
     if (Variety == "GCC") {
-      // Gin up two new spelling objects to add into the list.
       Ret.emplace_back("GNU", std::string(Name), "", true);
       Ret.emplace_back("CXX11", std::string(Name), "gnu", true);
+      if (Spelling->getValueAsBit("AllowInC"))
+        Ret.emplace_back("C2x", std::string(Name), "gnu", true);
     } else if (Variety == "Clang") {
       Ret.emplace_back("GNU", std::string(Name), "", false);
       Ret.emplace_back("CXX11", std::string(Name), "clang", false);
       if (Spelling->getValueAsBit("AllowInC"))
         Ret.emplace_back("C2x", std::string(Name), "clang", false);
     } else
       Ret.push_back(FlattenedSpelling(*Spelling));
   }