[PATCH] Attribute parsing following labels

[Richard Smith]

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.


> >
> > Oh, also: please add test cases for #pragmas appearing near the
> attributes.
>
> Shall do.
>
> Thanks!
>
> ~Aaron
>
> > On Sat, Oct 5, 2013 at 2:09 PM, Aaron Ballman <aaron at aaronballman.com>
> > wrote:
> >>
> >> Here is another attempt at this patch, without the tentative parsing.
> >>
> >> Thanks!
> >>
> >> ~Aaron
> >>
> >> On Tue, Oct 1, 2013 at 12:43 PM, Richard Smith <richard at metafoo.co.uk>
> >> wrote:
> >> > OK, that makes some sense, since the pragma acts like a statement that
> >> > cannot have an attribute.
> >> >
> >> > On 1 Oct 2013 05:26, "Aaron Ballman" <aaron at aaronballman.com> wrote:
> >> >>
> >> >> On Mon, Sep 30, 2013 at 6:03 PM, Richard Smith <
> richard at metafoo.co.uk>
> >> >> wrote:
> >> >> > It should be possible to handle this case without tentative parsing
> >> >> > or
> >> >> > lookahead. Consume the attributes onto a new list; if the next
> token
> >> >> > is
> >> >> > a
> >> >> > semicolon, then push them into the label's attribute list;
> otherwise,
> >> >> > ParseStatementOrDeclarationAfterAttributes.
> >> >>
> >> >> Ah, thanks!
> >> >>
> >> >> > What does GCC do if there are #pragmas between the case label and
> the
> >> >> > attribute, and/or between the attribute and the semicolon?
> >> >>
> >> >> void func(void) {
> >> >> label: __attribute__((unused))
> >> >> #pragma message "stuff"
> >> >> ;
> >> >> }
> >> >>
> >> >> Prints the message, no warning about the label or the attribute.
> >> >>
> >> >> void func(void) {
> >> >> label:
> >> >> #pragma message "stuff"
> >> >> __attribute__((unused))
> >> >> ;
> >> >> }
> >> >>
> >> >> Prints the message, then prints an error about expecting an
> expression
> >> >> before the attribute, and a warning for the label being unused.
> >> >>
> >> >> ~Aaron
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131007/1e2808b2/attachment.html>