[PATCH] [ms-cxxabi] Fix the base/complete dtor logic.

[Timur Iskhodzhanov]

  > Previously it was thought that the Microsoft ABI used '?1' for
  > complete dtors. In fact, '?1' is for base dtors, and '?_D' is for
  > complete dtors. (Some confusion arose from documentation referring
  > to complete dtors as 'vbase dtors'.)

  Well if you look at the debug information CL generates for this code:

    struct A { virtual ~A(); };
    struct B : virtual A { virtual ~B(); };

  you'll notice that `??1B@@UAE at XZ` is `B::~B`
  and `??_DB@@QAEXXZ` is `B::'vbase destructor'`.

  Yes, one needs to use a `?_D` in order to destroy an object of a class with virtual bases - so "vbase dtor" means "the one that destroys the virtual bases as its last action" and yes it's similar to Itanium's complete destructors in this sense.

  However, destroying an object of a class without virtual bases in Microsoft ABI doesn't need to use a `?_D` destructor and it uses a `?1` dtor [and this saves us some size on each object file],
  in contrast with the Itanium ABI which always uses complete dtors to destroy objects, but optimizes the object file sizes by using dtor type aliases.

  Do you agree with this data points and reasoning?

  I'd still prefer that we use `?1`.

  Can you for example let the CGCXXABI tell you which CXXDtorType to use to destroy an object of a given type?
  Or at least add a few FIXME that this in fact should be `?1` but works for us as long as we define extra auxillary dtors.


================
Comment at: lib/AST/MicrosoftMangle.cpp:509
@@ -508,3 +508,3 @@
         // class with a destructor is declared within a destructor.
-        mangleCXXDtorType(Dtor_Complete);
+        mangleCXXDtorType(Dtor_Base);
       break;
----------------
Please add a test that checks mangling of a static in a destructor of a class with virtual bases

================
Comment at: lib/CodeGen/CGCXX.cpp:244
@@ -241,3 +243,3 @@
 
   // The destructor used for destructing this as a base class; ignores
   // virtual bases.
----------------
This comment is no longer true?

You should also describe how Base/Complete/Deleting dtors work as a comment somewhere in MicrosoftCXXABI
or generalize the comments at the CXXDtorType definition
(e.g. Base = dtor to destroy this as a base or as a complete object if there are no virtual bases, etc)

================
Comment at: test/CodeGenCXX/microsoft-abi-structors.cpp:55
@@ +54,3 @@
+// We shouldn't emit the other destructor forms as they aren't needed by the TU.
+// CHECK-NOT: @"\01??_DB at basic@@UAE at XZ"
+// CHECK-NOT: @"\01??_GB at basic@@UAEPAXI at Z"
----------------
I think these should be checked by an extra FileChecker invocation to be 100% they are not emitted in this TU.

================
Comment at: test/CodeGenCXX/microsoft-abi-structors.cpp:247
@@ +246,3 @@
+// The emission of the ctor requires emission of the vtable and therefore
+// of the other dtor forms, despite not being defined in this TU.
+
----------------
how about
"of the auxillary dtor types, despite the base destructor is not being defined in this TU."

================
Comment at: test/CodeGenCXX/microsoft-abi-structors.cpp:250
@@ +249,3 @@
+// DTOR5: define linkonce_odr x86_thiscallcc void @"\01??_GA at bodyless_dtors@@UAEPAXI at Z"(%"struct.bodyless_dtors::A"* %this, i32 %flags)
+// DTORS: define linkonce_odr x86_thiscallcc void @"\01??_DA at bodyless_dtors@@UAE at XZ"(%"struct.bodyless_dtors::A"* %this)
+
----------------
In fact, defining a `vbase destructor` is not required in this TU if we get back to using `?1` dtors for classes without virtual bases.


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