[PATCH] D93630: [Attr] Apply GNU-style attributes to expression statements

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 11 06:09:05 PST 2021


aaron.ballman added a comment.

In D93630#2486574 <https://reviews.llvm.org/D93630#2486574>, @aaron.ballman wrote:

> I'm going to see if I can run the patch against our internal corpus here at work to see if it shakes out any breakages to real world code. I've not tried this before, so I have no idea how long it will take before I get results back, but I'll let you know when I have them.

The tests finally finished over the weekend and came back with no new failures -- that gives me a lot more confidence that this doesn't accidentally change behavior in cases users are likely to hit (at least in C and C++). I think the next steps are to clean up the tests and then coordinate with the GCC folks to ensure they don't have any strong objections we should be aware of.



================
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}}
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93630



More information about the cfe-commits mailing list