[PATCH] MS ABI: Improve selection of an inheritance model

[Reid Kleckner]

================
Comment at: lib/AST/MicrosoftCXXABI.cpp:136
@@ +135,3 @@
+
+  return calculateInheritanceModel(this);
+}
----------------
This should probably be llvm_unreachable("getting inheritance model from type that wasn't required to be complete");

Otherwise, if we have a bug where this actually executes, the inheritance model might change once the type is completed.

================
Comment at: lib/AST/MicrosoftCXXABI.cpp:119
@@ +118,3 @@
+  }
+}
+
----------------
MSVC will warn about falling off the end of the function despite the fully covered switch, so you want an unreachable after the switch.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1355
@@ -1354,1 +1354,3 @@
+    return true;
+  return RD->isPolymorphic();
 }
----------------
This can be:
  return RD->isDynamicClass():

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1828
@@ +1827,3 @@
+      (IsFunc ||
+       nullFieldOffsetIsZero(SrcTy->getClass()->getAsCXXRecordDecl()) ==
+           nullFieldOffsetIsZero(DstTy->getClass()->getAsCXXRecordDecl())))
----------------
I'd return early if (IsReinterpret && IsFunc), and then pull out SrcRD and DstRD outside of the comparison for readability.

================
Comment at: lib/Sema/SemaType.cpp:5077
@@ +5076,3 @@
+              ->getAsCXXRecordDecl()
+              ->getMostRecentDecl()
+              ->setMSInheritanceModel();
----------------
I'm curious, why is it necessary to get the most recent decl?

================
Comment at: lib/Sema/SemaType.cpp:5072
@@ +5071,3 @@
+      if (const MemberPointerType *MPTy = T->getAs<MemberPointerType>()) {
+        if (!MPTy->getClass()->isDependentType()) {
+          RequireCompleteType(Loc, QualType(MPTy->getClass(), 0), 0);
----------------
If our class type is dependent and a complete type was required, isn't the member pointer type still incomplete?  If so, should we return true after emitting a diagnostic?

================
Comment at: lib/Sema/SemaType.cpp:5049
@@ -5077,3 +5048,3 @@
   NamedDecl *Def = 0;
   if (!T->isIncompleteType(&Def)) {
     // If we know about the definition but it is not visible, complain.
----------------
I wonder if MPT->isIncompleteType() should return true if the class type is incomplete.

================
Comment at: test/SemaCXX/microsoft-abi-ptm.cpp:10
@@ +9,3 @@
+void f(int B::*mp);
+static_assert(sizeof(int B::*) == sizeof(int) * 3, "pointer-to-member should be sizeof(int)*4");
+struct B { };
----------------
Hah.  This can be 12, but sizeof(int)*4 is wrong.


http://llvm-reviews.chandlerc.com/D2548