[PATCH] D64838: [Attr] Support _attribute__ ((fallthrough))

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D64838#1592520 <https://reviews.llvm.org/D64838#1592520>, @Nathan-Huckleberry wrote:

>   void foo() {
>     __attribute__((address_space(0))) *x;
>     *y;
>   }
>
>
> If the attributes are parsed then function body looks like this to the parser:
>
>   {
>     *x; //this one has attributes now
>     *y;
>   {
>
>
> The first line should be a valid declaration and the second like should be a dereference of an uninitialized variable. If the attributes token is discarded before parsing the rest of the line the only way to differentiate these is by looking at the attributes added to them.
>
> An alternative may be parse the attributes list and immediately try to parse as a declaration then if that parsing fails attempt to parse as something else. Although this approach also has the scary implication of things that are supposed to be declarations getting reparsed as something entirely different.


The issue is that a leading GNU-style attribute is not sufficient information to determine whether we're parsing a declaration or a statement; it shouldn't always be treated as a decl-specifier. I spoke with a GCC dev about how they handle this, and effectively, they parse the attributes first then attempt to parse a declaration; if that fails, they fall back to parsing a statement. I think the way forward for us that should be similar is to parse the attributes first and then wait until we see a decl-specifier before determining whether we want to parse a declaration or a statement, and attach the attributes after we've figured out which production we have. @rsmith may have even better approaches in mind, but we're definitely agreed that we should not parse statement/decl based on attribute identity. I would hope we could find a way to avoid lots of re-parsing work if we can (even to the point of perhaps breaking the `address_space` case because implicit int is pretty horrible to rely on in the first place; it depends on whether breaking that will break a lot of code or not).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64838/new/

https://reviews.llvm.org/D64838