[PATCH] D37308: Fix the __interface inheritence rules to work better with IUnknown and IDispatch

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked 4 inline comments as done.
erichkeane added a comment.

Thanks for the quick  response!  New patch coming soon.



================
Comment at: lib/AST/DeclCXX.cpp:1478
+
+  // No interface-like types can have a user declared constructor, destructor,
+  // friends, VBases, conersion functions, or fields.  Additionally, lambdas
----------------
rnk wrote:
> Maybe "Interface-like types cannot have ..."
> 
> Can an interface-like type have an explicitly defaulted copy ctor? That will be user declared, but it will probably be trivial.
It cannot according to godbolt. https://godbolt.org/g/wZuNzT


================
Comment at: lib/AST/DeclCXX.cpp:1486-1489
+  // No interface-like type can have a method with a definition.
+  for (const auto *const Method : methods())
+    if (Method->isDefined())
+      return false;
----------------
rnk wrote:
> Huh, so they can be declared, but they don't have to be pure.
Correct, and strange.  Annoyingly, it is possible to do:

  struct S : someIFace {
  void foo();
  };
    __interface F  : public S {}; // This one is OK.
    void S::foo();
    __interface F2  : public S {}; // This one is NOT OK.


================
Comment at: lib/AST/DeclCXX.cpp:1516-1517
+    const auto *Base = BS.getType()->getAsCXXRecordDecl();
+    if (Base->isInterface())
+      continue;
+    if (Base->isInterfaceLike()) {
----------------
rnk wrote:
> Are you sure you want this? MSVC rejects the test case that exercises this case.
AM I sure?   Not anymore.  WAS I sure?  Pretty darn.  Thanks for the catch!


================
Comment at: test/SemaCXX/ms-iunknown-outofline-def.cpp:9-10
+void IUnknown::foo() {}
+// expected-error at +1{{interface type cannot inherit from}}
+__interface HasError : public IUnknown {}; 
----------------
rnk wrote:
> Again, this is a pretty weird error. If the method isn't pure, there must be a definition *somewhere* in the program, unless interface types are always declared with novtable.
Yeah, I agree.  I have no idea if I can limit this to just pure functions, since I'm not totally sure about the use cases.  We have at least 1 test in our testbase that isn't virtual, but the context has been lost (might have been minimized from another repro).


https://reviews.llvm.org/D37308