[cfe-commits] [PATCH][Review request] emit diagnostics for ignored attributes appear before class/struct/union keywords

[Michael Han]

Hi John,

Thanks for reviewing the patch over holidays. Please refer to the attached patch for the fixes.

Cheers
~Michael

-----Original Message-----
From: John McCall [mailto:rjmccall at apple.com] 
Sent: Tuesday, December 28, 2010 2:46 PM
To: Michael Han
Cc: cfe-commits at cs.uiuc.edu
Subject: Re: [PATCH][Review request] emit diagnostics for ignored attributes appear before class/struct/union keywords

On Dec 21, 2010, at 8:56 PM, Michael Han wrote:
> The attached patch adds a diagnostic kind and makes parser to emit warning diagnostic when it found there are attributes appear before class/struct/union keywords. For gcc compatibility, I have checked that gcc does emit warnings for such ignored attributes so this patch would enable clang to emit similar warnings. 

First off, thanks for working on this, and I apologize for the long delay in reviewing your patch.

I have a few things I'd like to see fixed before this is committed:

1)  The diagnostic should point to the location of the attribute, rather than the location of the start of the declaration specifiers, which is not always the same thing.

2)  Your proposed diagnostic message is ungrammatical;  allow me to suggest the following alternative:
  "attribute is ignored;  place it after \"%select{class|struct|union|enum}0\" to apply it to the type declaration"

3)  That's not an appropriate place to trigger the warning:  first, it will only trigger in C++, and second, it will trigger even when the attribute is not ignored, e.g. in the following code:
  __attribute__((visibility(hidden))) struct A foo;
The appropriate place to trigger this warning is in Sema::ParsedFreeStandingDeclSpec when Tag exists and is not invalid.

John.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: attr.patch
Type: application/octet-stream
Size: 5568 bytes
Desc: attr.patch
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20101229/f7c4f2f8/attachment.obj>