vsavchenko added a comment. In D93630#2479260 <https://reviews.llvm.org/D93630#2479260>, @aaron.ballman wrote:
> In D93630#2478977 <https://reviews.llvm.org/D93630#2478977>, @vsavchenko > wrote: > >> I guess I want to clarify one point here, after this patch the parser **will >> not assume** that statement always follows statement attributes. We simply >> turn off the assumption that what follows is a declaration, parser will >> simply determine whether it is a statement or a declaration, it can do it on >> its own without attribute's "help". > > We used to say that if there's a GNU attribute (or we're parsing a > declaration statement), what follows must be a declaration. Now we're using > the attributes in the given attribute list to decide whether what follows is > a declaration or a statement (if all of the attributes are statement > attributes, we don't treat what follows as being a declaration unless we're > in a declaration statement). Or am I misreading the code? We do not decide whether it is declaration or statement. I do get that it is a matter of perspective, but right now I prefer to read it like this: **BEFORE**: if we've seen a GNU-style attribute parse whatever comes next as a declaration **AFTER**: if we've seen a GNU-style attribute except for the case when all of the parsed attributes are known to be statement attributes, parse whatever comes next as a declaration So, essentially, when we see statement attributes only, attribute/declaration heuristic is canceled and we parse exactly the way we would've parsed as if no attributes present. > e.g., > > void foo(void) { > __attribute__((fallthrough)) i = 12; > } > > Before patch: > > F:\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe -fsyntax-only > "F:\Aaron Ballman\Desktop\test.c" > F:\Aaron Ballman\Desktop\test.c:2:32: warning: type specifier missing, > defaults to 'int' [-Wimplicit-int] > __attribute__((fallthrough)) i = 12; > ^ > F:\Aaron Ballman\Desktop\test.c:2:18: error: 'fallthrough' attribute cannot > be applied to a declaration > __attribute__((fallthrough)) i = 12; > ^ ~ > 1 warning and 1 error generated. > > After patch: > > F:\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe -fsyntax-only > "F:\Aaron Ballman\Desktop\test.c" > F:\Aaron Ballman\Desktop\test.c:2:32: error: use of undeclared identifier > 'i' > __attribute__((fallthrough)) i = 12; > ^ > 1 error generated. In both situations, the code contains errors. I don't think that this is a valid argument, to be honest. Actually, the nature of this change is that it starts to parse strictly MORE cases. > So I think this will cause issues for the suppression attribute you're > working on if we allow it to have a GNU spelling with valid C89 code like; > `__attribute__((suppress)) i = 12;` This is **not** a valid code if you remove `__attribute__((suppress))`. `i = 12` without a prior declaration of `i` will cause the `use of undeclared identifier 'i'` error. You can try it with any compiler: https://godbolt.org/z/P43nxn. I honestly don't see a scenario when the user has some piece of code that is parsed as a statement, then they mindfully add a //statement// attribute, and expect it to be parsed as a declaration after that. > or `__attribute__((suppress)) g() { return 12; }` which may be intended to > suppress the "type specifier missing" diagnostic (somewhat amusingly, since > the user could also just add the `int` to the declaration since they're > changing the code anyway). Again, we **do not force** the parser to parse whatever follows the statement attribute as a statement. `__attribute__((suppress))` will not change how we parse things here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93630/new/ https://reviews.llvm.org/D93630 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
