r210925 - Adds a Pragma spelling for attributes to tablegen and makes use of it for loop

Aaron Ballman aaron at aaronballman.com
Thu Nov 20 13:35:23 PST 2014


Fixed in r222456

~Aaron

On Thu, Nov 20, 2014 at 4:31 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Thu, Nov 20, 2014 at 4:24 PM, Douglas Gregor <dgregor at apple.com> wrote:
>> [Dropping Tyler, who is no longer at this address]
>>
>> On Nov 20, 2014, at 1:12 PM, Aaron Ballman <aaron at aaronballman.com> wrote:
>>
>> On Thu, Nov 20, 2014 at 3:59 PM, Douglas Gregor <dgregor at apple.com> wrote:
>>
>>
>> On Jun 13, 2014, at 10:57 AM, Tyler Nowicki <tnowicki at apple.com> wrote:
>> Modified: cfe/trunk/include/clang/Sema/AttributeList.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/AttributeList.h?rev=210925&r1=210924&r2=210925&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Sema/AttributeList.h (original)
>> +++ cfe/trunk/include/clang/Sema/AttributeList.h Fri Jun 13 12:57:25 2014
>> @@ -80,7 +80,9 @@ public:
>>    /// __declspec(...)
>>    AS_Declspec,
>>    /// __ptr16, alignas(...), etc.
>> -    AS_Keyword
>> +    AS_Keyword,
>> +    /// #pragma ...
>> +    AS_Pragma
>>  };
>>
>>
>> Amusingly, this new enumerator overflows the bitfield used to store the
>> Syntax value:
>>
>>  /// The number of expression arguments this attribute has.
>>  /// The expressions themselves are stored after the object.
>>  unsigned NumArgs : 15;
>>
>>  /// Corresponds to the Syntax enum.
>>  unsigned SyntaxUsed : 2;
>>
>>
>> Yoiks! Good catch, Doug. It seems we need a bit more test coverage. ;-)
>>
>>
>> Yet more amusingly, extending this bit-ield to three bits breaks things, as
>> does replacing the one source of AS_Syntax attributes with AS_GNU :)
>>
>>
>> I will investigate, but what "things" does it break when you extend
>> the bit field to three bits? I modified NumArgs : 15 (down one from
>> 16) and SyntaxUsed : 3 (up one from 2), and all tests passed for me
>> with MSVC on Windows (not that test coverage is the end-all, be-all).
>>
>>
>> … and it was apparently a bogeyman. Moving to three bits doesn’t actually
>> break anything.
>
> Phew!
>
> Unfortunately, testing this specifically is kind of hard to do since
> (UB issues aside), it would only manifest itself by turning a valid
> attribute into an invalid one, which is kind of hard to test for
> because only a few places worry about invalid attributes, and none of
> them can be triggered by a pragma-based attribute yet. If someone has
> suggestions, I would love to add a test for this. However, it's
> important enough to fix that I'm willing to commit a change without a
> test.
>
> ~Aaron




More information about the cfe-commits mailing list