Fixed in r222456 ~Aaron
On Thu, Nov 20, 2014 at 4:31 PM, Aaron Ballman <[email protected]> wrote: > On Thu, Nov 20, 2014 at 4:24 PM, Douglas Gregor <[email protected]> wrote: >> [Dropping Tyler, who is no longer at this address] >> >> On Nov 20, 2014, at 1:12 PM, Aaron Ballman <[email protected]> wrote: >> >> On Thu, Nov 20, 2014 at 3:59 PM, Douglas Gregor <[email protected]> wrote: >> >> >> On Jun 13, 2014, at 10:57 AM, Tyler Nowicki <[email protected]> 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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
