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
