[PATCH] Attribute parsing following labels

[Richard Smith]

+      SourceLocation Loc = Tok.getLocation();

Unused variable.

+      // If the SubStmt is invalid, we want the attributes to attach to the
+      // label instead.
+      if (!SubStmt.isUsable() || SubStmt.isInvalid())
+        attrs.takeAllFrom(TempAttrs);

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:

  ParsedAttributesWithRange TempAttrs(AttrFactory);
  ParseGNUAttributes(TempAttrs);
  StmtResult SubStmt;
  if (!TempAttrs.empty()) {
    if (Tok.is(tok::semi))
      attrs.takeAllFrom(TempAttrs);
    else {
      StmtVector Stmts;
      SubStmt = ParseStatementOrDeclarationAfterAttributes(Stmts, true, 0,
TempAttrs);
    }
  }
  if (!SubStmt.isInvalid() && !SubStmt.isUsable())
    SubStmt = ParseStatement();


On Wed, Oct 9, 2013 at 6:11 AM, Aaron Ballman <aaron at aaronballman.com>wrote:

> 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 --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131014/995598ee/attachment.html>