aaron.ballman added inline comments.

================
Comment at: clang/test/Parser/stmt-attributes.c:5
+
+  __attribute__((unknown_attribute));            // expected-warning {{unknown 
attribute 'unknown_attribute' ignored}}
+  __attribute__((unknown_attribute)) {}          // expected-warning {{unknown 
attribute 'unknown_attribute' ignored}}
----------------
vsavchenko wrote:
> aaron.ballman wrote:
> > Given that these are parser tests, I think you can drop the attribute name 
> > itself except in the cases where you explicitly want to test that an 
> > unknown attribute introduces the start of a declaration (or things where 
> > the attribute identifier matters for some reason). This will reduce the 
> > amount of `expected-warning` comments we need to add.
> > 
> > Also, because this change impacts C++ and ObjC/C++ as well, I think we 
> > should have some tests for language-specific constructs like lambdas, 
> > range-based for loops, blocks, etc.
> This test is mostly a copy-paste of a similar test for C++11 statement 
> attributes.
> I think that these cases show that GNU spelling for attributes doesn't break 
> anything if we put it in front of these various statements.  And you are 
> absolutely right that we need to add more cases for C++ and Obj-C constructs. 
>  But I don't understand about 'unknown_attribute'.  What should I replace it 
> with?
> But I don't understand about 'unknown_attribute'. What should I replace it 
> with?

You can use `__attribute__(())` to test that we parse a GNU style attribute in 
a position without bothering to name any attributes. This will eliminate the 
"unknown attribute" sema warnings from the test without impacting the parser 
test functionality, and it'll make it more clear in what cases the specific 
attribute named impacts the parsing behavior.

e.g., `__attribute__((unknown_attribute)) if (0) {}` converted into 
`__attribute__(()) if (0) {}` should still test what we need tested.


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