[PATCH] Attribute parsing following labels

[Aaron Ballman]

It figures that I'd hit the wrong case.  ;-)  Thank you for the
explanation.  New patch attached.

~Aaron

On Wed, Oct 9, 2013 at 3:10 AM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> On 8 Oct 2013 14:22, "Aaron Ballman" <aaron at aaronballman.com> wrote:
>>
>> On Mon, Oct 7, 2013 at 4:41 PM, Richard Smith <richard at metafoo.co.uk>
>> wrote:
>> > On Mon, Oct 7, 2013 at 1:21 PM, Aaron Ballman <aaron at aaronballman.com>
>> > wrote:
>> >>
>> >> On Mon, Oct 7, 2013 at 4:16 PM, Richard Smith <richard at metafoo.co.uk>
>> >> wrote:
>> >> > +  // Read label attributes, if present. TempAttrs will contain both
>> >> > C++11
>> >> > and
>> >> > +  // GNU attributes (if present) after this point.
>> >> >
>> >> > This isn't true: we're only parsing GNU attributes here.
>> >>
>> >> Sorry, that's a holdover from the existing comment.  I'll correct it.
>> >> >
>> >> > +    } else {
>> >> > +      StmtVector Stmts;
>> >> > +      SubStmt = ParseStatementOrDeclarationAfterAttributes(Stmts,
>> >> > true,
>> >> > 0,
>> >> > +
>> >> > TempAttrs);
>> >> >
>> >> > This doesn't look like it'll correctly handle the case where
>> >> > ParseStatementOrDeclarationAfterAttributes returns null (because we
>> >> > parsed a
>> >> > pragma or the like). If you're relying on this being impossible
>> >> > (because
>> >> > an
>> >> > attribute can't apply to a #pragma), please insert an assert here
>> >> > (that
>> >> > SubStmt is invalid or usable).
>> >>
>> >> Doesn't that get handled slightly further down?
>> >>
>> >>   // Broken substmt shouldn't prevent the label from being added to the
>> >> AST.
>> >>   if (SubStmt.isInvalid())
>> >>     SubStmt = Actions.ActOnNullStmt(ColonLoc);
>> >
>> >
>> > This does not handle the case where SubStmt is valid but null.
>> > ParseStatementOrDeclarationAfterAttributes returns such a value if it
>> > hits a
>> > #pragma.
>>
>> What I am finding in practice is that ConsumeToken for the colon is
>> also consuming the #pragma in this test case:
>>
>> void f() {
>> // Invalid
>> D: // expected-warning {{unused label 'D'}}
>> #pragma message "stuff"  // expected-warning {{stuff}}
>> __attribute__((unused))  // expected-error {{expected expression}}
>> ;
>> }
>>
>> So ParseStatementOrDeclarationAfterAttributes does not return a null
>> statement because it never sees the #pragma.  I'm not certain of the
>> proper way to proceed as there appears to be no way to tell
>> ConsumeToken to not consume the pragma.
>
> There are two very different kinds of pragmas: those that take effect when
> we lex them (these are handled in the lexer and never reach the token
> stream) and those that take effect when we parse them (these can only appear
> where a declaration can appear, and get translated into tokens). You got
> unlucky and picked a pragma of the first kind. Try #pragma weak instead.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: LabelAttr.patch
Type: application/octet-stream
Size: 3423 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131009/9c9b0176/attachment.obj>