aaron.ballman added inline comments.
================
Comment at: lib/Parse/ParseStmt.cpp:102
ParsedAttributesWithRange Attrs(AttrFactory);
+ MaybeParseGNUAttributes(Attrs);
MaybeParseCXX11Attributes(Attrs, nullptr, /*MightBeObjCMessageSend*/ true);
----------------
xbolva00 wrote:
> xbolva00 wrote:
> > Maybe we check if Tok is kw__attribute and look ahead a few tokens to check
> > if attribute name is fallthough in
> > ParseStatementOrDeclarationAfterAttributes.
> >
> > Now, we always fall into
> >
> > if ((getLangOpts().CPlusPlus || getLangOpts().MicrosoftExt ||
> > (StmtCtx & ParsedStmtContext::AllowDeclarationsInC) !=
> > ParsedStmtContext()) &&
> > isDeclarationStatement()) {
> > SourceLocation DeclStart = Tok.getLocation(), DeclEnd;
> > DeclGroupPtrTy Decl =
> > ParseDeclaration(DeclaratorContext::BlockContext,
> > DeclEnd, Attrs);
> > return Actions.ActOnDeclStmt(Decl, DeclStart, DeclEnd);
> > }
> Then we go to "ParseSimpleDeclaration" -> "ParseDeclarationSpecifier". What
> is quite strange for me, we do not set "Attrs" in ParseSimpleDeclaration from
> DS.getAttributes()..
> Maybe we check if Tok is kw__attribute and look ahead a few tokens to check
> if attribute name is fallthough in ParseStatementOrDeclarationAfterAttributes.
Why not check whether the attribute is a statement attribute or a declaration
attribute, rather than tying it to fallthrough specifically?
But the result then would be: if it's a statement attribute, we should be
trying to parse it as a statement rather than declaration statement; if it's a
declaration attribute, we should try to parse as a declaration. This seems very
much like spooky action at a distance -- attributes should not dictate whether
something is parsed as a decl or a stmt.
That said, this really is the right place for that code to go, I think. We've
never had a GNU null attribute statement before, so the parser may be written
in an inconvenient way to support that (IIRC, we have similar deficiencies with
things like attributes appertaining to declaration specifiers -- we don't have
attributes that need it yet, so support was never added for it).
This may require a bit more surgery to fix up, unfortunately.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63299/new/
https://reviews.llvm.org/D63299
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
