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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to