<div dir="ltr"><div>+ SourceLocation Loc = Tok.getLocation();</div><div><br></div><div>Unused variable.</div><div><br></div><div><div>+ // If the SubStmt is invalid, we want the attributes to attach to the</div><div>
+ // label instead.</div><div>+ if (!SubStmt.isUsable() || SubStmt.isInvalid())</div><div>+ attrs.takeAllFrom(TempAttrs);</div></div><div><br></div><div>I thought your experiments showed the attributes were ill-formed in this case. We don't want to move the attributes onto the label if we've already diagnosed them on a pragma. If the substatement is neither usable nor invalid here, we can just call ParseStatement(), since we've already used up our attributes by handing them to ParseStatementOrDeclarationAfterAttributes. Something like:</div>
<div><br></div><div><div> ParsedAttributesWithRange TempAttrs(AttrFactory);</div><div> ParseGNUAttributes(TempAttrs);</div><div> StmtResult SubStmt;</div><div> if (!TempAttrs.empty()) {</div><div> if (Tok.is(tok::semi))<br>
</div><div> attrs.takeAllFrom(TempAttrs);</div><div> else {</div><div> StmtVector Stmts;<br></div><div> SubStmt = ParseStatementOrDeclarationAfterAttributes(Stmts, true, 0, TempAttrs);</div><div> }</div>
<div> }</div><div> if (!SubStmt.isInvalid() && !SubStmt.isUsable())<br></div><div> SubStmt = ParseStatement();</div></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Oct 9, 2013 at 6:11 AM, 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">It figures that I'd hit the wrong case. ;-) Thank you for the<br>
explanation. New patch attached.<br>
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On Wed, Oct 9, 2013 at 3:10 AM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>
><br>
> On 8 Oct 2013 14:22, "Aaron Ballman" <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br>
>><br>
>> On Mon, Oct 7, 2013 at 4:41 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
>> wrote:<br>
>> > On Mon, Oct 7, 2013 at 1:21 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>><br>
>> > wrote:<br>
>> >><br>
>> >> On Mon, Oct 7, 2013 at 4:16 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
>> >> wrote:<br>
>> >> > + // Read label attributes, if present. TempAttrs will contain both<br>
>> >> > C++11<br>
>> >> > and<br>
>> >> > + // GNU attributes (if present) after this point.<br>
>> >> ><br>
>> >> > This isn't true: we're only parsing GNU attributes here.<br>
>> >><br>
>> >> Sorry, that's a holdover from the existing comment. I'll correct it.<br>
>> >> ><br>
>> >> > + } else {<br>
>> >> > + StmtVector Stmts;<br>
>> >> > + SubStmt = ParseStatementOrDeclarationAfterAttributes(Stmts,<br>
>> >> > true,<br>
>> >> > 0,<br>
>> >> > +<br>
>> >> > TempAttrs);<br>
>> >> ><br>
>> >> > This doesn't look like it'll correctly handle the case where<br>
>> >> > ParseStatementOrDeclarationAfterAttributes returns null (because we<br>
>> >> > parsed a<br>
>> >> > pragma or the like). If you're relying on this being impossible<br>
>> >> > (because<br>
>> >> > an<br>
>> >> > attribute can't apply to a #pragma), please insert an assert here<br>
>> >> > (that<br>
>> >> > SubStmt is invalid or usable).<br>
>> >><br>
>> >> Doesn't that get handled slightly further down?<br>
>> >><br>
>> >> // Broken substmt shouldn't prevent the label from being added to the<br>
>> >> AST.<br>
>> >> if (SubStmt.isInvalid())<br>
>> >> SubStmt = Actions.ActOnNullStmt(ColonLoc);<br>
>> ><br>
>> ><br>
>> > This does not handle the case where SubStmt is valid but null.<br>
>> > ParseStatementOrDeclarationAfterAttributes returns such a value if it<br>
>> > hits a<br>
>> > #pragma.<br>
>><br>
>> What I am finding in practice is that ConsumeToken for the colon is<br>
>> also consuming the #pragma in this test case:<br>
>><br>
>> void f() {<br>
>> // Invalid<br>
>> D: // expected-warning {{unused label 'D'}}<br>
>> #pragma message "stuff" // expected-warning {{stuff}}<br>
>> __attribute__((unused)) // expected-error {{expected expression}}<br>
>> ;<br>
>> }<br>
>><br>
>> So ParseStatementOrDeclarationAfterAttributes does not return a null<br>
>> statement because it never sees the #pragma. I'm not certain of the<br>
>> proper way to proceed as there appears to be no way to tell<br>
>> ConsumeToken to not consume the pragma.<br>
><br>
> There are two very different kinds of pragmas: those that take effect when<br>
> we lex them (these are handled in the lexer and never reach the token<br>
> stream) and those that take effect when we parse them (these can only appear<br>
> where a declaration can appear, and get translated into tokens). You got<br>
> unlucky and picked a pragma of the first kind. Try #pragma weak instead.<br>
</div></div></blockquote></div><br></div>