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

Nico Weber thakis at chromium.org
Wed Apr 3 15:00:34 PDT 2013


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.)


> - 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.


>
> - Fariborz
>
>
> Ok?
>
> Nico
> <clang-objc-attrs-2.patch>_______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130403/48c584f5/attachment.html>


More information about the cfe-commits mailing list