[PATCH] D37308: Interface class with uuid base record

Erich Keane via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 13 13:59:10 PDT 2017


erichkeane added a comment.

Actually... disregard that... the rule is more complex than that.

Based on some playing around with MSVC on godbolt, it seems that it actually marks any type that inherits ONLY from interface types or IUnknown as an interface itself.  We may be better off doing that instead.

Additionally, the implementation of "isIUnknown" is actually more complicated too, since it contains function declarations.



================
Comment at: lib/Sema/SemaDeclCXX.cpp:2388
+
+  return Uuid && Uuid->getGuid() =="00000000-0000-0000-C000-000000000046" &&
+         RD->isStruct() && RD->getName() == "IUnknown" && RD->isEmpty() &&
----------------
erichkeane wrote:
> Up to @aaron.ballman, but moving the getGuid string check (a massive string-diff) to 2nd defeats the purpose of moving UUID up front, which is to put the 'easy' things first.  IsTU, isStruct, and isEmpty are likely better.
> 
> Additionally, I'd mentioned it before, this still doesn't check to make sure that the IUnknown doesn't inherit from anywhere, does it?  MSVC also will error if IUnknown's definition includes an inherited type.
Also, "isEmpty" isn't sufficient.  IUnknown actually contains functions as long as none have an implementation. (Declarations only).


https://reviews.llvm.org/D37308





More information about the cfe-commits mailing list