[cfe-commits] [PATCH] Automatically handle parsing of attribute namespaces.

[Richard Smith]

On Fri, Jun 15, 2012 at 2:54 PM, Sean Hunt <scshunt at csclub.uwaterloo.ca> wrote:
> On Fri, Jun 15, 2012 at 2:37 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>> Hi Sean,
>>
>> On Fri, Jun 15, 2012 at 2:07 PM, Sean Hunt <scshunt at csclub.uwaterloo.ca> wrote:
>>> On the basis that the code should be self-descriptive, no description.
>>
>> Is the diff missing part of the patch sequence?
>>
>> --- a/lib/Sema/AttributeList.cpp
>> +++ b/lib/Sema/AttributeList.cpp
>> @@ -109,10 +111,13 @@ AttributeList::Kind AttributeList::getKind(const
>> IdentifierInfo *Name,
>>       AttrName.size() >= 4)
>>     AttrName = AttrName.substr(2, AttrName.size() - 4);
>>
>> -  // FIXME: implement attribute namespacing correctly.
>>   SmallString<64> Buf;
>>   if (ScopeName)
>> -    (Buf += ScopeName->getName()) += "::";
>>
>> This doesn't match the code in my checkout.
>>
>
> Ah it is, good catch.
>
> Fix attached.

--- a/include/clang/Basic/Attr.td
+++ b/include/clang/Basic/Attr.td
@@ -320,8 +320,9 @@ def ExtVectorType : Attr {
 }

 def FallThrough : Attr {
-  let Spellings = ["clang___fallthrough"];
-  let Subjects = [CaseStmt, DefaultStmt];
+  let Namespaces = ["clang"];
+  let Spellings = ["fallthrough"];
+  let Subjects = [NullStmt];
 }

I don't think this Namespaces approach is the right long-term
solution: we'll need a mechanism to specify GNU spellings, C++11
spellings and __declspec spelling independently. It seems we can get
away with this for the fallthrough attribute because it's a statement
attribute and only C++11 syntax allows statement attributes.

Anyway, this is a definite improvement. LGTM with a few tweaks:

--- a/include/clang/Sema/AttributeList.h
+++ b/include/clang/Sema/AttributeList.h
@@ -52,6 +52,13 @@ struct AvailabilityChange {
 /// 4: __attribute__(( aligned(16) )). ParmName is unused, Args/Num used.
 ///
 class AttributeList { // TODO: This should really be called ParsedAttribute
+public:
+  /// The style used to specify an attribute.
+  enum DeclStyle {
+    ADS_GNU,
+    ADS_CXX11,
+    ADS_Declspec
+  };

I'm not a huge fan of this enum name. Maybe AttributeSyntax or
AttributeSyntaxKind?

--- a/utils/TableGen/ClangAttrEmitter.cpp
+++ b/utils/TableGen/ClangAttrEmitter.cpp
@@ -1136,6 +1139,23 @@ void EmitClangAttrParsedAttrKinds(RecordKeeper
&Records, raw_ostream &OS) {
                                                  : Spellings.front());
         StringRef Spelling = NormalizeAttrSpelling(*I);

+        for (std::vector<StringRef>::const_iterator NI = Namespaces.begin(),
+             NE = Namespaces.end(); NI != NE; ++NI) {
+          SmallString<64> Buf;
+          ((Buf += *NI) += "::" ) += Spelling;

I'd prefer to see this written as three statements.

+
+          if (SemaHandler)
+            Matches.push_back(
+              StringMatcher::StringPair(
+                Buf.str(),
+                std::string("return
AttributeList::AT_")+AttrName.str() + ";"));

No need for the "std::string(" and ")"; StringRef::str() returns a
std::string. Also add spaces around the '+'.

+          else
+            Matches.push_back(
+              StringMatcher::StringPair(
+                Buf.str(),
+                std::string("return AttributeList::IgnoredAttribute;")));

No need for the "std::string(", ")" here either.