<div dir="ltr">On Mon, Oct 7, 2013 at 1:21 PM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Mon, Oct 7, 2013 at 4:16 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>

> +  // Read label attributes, if present. TempAttrs will contain both 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>
</div>Sorry, that's a holdover from the existing comment.  I'll correct it.<br>
<div class="im">><br>
> +    } else {<br>
> +      StmtVector Stmts;<br>
> +      SubStmt = ParseStatementOrDeclarationAfterAttributes(Stmts, true, 0,<br>
> +                                                           TempAttrs);<br>
><br>
> This doesn't look like it'll correctly handle the case where<br>
> ParseStatementOrDeclarationAfterAttributes returns null (because we parsed a<br>
> pragma or the like). If you're relying on this being impossible (because an<br>
> attribute can't apply to a #pragma), please insert an assert here (that<br>
> SubStmt is invalid or usable).<br>
<br>
</div>Doesn't that get handled slightly further down?<br>
<br>
  // Broken substmt shouldn't prevent the label from being added to the AST.<br>
  if (SubStmt.isInvalid())<br>
    SubStmt = Actions.ActOnNullStmt(ColonLoc);</blockquote><div><br></div><div>This does not handle the case where SubStmt is valid but null. ParseStatementOrDeclarationAfterAttributes returns such a value if it hits a #pragma.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">
><br>
> Oh, also: please add test cases for #pragmas appearing near the attributes.<br>
<br>
</div>Shall do.<br>
<br>
Thanks!<br>
<span class="HOEnZb"><font color="#888888"><br>
~Aaron<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
> On Sat, Oct 5, 2013 at 2:09 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>><br>
> wrote:<br>
>><br>
>> Here is another attempt at this patch, without the tentative parsing.<br>
>><br>
>> Thanks!<br>
>><br>
>> ~Aaron<br>
>><br>
>> On Tue, Oct 1, 2013 at 12:43 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
>> wrote:<br>
>> > OK, that makes some sense, since the pragma acts like a statement that<br>
>> > cannot have an attribute.<br>
>> ><br>
>> > On 1 Oct 2013 05:26, "Aaron Ballman" <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:<br>
>> >><br>
>> >> On Mon, Sep 30, 2013 at 6:03 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
>> >> wrote:<br>
>> >> > It should be possible to handle this case without tentative parsing<br>
>> >> > or<br>
>> >> > lookahead. Consume the attributes onto a new list; if the next token<br>
>> >> > is<br>
>> >> > a<br>
>> >> > semicolon, then push them into the label's attribute list; otherwise,<br>
>> >> > ParseStatementOrDeclarationAfterAttributes.<br>
>> >><br>
>> >> Ah, thanks!<br>
>> >><br>
>> >> > What does GCC do if there are #pragmas between the case label and the<br>
>> >> > attribute, and/or between the attribute and the semicolon?<br>
>> >><br>
>> >> void func(void) {<br>
>> >> label: __attribute__((unused))<br>
>> >> #pragma message "stuff"<br>
>> >> ;<br>
>> >> }<br>
>> >><br>
>> >> Prints the message, no warning about the label or the attribute.<br>
>> >><br>
>> >> void func(void) {<br>
>> >> label:<br>
>> >> #pragma message "stuff"<br>
>> >> __attribute__((unused))<br>
>> >> ;<br>
>> >> }<br>
>> >><br>
>> >> Prints the message, then prints an error about expecting an expression<br>
>> >> before the attribute, and a warning for the label being unused.<br>
>> >><br>
>> >> ~Aaron<br>
><br>
><br>
</div></div></blockquote></div><br></div></div>