[patch] Make the ObjC attributes diagnostics a bit more informative

[jahanian]

On Apr 3, 2013, at 3:00 PM, Nico Weber <thakis at chromium.org> wrote:

> Thanks for the review! Replies below:
> 
> On Wed, Apr 3, 2013 at 2:56 PM, jahanian <fjahanian at apple.com> wrote:
> 
> On Apr 3, 2013, at 2:46 PM, Nico Weber <thakis at chromium.org> wrote:
> 
>> Hi,
>> 
>> the attached patch changes the attributes diag I added earlier today from
>> 
>>   test2.mm:3:12: error: postfix attributes are not allowed on Objective-C directives
>> 
>> to 
>> 
>>   test2.mm:3:12: error: postfix attributes are not allowed on Objective-C directives, place them in front of '@interface'
>> 
>> for @interface and @protocol (and keeps the diagnostic as is for @class and @implementation).
> 
> What is the rational for hint for @interface/@protocol and not others ?
> 
> Prefix attributes must be followed by interface or protocol; clang will diag on __attribute__ in front of @class and @implementation. So suggesting to move the attribute their seems wrong.
>  
> - at implementation EXP I @end // expected-error {{postfix attributes are not allowed on Objective-C directives}}
> + at implementation EXP I @end // expected-error-re {{postfix attributes are not allowed on Objective-C directives$}}
>                                                                               ????                                                                                        ??????
> 
> expected-error-re checks for a regular expression instead of just a string. The "$" matched "end of line", this is done to check that there's nothing following the word "directives" in the diag. (Since expected-error only checks for substring matches.)

Good to know thanks.

>  
> - at class EXP OC; // expected-error {{postfix attributes are not allowed on Objective-C directives}}
> + at class EXP OC; // expected-error-re {{postfix attributes are not allowed on Objective-C directives$}}
> 
> These don’t look right.
> 
> Also, why can’t you pass the token down instead of adding and using "enum ObjCAttrSkipHint”?
> 
> Seemed cleaner to me, I can pass the token instead if you like that better.

Yes please. No point in defining a new enum. With that, lgtm.
- Fariborz


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130403/14bbb460/attachment.html>