aaron.ballman added a comment.
In D64838#1589770 <https://reviews.llvm.org/D64838#1589770>,
@Nathan-Huckleberry wrote:
> The main problem that we have is that the `__attribute__` token always causes
> the parser to read the line as a declaration. Then the declaration parser
> handles reading the attributes list.
>
> This case demonstrates the problem:
>
> void foo() {
> __attribute__((address_space(0))) *x;
> }
>
>
> If the attribute token is read beforehand this code just becomes `*x` and is
> parsed as a dereference of an undefined variable when it should actually be a
> declaration.
>
> Maybe the best solution is to pull the attributes parsing out to
> `ParseStatementOrDeclaration` then pass those attributes through to following
> functions.
> When the determination is made whether a line is a declaration or a
> statement the attributes list can be looked at to determine if the attributes
> are statement or declaration attributes and continue accordingly. Maybe
> throwing a warning if mixing of declaration and statement attributes occur.
>
> Does that sound like a good solution?
Please see the discussion in https://reviews.llvm.org/D63299#inline-564887 --
if I understand your approach properly, this approach leads to spooky action at
a distance, where the name of the attribute dictates whether something is a
statement or a declaration. I'm not comfortable with that. There's no reason we
could not have an attribute that is both a statement attribute and a
declaration attribute, and how would *that* parse?
I think this requires some deeper surgery in the parsing logic so that
attributes do not impact whether something is treated as a declaration or a
statement. The parser should parse attributes first, keep them around for a
while, and attach them to either the decl or the statement at a later point.
================
Comment at: clang/lib/Parse/ParseStmt.cpp:104
+
+ if (isNullStmtWithAttributes()) {
+ MaybeParseGNUAttributes(Attrs);
----------------
efriedma wrote:
> If we're going to generally support statement attributes, it should be
> possible to apply them to non-null statements. (Even if there aren't any
> valid attributes right now, that's likely to change in the future.) Or is
> that not possible to implement for some reason?
We already generally support statement attributes. See `ProcessStmtAttribute()`
in SemaStmtAttr.cpp. They definitely need to apply to non-null statements.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64838/new/
https://reviews.llvm.org/D64838
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
