[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