[PATCH] [ms-cxxabi] Implement member pointer conversions

[Richard Smith]

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:928
@@ +927,3 @@
+  if (E->getCastKind() == CK_ReinterpretMemberPointer)
+    return Builder.CreateSelect(IsNotNull, Src, DstNull);
+
----------------
Will we have rejected this in Sema if Src and Dst would have different types, or are we missing a bitcast on Src here? (What if the member pointers would have the same size but different types because one is missing a pointer member and the other is missing an int member?)

Also, it'd be nice to not emit the select if the null representation didn't change, which is presumably the common case.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:982-983
@@ +981,4 @@
+      Dst = Builder.CreateInsertValue(Dst, NonVirtualBaseAdjustment, Idx++);
+    // FIXME: Bug here if src had no VBPtrOffset but dst does, Idx isn't
+    // updated.
+    if (VBPtrOffset && hasVBPtrOffsetField(DstInheritance))
----------------
Right. Maybe unconditionally create an 'insertvalue' for every field in the destination, and use getConstantOrZeroInt? Also, start with an 'undef' rather than zero-initialized aggregate if you do this.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1016
@@ +1015,3 @@
+  if (E->getCastKind() == CK_ReinterpretMemberPointer)
+    return Src;
+
----------------
Do we ever need a bitcast here (if the representations are the same size but different types)?

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1045
@@ +1044,3 @@
+      IsFunc ? NonVirtualBaseAdjustment : FirstField;
+    bool isDerivedToBase = (E->getCastKind() == CK_DerivedToBaseMemberPointer);
+    if (!NVAdjustField)  // If this field didn't exist in src, it's zero.
----------------
*IsDerivedToBase

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:611-625
@@ +610,17 @@
+  // FIXME PR15713: Support virtual inheritance paths.
+  CharUnits ThisAdjustment = CharUnits::Zero();
+  ArrayRef<const CXXRecordDecl*> Path = MP.getMemberPointerPath();
+  bool DerivedMember = MP.isMemberPointerToDerivedMember();
+  const CXXRecordDecl *RD = cast<CXXRecordDecl>(MPD->getDeclContext());
+  for (unsigned I = 0, N = Path.size(); I != N; ++I) {
+    const CXXRecordDecl *Base = RD;
+    const CXXRecordDecl *Derived = Path[I];
+    if (DerivedMember)
+      std::swap(Base, Derived);
+    ThisAdjustment +=
+      getContext().getASTRecordLayout(Derived).getBaseClassOffset(Base);
+    RD = Path[I];
+  }
+  if (DerivedMember)
+    ThisAdjustment = -ThisAdjustment;
+
----------------
Please factor out the duplication between this and ItaniumCXXABI (maybe add an APValue overload for CGCXXABI::getMemberPointerAdjustment). Also, it'd be great to reuse CGM.GetNonVirtualBaseClassOffset for the actual computation here (but it looks like it expects the path in a slightly different format, so it may be more effort than it's worth to get that to happen).


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