erichkeane requested changes to this revision.
erichkeane added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Sema/SemaDeclCXX.cpp:2377
+/// \brief Tests if the __interface base is public.
+static bool IsRecordPublicInterface(const CXXRecordDecl *RD,
+                                  AccessSpecifier spec) {
----------------
Not sure that 'Record' is the correct word here.  You should likely just do 
"IsDeclPublicInterface".  Additionally (though not necessary to change), the 
only requirement for this type is the isInterface, which comes from TagDecl.


================
Comment at: lib/Sema/SemaDeclCXX.cpp:2385
+/// behavior in the CL compiler.
+static bool IsUnknownType(const CXXRecordDecl *RD) {
+  auto *Uuid = RD->getAttr<UuidAttr>();
----------------
The type name is "IUnknown".  This function name (and the comment above) needs 
to match this.


================
Comment at: lib/Sema/SemaDeclCXX.cpp:2393
+/// \brief Test if record and records in inheritance tree is a an Unknown.
+static bool IsOrInheritsFromUnknown(const CXXRecordDecl *RD) {
+  if (RD->getNumBases()) {
----------------
This logic is completely wrong.  You are trying to test if RD is IUnknown OR if 
ANY of its bases are IUnknown (or if they inherit).  You absolutely have to 
loop over the bases here, which you aren't doing here at all.




================
Comment at: test/SemaCXX/ms-uuid.cpp:97
+
+struct __declspec(uuid("00000000-0000-0000-C000-000000000046")) IUnknown {};
+__interface ISfFileIOPropertyPage : public IUnknown {};
----------------
Add some should-compile examples that show multiple inheritance.  Particularly 
where the IUnknown is inherited from the 2nd or later one.


https://reviews.llvm.org/D37308



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to