[PATCH] D121245: [clang][parser] Allow GNU attributes before namespace identifier
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 11 07:21:40 PST 2022
aaron.ballman added a comment.
Oh wow, GCC has supported this for a long time; I wasn't aware of that. Thanks for this patch!
================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:79
+
+ auto ReadLabelAttrubutes = [&] {
+ // Read label attributes, if present.
----------------
However, I don't think there's a reason we need this lambda -- it seems we can call `MaybeParseGNUAttributes()` instead, and get the attribute location from the `ParsedAttributesWithRange` object passed in.
================
Comment at: clang/test/Parser/namespace-attributes.cpp:3
+
+namespace __attribute__((visibility("hidden"))) A
+{
----------------
Another test case that I think we need is to test attribute parsing order between the different syntaxes. e.g.,
```
namespace __attribute__(()) [[]] A
{
}
namespace [[]] __attribute__(()) B
{
}
```
It's not clear to me that we want to match how GCC handles this case: https://godbolt.org/z/e1bjcEje6 but instead handle it more like how Clang handles it in structure declarations: https://godbolt.org/z/drGhb65ET.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121245/new/
https://reviews.llvm.org/D121245
More information about the cfe-commits
mailing list