r199144 - 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).

[Argyrios Kyrtzidis]

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 <aaron at aaronballman.com> 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 <aaron at aaronballman.com> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits