<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Apr 3, 2013, at 3:00 PM, Nico Weber <<a href="mailto:thakis@chromium.org">thakis@chromium.org</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;"><div dir="ltr"><div>Thanks for the review! Replies below:</div><br><div class="gmail_extra"><div class="gmail_quote">On Wed, Apr 3, 2013 at 2:56 PM, jahanian<span class="Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:fjahanian@apple.com" target="_blank">fjahanian@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;"><br><div><div><div class="h5"><div>On Apr 3, 2013, at 2:46 PM, Nico Weber <<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>> wrote:</div><br><blockquote type="cite"><div style="letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px;"><div dir="ltr">Hi,<div><br></div><div>the attached patch changes the attributes diag I added earlier today from</div><div><br></div><div><div> <span class="Apple-converted-space"> </span>test2.mm:3:12: error: postfix attributes are not allowed on Objective-C directives</div><div><br></div><div>to </div><div><br></div><div><div> <span class="Apple-converted-space"> </span>test2.mm:3:12: error: postfix attributes are not allowed on Objective-C directives, place them in front of '@interface'</div><div><br></div><div>for @interface and @protocol (and keeps the diagnostic as is for @class and @implementation).</div></div></div></div></div></blockquote><div><br></div></div></div>What is the rational for hint for @interface/@protocol and not others ?</div></div></blockquote><div><br></div><div>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.<br></div><div> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;"><div>-@implementation EXP I @end // expected-error {{postfix attributes are not allowed on Objective-C directives}}<br></div><div><div>+@implementation EXP I @end // expected-error-re {{postfix attributes are not allowed on Objective-C directives$}}</div><div>                                                                             <span class="Apple-converted-space"> </span>????                                                                                        ??????</div></div></div></blockquote><div><br></div><div>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.)</div></div></div></div></div></blockquote><div><br></div>Good to know thanks.</div><div><br><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;"><div>-@class EXP OC; // expected-error {{postfix attributes are not allowed on Objective-C directives}}<br></div><div><div>+@class EXP OC; // expected-error-re {{postfix attributes are not allowed on Objective-C directives$}}</div><div><br></div><div>These don’t look right.</div><div><br></div><div>Also, why can’t you pass the token down instead of adding and using "enum ObjCAttrSkipHint”?</div></div></div></blockquote><div><br></div><div>Seemed cleaner to me, I can pass the token instead if you like that better.</div></div></div></div></div></blockquote><div><br></div>Yes please. No point in defining a new enum. With that, lgtm.</div><div>- Fariborz</div><div><br></div><br></body></html>