I've cleaned the patch up and implemented your suggestions. Just to clarify something:
> Index: lib/Parse/ParseDecl.cpp > =================================================================== > --- lib/Parse/ParseDecl.cpp (revision 213263) > +++ lib/Parse/ParseDecl.cpp (working copy) > @@ -5535,7 +5541,7 @@ > // If there is a type-qualifier-list, read it now. > // Type qualifiers in an array subscript are a C99 feature. > DeclSpec DS(AttrFactory); > - ParseTypeQualifierListOpt(DS, false /*no attributes*/); > + ParseTypeQualifierListOpt(DS, NoAttributesAllowed); The old code used to allow C++11-style attributes, but from what I can tell, those should have been prohibited here as well, so I switched to NoAttributesAllowed. Is my understanding correct? Is there a sensible testcase I should add for this? Thanks! Assuming my understanding is correct, I intend to commit after 3.5 branches. ~Aaron On Thu, Jul 17, 2014 at 6:40 PM, Richard Smith <[email protected]> wrote: > On Thu, Jul 17, 2014 at 11:36 AM, Aaron Ballman <[email protected]> > wrote: >> >> GNU-style type attributes are not allowed in new expressions, such as: >> >> auto *P = new int * __attribute__((attr)); >> >> We silently accept them in clang, but have a FIXME in the code to >> address the issue. This patch is an initial stab at addressing the >> issue, and I am looking for feedback on whether my approach is >> reasonable or not. (It disables support for GNU attributes, but not >> other vendor attributes which should remain supported.) >> >> Note, the attached test case does not currently pass. It only checks >> for one error diagnostic, but the actual diagnostics for that code >> are: >> >> ..\llvm\tools\clang\test\SemaCXX\attr-gnu.cpp:4:22: error: an >> attribute list cannot appear here >> auto P = new int * __attribute__((vector_size(8))); // expected-error >> ... >> ^ >> ..\llvm\tools\clang\test\SemaCXX\attr-gnu.cpp:4:21: error: expected >> ';' at end of declaration >> auto P = new int * __attribute__((vector_size(8))); // expected-error >> ... >> ^ >> ; >> ..\llvm\tools\clang\test\SemaCXX\attr-gnu.cpp:4:22: warning: >> declaration does not declare anything [-Wmissing-declarations] >> auto P = new int * __attribute__((vector_size(8))); // expected-error >> ... >> ^~~~~~~~~~~~~ >> 1 warning and 2 errors generated. >> >> The first diagnostic is obviously correct, but are the other two >> reasonable diagnostics as well? > > > I would prefer for them to be suppressed: if you're in the > GNUAttributesProhibited case and you see a GNU attribute, I think you should > both parse it and diagnose it. > >> If the approach is reasonable, I'll clean the patch up, add more test >> cases, etc. I mostly wanted to check my direction first. Thanks! > > > The direction looks good to me. I think your AttrRequirements enum could be > made clearer (separate XAllowed and XProhibited flags seem a bit confusing > at first -- perhaps XParsed and XParsedAndRejected or something along those > lines might be clearer?) _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
