rsmith added inline comments.

================
Comment at: clang/lib/Sema/SemaDecl.cpp:15545-15546
+    // is only used for C code.
+    if (getLangOpts().MSVCCompat && (Kind == TTK_Enum) && Previous.empty() &&
+        (TUK == TUK_Reference)) {
+      LookupResult TypedefEnumLookup(*this, Name, NameLoc, LookupOrdinaryName,
----------------
Our coding style does not parenthesize `==` within `&&`.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:15551-15567
+      if (!TypedefEnumLookup.empty()) {
+        if (TypedefNameDecl *TD =
+                dyn_cast<TypedefNameDecl>(TypedefEnumLookup.getFoundDecl())) {
+          const Type *UnderlyingTypePtr =
+              TD->getUnderlyingType().getTypePtrOrNull();
+
+          if (UnderlyingTypePtr) {
----------------
This can be simplified substantially.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:15752-15754
+          bool AnonymousEnumEligible = getLangOpts().MSVCCompat &&
+                                       (Kind == TTK_Enum) &&
+                                       Tag->getDeclName().isEmpty();
----------------
In the anonymous union case, we also need to check for qualifiers on the 
typedef type, and more broadly perhaps we should be using 
`getAnonDeclWithTypedefName` here too. Presumably the new case should also only 
apply for `TUK_Reference`.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:15753
+          bool AnonymousEnumEligible = getLangOpts().MSVCCompat &&
+                                       (Kind == TTK_Enum) &&
+                                       Tag->getDeclName().isEmpty();
----------------



================
Comment at: clang/test/Sema/enum-typedef-msvc.c:15
+  (void)foo;
+}
----------------
Please also add a testcase for the situation where the use of `enum Foo` occurs 
in the same scope as the typedef and anonymous `enum` declaration. (MSVC has 
weird behavior here that we don't necessarily need to follow, but we should 
make sure we at least do something defensible.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91659

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D91659: A... Shivanshu Goyal via Phabricator via cfe-commits
    • [PATCH] D916... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] D916... Shivanshu Goyal via Phabricator via cfe-commits
    • [PATCH] D916... Shivanshu Goyal via Phabricator via cfe-commits

Reply via email to