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

[Sean Hunt]

On Fri, Jun 15, 2012 at 8:05 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> ENOREPLYTOALL :)
>
> On Fri, Jun 15, 2012 at 4:55 PM, Sean Hunt <scshunt at csclub.uwaterloo.ca> wrote:
>> On Fri, Jun 15, 2012 at 6:34 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>> --- 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.
>>
>> Indeed, this should probably be improved (and at the very least, we
>> need Attr.td able to explicitly distinguish between various types of
>> attributes. That's future work, though.
>>
>>> 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?
>>
>> Sure, that's a better term.
>>
>>> --- 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.
>>
>> Anyone else want to comment on this style before I make the change?
>> It's based on existing code in getAttr.
>>
>>> +
>>> +          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 '+'.
>>
>> Copy-paste, but I'l look into that.
>>
>>> +          else
>>> +            Matches.push_back(
>>> +              StringMatcher::StringPair(
>>> +                Buf.str(),
>>> +                std::string("return AttributeList::IgnoredAttribute;")));
>>>
>>> No need for the "std::string(", ")" here either.
>>>
>>
>> Same here.
>>
>> Sean
>