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

2020-06-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman closed this revision.
aaron.ballman added a comment.

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



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

https://reviews.llvm.org/D80836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-06-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman updated this revision to Diff 267606.
aaron.ballman added a comment.

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


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

https://reviews.llvm.org/D80836

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/Sema/attr-c2x.c
  clang/utils/TableGen/ClangAttrEmitter.cpp


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -48,7 +48,7 @@
 
 class FlattenedSpelling {
   std::string V, N, NS;
-  bool K;
+  bool K = false;
 
 public:
   FlattenedSpelling(const std::string , const std::string ,
@@ -61,8 +61,6 @@
"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 () const { return V; }
@@ -82,9 +80,10 @@
 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);
Index: clang/test/Sema/attr-c2x.c
===
--- clang/test/Sema/attr-c2x.c
+++ clang/test/Sema/attr-c2x.c
@@ -27,3 +27,15 @@
 
 [[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}}
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -257,7 +257,6 @@
 class Spelling {
   string Name = name;
   string Variety = variety;
-  bit KnownToGCC;
 }
 
 class GNU : Spelling;
@@ -277,11 +276,11 @@
   string Namespace = namespace;
 }
 
-// The GCC spelling implies GNU and CXX11<"gnu", name> and also sets
-// KnownToGCC to 1. This spelling should be used for any GCC-compatible
+// The GCC spelling implies GNU, CXX11<"gnu", name>, and optionally,
+// C2x<"gnu", name>. This spelling should be used for any GCC-compatible
 // attributes.
-class GCC : Spelling {
-  let KnownToGCC = 1;
+class GCC : Spelling {
+  bit AllowInC = allowInC;
 }
 
 // The Clang spelling implies GNU, CXX11<"clang", name>, and optionally,
@@ -605,7 +604,7 @@
 //
 
 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;
@@ -2113,7 +2112,7 @@
 }
 
 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];


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -48,7 +48,7 @@
 
 class FlattenedSpelling {
   std::string V, N, NS;
-  bool K;
+  bool K = false;
 
 public:
   FlattenedSpelling(const std::string , const std::string ,
@@ -61,8 +61,6 @@
"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 () const { return V; }
@@ -82,9 +80,10 @@
 StringRef Variety = Spelling->getValueAsString("Variety");
 StringRef Name = Spelling->getValueAsString("Name");
 if (Variety == "GCC") {
-  // Gin up two new spelling

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

2020-06-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added a comment.

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


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

https://reviews.llvm.org/D80836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-06-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman updated this revision to Diff 267604.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

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


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

https://reviews.llvm.org/D80836

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/Sema/attr-c2x.c
  clang/utils/TableGen/ClangAttrEmitter.cpp


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -82,9 +82,10 @@
 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);
Index: clang/test/Sema/attr-c2x.c
===
--- clang/test/Sema/attr-c2x.c
+++ clang/test/Sema/attr-c2x.c
@@ -27,3 +27,15 @@
 
 [[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}}
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -257,7 +257,6 @@
 class Spelling {
   string Name = name;
   string Variety = variety;
-  bit KnownToGCC;
 }
 
 class GNU : Spelling;
@@ -277,11 +276,11 @@
   string Namespace = namespace;
 }
 
-// The GCC spelling implies GNU and CXX11<"gnu", name> and also sets
-// KnownToGCC to 1. This spelling should be used for any GCC-compatible
+// The GCC spelling implies GNU, CXX11<"gnu", name>, and optionally,
+// C2x<"gnu", name>. This spelling should be used for any GCC-compatible
 // attributes.
-class GCC : Spelling {
-  let KnownToGCC = 1;
+class GCC : Spelling {
+  bit AllowInC = allowInC;
 }
 
 // The Clang spelling implies GNU, CXX11<"clang", name>, and optionally,
@@ -605,7 +604,7 @@
 //
 
 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;
@@ -2113,7 +2112,7 @@
 }
 
 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];


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -82,9 +82,10 @@
 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);
