aaron.ballman added a comment.

In D93630#2468241 <https://reviews.llvm.org/D93630#2468241>, @vsavchenko wrote:

> In D93630#2468197 <https://reviews.llvm.org/D93630#2468197>, @aaron.ballman 
> wrote:
>
>> However, taking a step back -- what attributes would need this functionality 
>> (and couldn't be written on something within the expression statement)?
>
> It is still good old `suppress`, it is very counter-intuitive when you put 
> `suppress` and it causes some weird parse errors: https://godbolt.org/z/zzY64q

Yeah, I kind of figured that might be the cause. I'm not 100% convinced (one 
way or the other) if the suppress attribute should get a GNU spelling. The 
`[[]]` spellings are available in all language modes (we have 
`-fdouble-square-bracket-attributes` to enable this) and don't run afoul of the 
"guess what this attribute appertains to" problem that GNU-style attributes do.



================
Comment at: clang/lib/Parse/ParseStmt.cpp:213
              ParsedStmtContext()) &&
-        (GNUAttributeLoc.isValid() || isDeclarationStatement())) {
+        ((GNUAttributeLoc.isValid() && !Attrs.back().isStmtAttr()) ||
+         isDeclarationStatement())) {
----------------
I think you need to ensure there's at least one attribute before checking 
`!Attrs.back().isStmtAttr()` as this may cause problems if the user does 
something odd like `__attribute__(()) int x;` (I don't know if this will result 
in a valid `GNUAttributeLoc` with no attributes or not.)

I'm not certain that logic is perfect either. It would be pretty mysterious to 
handle these cases differently:
```
__attribute__((stmt_attr, decl_attr)) int a, b, c;
__attribute__((decl_attr, stmt_attr)) int x, y, z;
```


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

Reply via email to