[PATCH] D147989: [clang] Fix Attribute Placement

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.

Thank you for working on this! I spotted a little more that needs to be done, but this is pretty close to ready. I *think* the precommit CI failures are unrelated to this change, but you should double-check just to be sure.



================
Comment at: clang/lib/Sema/SemaDecl.cpp:5031
+static unsigned GetDiagnosticTypeSpecifierID(const DeclSpec &DS) {
+  DeclSpec::TST T=DS.getTypeSpecType();
   switch (T) {
----------------
Style nit; NFC


================
Comment at: clang/lib/Sema/SemaDecl.cpp:5042-5046
+    if (const EnumDecl *ED = dyn_cast<EnumDecl>(DS.getRepAsDecl())) {
+      if (ED->isScopedUsingClassTag())
+        return 5;
+    }
     return 4;
----------------
There's a subtle bug hiding in here, consider code like:
```
 __attribute__((visibility("hidden")))  __attribute__((aligned)) enum struct E {};
```
The diagnostic will emit `enum` because the `EnumDecl` is scoped using the `struct` tag rather than the `class` tag. I think you should add another entry to the `%select` for `enum struct` and return that value here if `ED->isScoped()` is true, and only return `4` for a plain `enum`.


================
Comment at: clang/test/Sema/attr-declspec-ignored.c:23
+__attribute__((visibility("hidden")))  __attribute__((aligned)) enum F {F} f;
\ No newline at end of file

----------------
I'm not seeing what changed with this line, I think  you can revert the changes to this file.


================
Comment at: clang/test/SemaCXX/attr-declspec-ignored.cpp:12
   // expected-warning{{attribute 'aligned' is ignored, place it after "enum" to apply attribute to type declaration}}
+  __attribute__((visibility("hidden")))  __attribute__((aligned)) enum class EC {}; // expected-warning{{attribute 'visibility' is ignored, place it after "enum class" to apply attribute to type declaration}} \
+  // expected-warning{{attribute 'aligned' is ignored, place it after "enum class" to apply attribute to type declaration}}
----------------
Once you fix the `enum struct` diagnostic above, I'd recommend adding some `enum struct` test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147989