That's a really great idea! I'll put it on The List and give some thought as to how to accomplish it. Thanks!
~Aaron On Fri, Jan 31, 2014 at 1:55 AM, Argyrios Kyrtzidis <[email protected]> wrote: > Hi Aaron, > > Thank you for your work on improving our attributes, much appreciated! > > We came across a project that was using "__attribute__ ((align(16)))" which > now gives a warning: > > warning: unknown attribute 'align' ignored > > I believe tightening our attribute syntax is goodness, but what do you think > about enhancing the attribute warning with "typo correction", so we can get > something like this: > > warning: unknown attribute 'align' ignored; did you mean 'aligned' ? > > along with a fixit ? > > -Argyrios > > On Jan 13, 2014, at 1:49 PM, Aaron Ballman <[email protected]> wrote: > >> Please note that this *may* affect existing code (it found numerous >> bugs in our test cases, all of which have been rectified by other >> commits). I will pay attention to build bots, but if you find that an >> attribute used to be supported with a given syntax and is no longer >> supported, please let me know. >> >> Thanks! >> >> ~Aaron >> >> On Mon, Jan 13, 2014 at 4:42 PM, Aaron Ballman <[email protected]> >> wrote: >>> Author: aaronballman >>> Date: Mon Jan 13 15:42:39 2014 >>> New Revision: 199144 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=199144&view=rev >>> Log: >>> When determining the attribute's parsed kind, pay attention to the syntax >>> used. This fixes bugs where an attribute has differing GNU and Declspec >>> spellings, but they are treated as the same. Eg) __declspec(aligned) when >>> it should be __attribute__((aligned)), and __attribute__((align)) when it >>> should be __declspec(align). >>> >>> Modified: >>> cfe/trunk/lib/Sema/AttributeList.cpp >>> cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp >>> >>> Modified: cfe/trunk/lib/Sema/AttributeList.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AttributeList.cpp?rev=199144&r1=199143&r2=199144&view=diff >>> ============================================================================== >>> --- cfe/trunk/lib/Sema/AttributeList.cpp (original) >>> +++ cfe/trunk/lib/Sema/AttributeList.cpp Mon Jan 13 15:42:39 2014 >>> @@ -139,7 +139,7 @@ AttributeList::Kind AttributeList::getKi >>> FullName += "::"; >>> FullName += AttrName; >>> >>> - return ::getAttrKind(FullName); >>> + return ::getAttrKind(FullName, SyntaxUsed); >>> } >>> >>> unsigned AttributeList::getAttributeSpellingListIndex() const { >>> >>> Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=199144&r1=199143&r2=199144&view=diff >>> ============================================================================== >>> --- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original) >>> +++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Mon Jan 13 15:42:39 2014 >>> @@ -2250,9 +2250,8 @@ void EmitClangAttrParsedAttrImpl(RecordK >>> void EmitClangAttrParsedAttrKinds(RecordKeeper &Records, raw_ostream &OS) { >>> emitSourceFileHeader("Attribute name matcher", OS); >>> >>> - std::vector<Record*> Attrs = Records.getAllDerivedDefinitions("Attr"); >>> - >>> - std::vector<StringMatcher::StringPair> Matches; >>> + std::vector<Record *> Attrs = Records.getAllDerivedDefinitions("Attr"); >>> + std::vector<StringMatcher::StringPair> GNU, Declspec, CXX11, Keywords; >>> std::set<std::string> Seen; >>> for (std::vector<Record*>::iterator I = Attrs.begin(), E = Attrs.end(); >>> I != E; ++I) { >>> @@ -2261,8 +2260,16 @@ void EmitClangAttrParsedAttrKinds(Record >>> bool SemaHandler = Attr.getValueAsBit("SemaHandler"); >>> bool Ignored = Attr.getValueAsBit("Ignored"); >>> if (SemaHandler || Ignored) { >>> - std::vector<Record*> Spellings = >>> Attr.getValueAsListOfDefs("Spellings"); >>> - >>> + // Attribute spellings can be shared between target-specific >>> attributes, >>> + // and can be shared between syntaxes for the same attribute. For >>> + // instance, an attribute can be spelled GNU<"interrupt"> for an ARM- >>> + // specific attribute, or MSP430-specific attribute. Additionally, an >>> + // attribute can be spelled GNU<"dllexport"> and >>> Declspec<"dllexport"> >>> + // for the same semantic attribute. Ultimately, we need to map each >>> of >>> + // these to a single AttributeList::Kind value, but the StringMatcher >>> + // class cannot handle duplicate match strings. So we generate a >>> list of >>> + // string to match based on the syntax, and emit multiple string >>> matchers >>> + // depending on the syntax used. >>> std::string AttrName; >>> if (Attr.isSubClassOf("TargetSpecificAttr") && >>> !Attr.isValueUnset("ParseKind")) { >>> @@ -2273,34 +2280,48 @@ void EmitClangAttrParsedAttrKinds(Record >>> } else >>> AttrName = NormalizeAttrName(StringRef(Attr.getName())).str(); >>> >>> + std::vector<Record*> Spellings = >>> Attr.getValueAsListOfDefs("Spellings"); >>> for (std::vector<Record*>::const_iterator I = Spellings.begin(), >>> E = Spellings.end(); I != E; ++I) { >>> std::string RawSpelling = (*I)->getValueAsString("Name"); >>> - >>> - SmallString<64> Spelling; >>> - if ((*I)->getValueAsString("Variety") == "CXX11") { >>> + std::vector<StringMatcher::StringPair> *Matches = 0; >>> + std::string Spelling, Variety = (*I)->getValueAsString("Variety"); >>> + if (Variety == "CXX11") { >>> + Matches = &CXX11; >>> Spelling += (*I)->getValueAsString("Namespace"); >>> Spelling += "::"; >>> - } >>> - Spelling += NormalizeAttrSpelling(RawSpelling); >>> + } else if (Variety == "GNU") >>> + Matches = &GNU; >>> + else if (Variety == "Declspec") >>> + Matches = &Declspec; >>> + else if (Variety == "Keyword") >>> + Matches = &Keywords; >>> >>> + assert(Matches && "Unsupported spelling variety found"); >>> + >>> + Spelling += NormalizeAttrSpelling(RawSpelling); >>> if (SemaHandler) >>> - Matches.push_back( >>> - StringMatcher::StringPair( >>> - StringRef(Spelling), >>> - "return AttributeList::AT_" + AttrName + ";")); >>> + Matches->push_back(StringMatcher::StringPair(Spelling, >>> + "return AttributeList::AT_" + AttrName + >>> ";")); >>> else >>> - Matches.push_back( >>> - StringMatcher::StringPair( >>> - StringRef(Spelling), >>> - "return AttributeList::IgnoredAttribute;")); >>> + Matches->push_back(StringMatcher::StringPair(Spelling, >>> + "return AttributeList::IgnoredAttribute;")); >>> } >>> } >>> } >>> >>> - OS << "static AttributeList::Kind getAttrKind(StringRef Name) {\n"; >>> - StringMatcher("Name", Matches, OS).Emit(); >>> - OS << "return AttributeList::UnknownAttribute;\n" >>> + OS << "static AttributeList::Kind getAttrKind(StringRef Name, "; >>> + OS << "AttributeList::Syntax Syntax) {\n"; >>> + OS << " if (AttributeList::AS_GNU == Syntax) {\n"; >>> + StringMatcher("Name", GNU, OS).Emit(); >>> + OS << " } else if (AttributeList::AS_Declspec == Syntax) {\n"; >>> + StringMatcher("Name", Declspec, OS).Emit(); >>> + OS << " } else if (AttributeList::AS_CXX11 == Syntax) {\n"; >>> + StringMatcher("Name", CXX11, OS).Emit(); >>> + OS << " } else if (AttributeList::AS_Keyword == Syntax) {\n"; >>> + StringMatcher("Name", Keywords, OS).Emit(); >>> + OS << " }\n"; >>> + OS << " return AttributeList::UnknownAttribute;\n" >>> << "}\n"; >>> } >>> >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
