[PATCH] D91659: Allow anonymous enum typedefs to be referenced with the 'enum' specifier under MSVC compat mode

[Shivanshu Goyal via Phabricator via cfe-commits]

shivanshu3 created this revision.
shivanshu3 added reviewers: zahen, tiagoma, rnk, hans.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
shivanshu3 requested review of this revision.

Goal: Clang should be able to parse the following code with '-fms-compatibility' because MSVC allows using the 'enum' specifier for typedef'd anonymous enums:

  typedef enum
  {
      First,
      Second
  } MyEnum;
  
  void func()
  {
      enum MyEnum foo;
  }

Today the code above produces the following compile error:

  <source>:9:7: error: typedef 'MyEnum' cannot be referenced with a enum specifier
          enum MyEnum foo;
               ^
  <source>:5:3: note: declared here
  } MyEnum;
    ^

The reason why this change is desired in Clang is because the MSVC tools 'mktyplib' and 'midl' produce C++ code which uses the 'enum' specifier in such a way. Here is a small repro:

MyEnum.idl:

  [
        uuid(da2889bf-6173-4c1c-a23d-52ad85fc91ca)
  ]
  library MyEnumLib
  {
        typedef enum { X } MyEnum;
  };

Application.idl:

  [
        uuid(4d0cc96f-3258-46f7-98bf-6654c1d027c5)
  ]
  library ApplicationLib
  {
        importlib("MyEnum.tlb");
  
        [
              uuid(6ac4c3d7-15c7-40c7-8b64-60459e29680b)
        ]
        interface ApplicationInterface : IDispatch
        {
              [
                    id(66),
                    propget
              ]
              HRESULT Foo([out, retval] enum MyEnum* Foo);
  
              [
                    id(66),
                    propput
              ]
              HRESULT Foo([in] enum MyEnum Foo);
        };
  };

Use the following commands to build:

  mktyplib /win32 /cpp_cmd cl.exe /cpp_opt /E /tlb MyEnum.tlb /h MyEnum.h MyEnum.idl
  midl -h Application.h -iid Application.c -tlb Application.tlb /cpp_cmd cl.exe Application.idl

This produces Application.h with the following snippet (note the usage of the 'enum' specifier):

  MIDL_INTERFACE("6ac4c3d7-15c7-40c7-8b64-60459e29680b")
  ApplicationInterface : public IDispatch
  {
  public:
      virtual /* [propget][id] */ HRESULT STDMETHODCALLTYPE get_Foo( 
          /* [retval][out] */ enum /* external definition not present */ MyEnum *Foo) = 0;
      
      virtual /* [propput][id] */ HRESULT STDMETHODCALLTYPE put_Foo( 
          /* [in] */ enum /* external definition not present */ MyEnum Foo) = 0;
      
  };

Note that the 'mktyplib' tool is deprecated now but it's used for building some code here at Microsoft, and probably other legacy build scenarios in the industry as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91659

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/enum-typedef-msvc.c
  clang/test/Sema/enum-typedef-msvc.cpp


Index: clang/test/Sema/enum-typedef-msvc.cpp
===================================================================
--- /dev/null
+++ clang/test/Sema/enum-typedef-msvc.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -fms-compatibility %s
+
+typedef enum {
+  First,
+  Second
+} MyEnum;
+
+class AMyInterface {
+  virtual void MyFunc(enum MyEnum *param) = 0;
+};
+
+class MyImpl : public AMyInterface {
+  virtual void MyFunc(enum MyEnum *param) override {}
+};
+
+// expected-no-diagnostics
Index: clang/test/Sema/enum-typedef-msvc.c
===================================================================
--- /dev/null
+++ clang/test/Sema/enum-typedef-msvc.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -fms-compatibility -DUSE_MSVC_COMPAT %s
+
+typedef enum {
+  A
+} Foo;
+
+void func() {
+#ifdef USE_MSVC_COMPAT
+  enum Foo foo; // expected-no-diagnostics
+#else
+  enum Foo foo; // expected-error {{variable has incomplete type 'enum Foo'}} // expected-note {{forward declaration of 'enum Foo'}}
+#endif
+  (void)foo;
+}
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -15539,6 +15539,34 @@
     // shouldn't be diagnosing.
     LookupName(Previous, S);
 
+    // Under MSVC, the 'enum' specifier can be used for typedef'd enums.
+    // Note that lookup only fails in C, not C++, so this if condition
+    // is only used for C code.
+    if (getLangOpts().MSVCCompat && (Kind == TTK_Enum) && Previous.empty() &&
+        (TUK == TUK_Reference)) {
+      LookupResult TypedefEnumLookup(*this, Name, NameLoc, LookupOrdinaryName,
+                                     Redecl);
+      LookupName(TypedefEnumLookup, S);
+
+      if (!TypedefEnumLookup.empty()) {
+        if (TypedefNameDecl *TD =
+                dyn_cast<TypedefNameDecl>(TypedefEnumLookup.getFoundDecl())) {
+          const Type *UnderlyingTypePtr =
+              TD->getUnderlyingType().getTypePtrOrNull();
+
+          if (UnderlyingTypePtr) {
+            if (EnumDecl *UnderlyingEnumDecl =
+                    dyn_cast<EnumDecl>(UnderlyingTypePtr->getAsTagDecl())) {
+              // We only allow this for anonymous enums
+              if (UnderlyingEnumDecl->getDeclName().isEmpty()) {
+                Previous.addDecl(UnderlyingEnumDecl);
+              }
+            }
+          }
+        }
+      }
+    }
+
     // When declaring or defining a tag, ignore ambiguities introduced
     // by types using'ed into this scope.
     if (Previous.isAmbiguous() &&
@@ -15721,9 +15749,12 @@
       if (TypedefNameDecl *TD = dyn_cast<TypedefNameDecl>(PrevDecl)) {
         if (const TagType *TT = TD->getUnderlyingType()->getAs<TagType>()) {
           TagDecl *Tag = TT->getDecl();
-          if (Tag->getDeclName() == Name &&
-              Tag->getDeclContext()->getRedeclContext()
-                          ->Equals(TD->getDeclContext()->getRedeclContext())) {
+          bool AnonymousEnumEligible = getLangOpts().MSVCCompat &&
+                                       (Kind == TTK_Enum) &&
+                                       Tag->getDeclName().isEmpty();
+          if ((Tag->getDeclName() == Name || AnonymousEnumEligible) &&
+              Tag->getDeclContext()->getRedeclContext()->Equals(
+                  TD->getDeclContext()->getRedeclContext())) {
             PrevDecl = Tag;
             Previous.clear();
             Previous.addDecl(Tag);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D91659.305903.patch
Type: text/x-patch
Size: 3536 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201117/bc5497be/attachment-0001.bin>