[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