On Fri, Jun 15, 2012 at 8:05 PM, Richard Smith <[email protected]> wrote: > ENOREPLYTOALL :) > > On Fri, Jun 15, 2012 at 4:55 PM, Sean Hunt <[email protected]> > wrote: >> On Fri, Jun 15, 2012 at 6:34 PM, Richard Smith <[email protected]> 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 >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
