[PATCH] D37308: Interface class with uuid base record

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: lib/Sema/SemaDeclCXX.cpp:2376
 
+/// \brief Tests if the __interface base is public.
+static bool IsDeclPublicInterface(const CXXRecordDecl *RD,
----------------
The comment does not match the behavior of the function.


================
Comment at: lib/Sema/SemaDeclCXX.cpp:2382
+
+/// \brief Test if record is an uuid Unknown.
+/// This is an MS SDK specific type that has a special
----------------
uuid Unknown -> UUID for IUnknown


================
Comment at: lib/Sema/SemaDeclCXX.cpp:2386
+static bool IsIUnknownType(const CXXRecordDecl *RD) {
+  auto *Uuid = RD->getAttr<UuidAttr>();
+
----------------
This can be `const auto *`


================
Comment at: lib/Sema/SemaDeclCXX.cpp:2389
+  return RD->isStruct() && RD->getName() == "IUnknown" && RD->isEmpty() &&
+         Uuid && Uuid->getGuid() =="00000000-0000-0000-C000-000000000046" &&
+         dyn_cast<TranslationUnitDecl>(RD->getDeclContext());
----------------
I would move the test for `Uuid` to be one of the first things tested, since it's the least expensive.


================
Comment at: lib/Sema/SemaDeclCXX.cpp:2390
+         Uuid && Uuid->getGuid() =="00000000-0000-0000-C000-000000000046" &&
+         dyn_cast<TranslationUnitDecl>(RD->getDeclContext());
+}
----------------
erichkeane wrote:
> @aaron.ballman This logic we'd like you to particularly check on.  Does this ensure it isn't in a namespace?
> 
> Zahira: since you don't need the value, "isa<TranslationUnitDecl>" is more appropriate here.
I would skip the casts entirely and use `RD->getDeclContext()->isTranslationUnit()`


https://reviews.llvm.org/D37308