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@+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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
