<div dir="ltr"><div>I made a few tweaks to the patch as I was reviewing, and I figured it'd be easier to just check that in than have you make the same tweaks and check in, so: committed with fixes for the below review comments as r194869.</div>
<div><br></div><div>--- lib/Parse/ParseStmt.cpp<span style="white-space:pre-wrap"> </span>(revision 193647)<br></div><div>+++ lib/Parse/ParseStmt.cpp<span style="white-space:pre-wrap"> </span>(working copy)</div><div>[...]</div>
<div>+ // attribute is immediately followed by a semi-colon to disambiguate it from<br></div><div>+ // attributes on declarations.</div><div><br></div><div>This isn't quite right: in C, the attributes *always* apply to the label (because a label cannot apply to a declaration). GCC only requires a semicolon after the label in C++ mode.</div>
<div><br></div><div><br></div><div>+ StmtResult SubStmt;</div><div>+ if (Tok.is(tok::kw___attribute)) {</div><div>+ ParsedAttributesWithRange TempAttrs(AttrFactory);</div><div>+ ParseGNUAttributes(TempAttrs);</div>
<div> </div><div>- StmtResult SubStmt(ParseStatement());</div><div>+ if (!TempAttrs.empty()) {</div><div><br></div><div>With this check, "A: __attribute__(( )) int n;" won't parse the "int n;", and "A: __attribute__(( )) f();" won't trigger an error as it should. We should do what follows regardless, because we know we saw a kw___attribute.</div>
<div><br></div><div><br></div><div>+ if (Tok.is(tok::semi))</div><div>+ attrs.takeAllFrom(TempAttrs);</div><div><br></div><div>We need to actually parse the substatement in this case.</div><div><br></div><div>
<br></div>
<div>+ } else<br></div><div>+ Diag(Tok, diag::err_expected_semi_after) << "__attribute__";</div><div><br></div><div>Again, we need to parse the substatement in this case.</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Oct 30, 2013 at 3:16 PM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Finally have some spare time to look into this again, thank you for<br>
your patience. :-)<br>
<div class="im"><br>
On Tue, Oct 15, 2013 at 12:46 AM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>
> + SourceLocation Loc = Tok.getLocation();<br>
><br>
> Unused variable.<br>
><br>
> + // If the SubStmt is invalid, we want the attributes to attach to the<br>
> + // label instead.<br>
> + if (!SubStmt.isUsable() || SubStmt.isInvalid())<br>
> + attrs.takeAllFrom(TempAttrs);<br>
><br>
> I thought your experiments showed the attributes were ill-formed in this<br>
> case.<br>
<br>
</div>Since it's been a while, I went back to re-test my experiments, and I<br>
think I made a mistake.<br>
<br>
void f() {<br>
A:<br>
__attribute__((unused)) int i; // attribute applies to variable<br>
B:<br>
__attribute__((unused)); // attribute applies to label<br>
<br>
C:<br>
__attribute__((unused))<br>
#pragma weak unused_local_static<br>
;<br>
<br>
D:<br>
#pragma weak unused_local_static<br>
__attribute__((unused))<br>
;<br>
}<br>
<br>
*Both* of these cases generate errors in gcc. What I didn't realize is<br>
that gcc stopped trying to report errors with D because C was<br>
ill-formed.<br>
<br>
With D commented out, I get:<br>
<br>
prog.cpp: In function ‘void f()’:<br>
prog.cpp:8:7: error: expected primary-expression before ‘__attribute__’<br>
__attribute__((unused))<br>
^<br>
prog.cpp:8:7: error: expected ‘;’ before ‘__attribute__’<br>
prog.cpp:9:39: error: expected ‘}’ before end of line<br>
#pragma weak unused_local_static<br>
^<br>
prog.cpp:2:5: warning: label ‘A’ defined but not used [-Wunused-label]<br>
A:<br>
^<br>
prog.cpp:7:5: warning: label ‘C’ defined but not used [-Wunused-label]<br>
C:<br>
^<br>
prog.cpp: At global scope:<br>
prog.cpp:9:39: error: expected declaration before end of line<br>
#pragma weak unused_local_static<br>
<br>
With C commented out, I get:<br>
<br>
prog.cpp: In function ‘void f()’:<br>
prog.cpp:14:7: error: expected primary-expression before ‘__attribute__’<br>
__attribute__((unused))<br>
^<br>
prog.cpp:14:7: error: expected ‘;’ before ‘__attribute__’<br>
prog.cpp:2:5: warning: label ‘A’ defined but not used [-Wunused-label]<br>
A:<br>
^<br>
prog.cpp:12:5: warning: label ‘D’ defined but not used [-Wunused-label]<br>
D:<br>
^<br>
<br>
So I've created a new patch that I believe has the correct behavior.<br>
If there's a semi-colon, the attributes are applied to the label. If<br>
there's a declaration, it is handled. Any other form of statement<br>
causes a specific error.<br>
<br>
Thanks!<br>
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span></blockquote></div><br></div>