On Mon, Jul 21, 2014 at 2:03 PM, Aaron Ballman <[email protected]> wrote:
> 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? > I think that we need something like this to reach that code: void f(int a[static [[]] 5]); ... where we produce an error on the 'static', but don't reject the attribute list. > 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
