Nathan-Huckleberry added a comment. In D64838#1593516 <https://reviews.llvm.org/D64838#1593516>, @aaron.ballman wrote:
> 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). @xbolva00's patch https://reviews.llvm.org/D63260?id=204583 essentially does that already. I'm not sure how to continue on this patch, several solutions have been suggested, but I'm not sure which to implement. 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
