[Sema] Correct IUnknown to support Unknwnbase.h Header.

[erichkeane]

Summary

Apparently, the MSVC SDK has a strange implementation that
causes a number of implicit functions as well as a template member
function of the IUnknown type. This patch allows these as InterfaceLike
types as well.

Additionally, it corrects the behavior where extern-C++ wrapped around an
Interface-Like type would permit an interface-like type to exist in a namespace.

[rnk]

lib/AST/DeclCXX.cpp

1506–1514

This would be nicer factored out as a static helper because it would let you early return and avoid the variable and break. You could just fold the call into the condition in the if below.

[erichkeane]

As @rnk suggested, pull my condition out into a static function.

[rnk]

lgtm

lib/AST/DeclCXX.cpp

1473

Lower case isDeclContext... is more inline with our style

[chapuni]

Appeased in rL314586. I don't think my tweak would be right, though.

See;
http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/6726
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/5135

cfe/trunk/test/SemaCXX/ms-iunknown-template-function.cpp

18 ↗

(On Diff #117218)

This is not seen for targeting *-win32.
I wonder why you checked it here.

[erichkeane]

@chapuni Thanks for the fix! That 'novtable' line is there simply because that is supposed to be a minimized replica of what happens in the UnknwnBase.h header.

I'll likely just remove the attribute and your fix, since the warning itself isn't important to the test itself otherwise.

Thanks!

lib/AST/DeclCXX.cpp

if (const CXXRecordDecl *RD =
             Field->getType()->getBaseElementTypeUnsafe()->getAsCXXRecordDecl())
       if (RD->isAnyDestructorNoReturn())
         return true;
 
   // All destructors are not noreturn.
   return false;
 }
 
+static bool IsDeclContextInNamespace(const DeclContext *DC) {
+  while (!DC->isTranslationUnit()) {
+    if (DC->isNamespace())
+      return true;
+    DC = DC->getParent();
+  }
+  return false;
+}
+
 bool CXXRecordDecl::isInterfaceLike() const {
   assert(hasDefinition() && "checking for interface-like without a definition");
   // All __interfaces are inheritently interface-like.
   if (isInterface())
     return true;
 
   // Interface-like types cannot have a user declared constructor, destructor,
   // friends, VBases, conversion functions, or fields.  Additionally, lambdas
   // cannot be interface types.
   if (isLambda() || hasUserDeclaredConstructor() ||
       hasUserDeclaredDestructor() || !field_empty() || hasFriends() ||
       getNumVBases() > 0 || conversion_end() - conversion_begin() > 0)
     return false;
 
   // No interface-like type can have a method with a definition.
   for (const auto *const Method : methods())
-    if (Method->isDefined())
+    if (Method->isDefined() && !Method->isImplicit())
       return false;
 
   // Check "Special" types.
   const auto *Uuid = getAttr<UuidAttr>();
-  if (Uuid && isStruct() && (getDeclContext()->isTranslationUnit() ||
-                             getDeclContext()->isExternCXXContext()) &&
+  // MS SDK declares IUnknown/IDispatch both in the root of a TU, or in an
+  // extern C++ block directly in the TU.  These are only valid if in one
+  // of these two situations.
+  if (Uuid && isStruct() && !getDeclContext()->isExternCContext() &&
+      !IsDeclContextInNamespace(getDeclContext()) &&
       ((getName() == "IUnknown" &&
         Uuid->getGuid() == "00000000-0000-0000-C000-000000000046") ||
        (getName() == "IDispatch" &&
         Uuid->getGuid() == "00020400-0000-0000-C000-000000000046"))) {
     if (getNumBases() > 0)
       return false;
     return true;
   }
 
   // FIXME: Any access specifiers is supposed to make this no longer interface
   // like.
 
   // If this isn't a 'special' type, it must have a single interface-like base.
   if (getNumBases() != 1)
     return false;

test/SemaCXX/ms-iunknown-template-function.cpp

+// RUN: %clang_cc1 -fsyntax-only -verify -fms-extensions %s 
+typedef long HRESULT;
+typedef unsigned long ULONG;
+typedef struct _GUID {
+  unsigned long Data1;
+  unsigned short Data2;
+  unsigned short Data3;
+  unsigned char Data4[8];
+} GUID;
+typedef GUID IID;
+
+// remove stdcall, since the warnings have nothing to do with
+// what is being tested.
+#define __stdcall
+
+extern "C" {
+extern "C++" {
+// expected-warning@+1 {{__declspec attribute 'novtable'}}
+struct __declspec(uuid("00000000-0000-0000-C000-000000000046")) __declspec(novtable)
+    IUnknown {
+public:
+  virtual HRESULT __stdcall QueryInterface(
+      const IID &riid,
+      void **ppvObject) = 0;
+
+  virtual ULONG __stdcall AddRef(void) = 0;
+
+  virtual ULONG __stdcall Release(void) = 0;
+
+  template <class Q>
+  HRESULT __stdcall QueryInterface(Q **pp) {
+    return QueryInterface(__uuidof(Q), (void **)pp);
+  }
+};
+}
+}
+
+__interface ISfFileIOPropertyPage : public IUnknown{};
+

test/SemaCXX/ms-iunknown.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify -fms-extensions %s 
 
 extern "C++" struct __declspec(uuid("00000000-0000-0000-C000-000000000046")) IUnknown {
   void foo();
+  // Definitions aren't allowed, unless they are a template.
+  template<typename T>
+  void bar(T t){}
 };
+
 struct IPropertyPageBase : public IUnknown {};
 struct IPropertyPage : public IPropertyPageBase {};
 __interface ISfFileIOPropertyPage : public IPropertyPage {};
 
 
 namespace NS {
   struct __declspec(uuid("00000000-0000-0000-C000-000000000046")) IUnknown {};
   // expected-error@+1 {{interface type cannot inherit from}}
-  __interface IPropertyPageBase : public IUnknown {}; 
+  __interface IPropertyPageBase : public IUnknown {};
 }
+
+namespace NS2 {
+extern "C++" struct __declspec(uuid("00000000-0000-0000-C000-000000000046")) IUnknown {};
+// expected-error@+1 {{interface type cannot inherit from}}
+__interface IPropertyPageBase : public IUnknown{};
+}
+
 // expected-error@+1 {{interface type cannot inherit from}}
-__interface IPropertyPageBase2 : public NS::IUnknown {}; 
+__interface IPropertyPageBase2 : public NS::IUnknown {};
 
 __interface temp_iface {};
 struct bad_base : temp_iface {};
 // expected-error@+1 {{interface type cannot inherit from}}
 __interface bad_inherit : public bad_base{};
 
 struct mult_inher_base : temp_iface, IUnknown {};
 // expected-error@+1 {{interface type cannot inherit from}}
 __interface bad_inherit2 : public mult_inher_base{};
 
 struct PageBase : public IUnknown {};
 struct Page3 : public PageBase {};
 struct Page4 : public PageBase {};
 __interface PropertyPage : public Page4 {};
 
 struct Page5 : public Page3, Page4{};
 // expected-error@+1 {{interface type cannot inherit from}}
-__interface PropertyPage2 : public Page5 {}; 
+__interface PropertyPage2 : public Page5 {};
 
 __interface IF1 {};
-__interface PP : IUnknown, IF1{}; 
+__interface PP : IUnknown, IF1{};
 __interface PP2 : PP, Page3, Page4{};