[PATCH] Disallowing just GNU-style attributes in certain positions

[Aaron Ballman]

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 <richard at metafoo.co.uk> wrote:
> On Thu, Jul 17, 2014 at 11:36 AM, Aaron Ballman <aaron at aaronballman.com>
> 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?)