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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