Index: clang/test/Sema/attr-c2x.c
===
--- clang/test/Sema/attr-c2x.c
+++ clang/test/Sema/attr-c2x.c
@@ -27,3 +27,15 @@
 
 [[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

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

2020-06-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman updated this revision to Diff 267603.
aaron.ballman added a comment.

New and improved with proper member initialization in all constructors.


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

https://reviews.llvm.org/D80836

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/Sema/attr-c2x.c
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -48,7 +48,7 @@
 
 class FlattenedSpelling {
   std::string V, N, NS;
-  bool K;
+  bool K = false;
 
 public:
   FlattenedSpelling(const std::string , const std::string ,
@@ -60,9 +60,7 @@
 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);
+  NS = std::string(Spelling.getValueAsString("Namespace"));  
   }
 
   const std::string () const { return V; }
@@ -82,9 +80,10 @@
 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);
Index: clang/test/Sema/attr-c2x.c
===
--- clang/test/Sema/attr-c2x.c
+++ clang/test/Sema/attr-c2x.c
@@ -27,3 +27,15 @@
 
 [[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}}
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -257,7 +257,6 @@
 class Spelling {
   string Name = name;
   string Variety = variety;
-  bit KnownToGCC;
 }
 
 class GNU : Spelling;
@@ -277,11 +276,11 @@
   string Namespace = namespace;
 }
 
-// The GCC spelling implies GNU and CXX11<"gnu", name> and also sets
-// KnownToGCC to 1. This spelling should be used for any GCC-compatible
+// The GCC spelling implies GNU, CXX11<"gnu", name>, and optionally,
+// C2x<"gnu", name>. This spelling should be used for any GCC-compatible
 // attributes.
-class GCC : Spelling {
-  let KnownToGCC = 1;
+class GCC : Spelling {
+  bit AllowInC = allowInC;
 }
 
 // The Clang spelling implies GNU, CXX11<"clang", name>, and optionally,
@@ -605,7 +604,7 @@
 //
 
 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;
@@ -2113,7 +2112,7 @@
 }
 
 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];
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-06-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman marked an inline comment as done.
aaron.ballman added inline comments.



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:65
-bool Unset;
-K = Spelling.getValueAsBitOrUnset("KnownToGCC", Unset);
   }

erichkeane wrote:
> Doesn't this result in 'K' being uninitialized now?  
`K` is still being initialized by the ctor init list above on line 56.


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

https://reviews.llvm.org/D80836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-06-01 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman marked 3 inline comments as done.
aaron.ballman added inline comments.



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:65
-bool Unset;
-K = Spelling.getValueAsBitOrUnset("KnownToGCC", Unset);
   }

erichkeane wrote:
> aaron.ballman wrote:
> > erichkeane wrote:
> > > Doesn't this result in 'K' being uninitialized now?  
> > `K` is still being initialized by the ctor init list above on line 56.
> Isn't that in a different constructor?
Yes, yes it is. :-D Thank you for helping to point that out. I've fixed in the 
latest patch.


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

https://reviews.llvm.org/D80836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-06-01 Thread Erich Keane via Phabricator via cfe-commits 
erichkeane added inline comments.



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:65
-bool Unset;
-K = Spelling.getValueAsBitOrUnset("KnownToGCC", Unset);
   }

aaron.ballman wrote:
> erichkeane wrote:
> > Doesn't this result in 'K' being uninitialized now?  
> `K` is still being initialized by the ctor init list above on line 56.
Isn't that in a different constructor?


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

https://reviews.llvm.org/D80836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-06-01 Thread Erich Keane via Phabricator via cfe-commits 
erichkeane added inline comments.



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:65
-bool Unset;
-K = Spelling.getValueAsBitOrUnset("KnownToGCC", Unset);
   }

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


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

https://reviews.llvm.org/D80836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-05-30 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman added inline comments.



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:87
   Ret.emplace_back("CXX11", std::string(Name), "gnu", true);
+  if (Spelling->getValueAsBit("AllowInC"))
+Ret.emplace_back("C2x", std::string(Name), "gnu", true);

erichkeane wrote:
> rsmith wrote:
> > erichkeane wrote:
> > > 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.
> > 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.)
> 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.
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.


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

https://reviews.llvm.org/D80836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-05-30 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman updated this revision to Diff 267466.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

Updated to remove the KnownToGCC tablegen bit.


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

https://reviews.llvm.org/D80836

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/Sema/attr-c2x.c
  clang/utils/TableGen/ClangAttrEmitter.cpp


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -60,9 +60,7 @@
 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);
+  NS = std::string(Spelling.getValueAsString("Namespace"));  
   }
 
   const std::string () const { return V; }
@@ -82,9 +80,10 @@
 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);
Index: clang/test/Sema/attr-c2x.c
===
--- clang/test/Sema/attr-c2x.c
+++ clang/test/Sema/attr-c2x.c
@@ -27,3 +27,15 @@
 
 [[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}}
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -257,7 +257,6 @@
 class Spelling {
   string Name = name;
   string Variety = variety;
-  bit KnownToGCC;
 }
 
 class GNU : Spelling;
