[PATCH] [-cxx-abi microsoft] Fix "this" argument/parameter offsets for virtual destructors in the presence of virtual bases

[Reid Kleckner]

  LGTM


================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:585
@@ -574,5 +584,3 @@
   if (ML.VBase) {
-    This = CGF.Builder.CreateBitCast(This, charPtrTy);
-    llvm::Value *VBaseOffset = CGM.getCXXABI()
-        .GetVirtualBaseClassOffset(CGF, This, MD->getParent(), ML.VBase);
-    This = CGF.Builder.CreateInBoundsGEP(This, VBaseOffset);
+    bool AvoidVirtualOffset = false;
+    if (isa<CXXDestructorDecl>(MD) && GD.getDtorType() == Dtor_Base) {
----------------
I'm guessing this optimization just makes it easier to test and read and test the LLVM IR output.

================
Comment at: lib/AST/VTableBuilder.cpp:2722-2723
@@ -2720,1 +2721,4 @@
 
+    // If there's at least one virtual destructor in a non-virtual base,
+    // the "this" parameter points at the most derived class.
+    if (!SeenVBase && isa<CXXDestructorDecl>(MD))
----------------
Wouldn't you return zero if it points at the most derived class, or was the old code wrong?

================
Comment at: lib/AST/VTableBuilder.cpp:2714
@@ +2713,3 @@
+
+        // The traversal ends at the vbase for virtual destructors.
+        if (isa<CXXDestructorDecl>(MD))
----------------
So, before calling a dtor, we adjust to the first virtual base with a virtual dtor.  Is that right?

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:575
@@ +574,3 @@
+    // There's only Dtor_Deleting in vftable but it shares the this adjustment
+    // with the deleting one, so look it up instead.
+    LookupGD = GlobalDecl(DD, Dtor_Deleting);
----------------
I don't understand this comment.  Do you mean that the base dtor should use the same this adjustment as the deleting one?


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