@@ -277,11 +276,11 @@
   string Namespace = namespace;
 }
 
-// The GCC spelling implies GNU and CXX11<"gnu", name> and also sets
-// KnownToGCC to 1. This spelling should be used for any GCC-compatible
+// The GCC spelling implies GNU, CXX11<"gnu", name>, and optionally,
+// C2x<"gnu", name>. This spelling should be used for any GCC-compatible
 // attributes.
-class GCC : Spelling {
-  let KnownToGCC = 1;
+class GCC : Spelling {
+  bit AllowInC = allowInC;
 }
 
 // The Clang spelling implies GNU, CXX11<"clang", name>, and optionally,
@@ -605,7 +604,7 @@
 //
 
 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;
@@ -2113,7 +2112,7 @@
 }
 
 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];


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -60,9 +60,7 @@
 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);
+  NS = std::string(Spelling.getValueAsString("Namespace"));  
   }
 
   const std::string () const { return V; }
@@ -82,9 +80,10 @@
 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);

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

2020-05-29 Thread Erich Keane via Phabricator via cfe-commits 
erichkeane accepted this revision.
erichkeane added a comment.

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.




Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:87
   Ret.emplace_back("CXX11", std::string(Name), "gnu", true);
+  if (Spelling->getValueAsBit("AllowInC"))
+Ret.emplace_back("C2x", std::string(Name), "gnu", true);

rsmith wrote:
> erichkeane wrote:
> > 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.
> 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.)
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.


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

https://reviews.llvm.org/D80836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-05-29 Thread Richard Smith - zygoloid via Phabricator via cfe-commits 
rsmith added inline comments.



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:87
   Ret.emplace_back("CXX11", std::string(Name), "gnu", true);
+  if (Spelling->getValueAsBit("AllowInC"))
+Ret.emplace_back("C2x", std::string(Name), "gnu", true);

erichkeane wrote:
> 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.
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.)


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

https://reviews.llvm.org/D80836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-05-29 Thread David Blaikie via Phabricator via cfe-commits 
dblaikie added a comment.

(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)


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

https://reviews.llvm.org/D80836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-05-29 Thread David Blaikie via Phabricator via cfe-commits 
dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

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


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

https://reviews.llvm.org/D80836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-05-29 Thread Erich Keane via Phabricator via cfe-commits 
erichkeane added a comment.

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?


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

https://reviews.llvm.org/D80836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-05-29 Thread Erich Keane via Phabricator via cfe-commits 
erichkeane added a comment.

1 comment, perhaps outside the scope of this patch.




Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:87
   Ret.emplace_back("CXX11", std::string(Name), "gnu", true);
+  if (Spelling->getValueAsBit("AllowInC"))
+Ret.emplace_back("C2x", std::string(Name), "gnu", true);

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.


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

https://reviews.llvm.org/D80836



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-05-29 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, erichkeane, dblaikie.
Herald added a subscriber: krytarowski.

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.


https://reviews.llvm.org/D80836

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/Sema/attr-c2x.c
  clang/utils/TableGen/ClangAttrEmitter.cpp


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -82,9 +82,10 @@
 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);
Index: clang/test/Sema/attr-c2x.c
===
--- clang/test/Sema/attr-c2x.c
+++ clang/test/Sema/attr-c2x.c
@@ -27,3 +27,15 @@
 
 [[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}}
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -277,11 +277,12 @@
   string Namespace = namespace;
 }
 
-// The GCC spelling implies GNU and CXX11<"gnu", name> and also sets
-// KnownToGCC to 1. This spelling should be used for any GCC-compatible
-// attributes.
-class GCC : Spelling {
+// The GCC spelling implies GNU, CXX11<"gnu", name>, and optionally,
+// C2x<"gnu", name>, and also sets KnownToGCC to 1. This spelling should be
+// used for any GCC-compatible attributes.
+class GCC : Spelling {
   let KnownToGCC = 1;
+  bit AllowInC = allowInC;
 }
 
 // The Clang spelling implies GNU, CXX11<"clang", name>, and optionally,
@@ -605,7 +606,7 @@
 //
 
 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;
@@ -2113,7 +2114,7 @@
 }
 
 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];


Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -82,9 +82,10 @@
 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);
Index: clang/test/Sema/attr-c2x.c
===
--- clang/test/Sema/attr-c2x.c
+++ clang/test/Sema/attr-c2x.c
@@ -27,3 +27,15 @@
 
 [[nodiscard]] int without_underscores(void);