[PATCH] [ms-cxxabi] Get closer to emitting the correct set of destructors in each TU (PR12784).

[Timur Iskhodzhanov]

  General comment: is there code we actually try to compile with Clang that really needs vector deleting dtors support?
  Are these dtors needed anywhere other than polymorphic array delete?

  IMO what we need in the MS ABI dtors handling at this stage is general correctness of when common types of destructors get called,
  that is roughly:

    # ?1X@@... to destroy an object of type X without inheritance/vftables
    # ?1Base@@... to destroy a non-virtual base class Base from ?1Child@@...
    # //Any// of the deleting destructors for a polymorphic non-array delete
    # The scalar deleting dtor of "struct Child : virtual Base" calls the virtual base dtor (?_DChild@@...).

  Currently we're pretty much at 1+2+3 if we just skip emitting the base dtors (similar to how we skip emitting base ctors).

  That said, I like some of the refactoring you've done in this patch, but I'm worried that the progress on vector deleting dtors was done at the cost of degrading the existing functionality.

  What do you think about working on the virtual base dtors first, writing more tests (so we understand the dtors ABI better) and then taking a new look at the vector deleting dtors?


================
Comment at: lib/CodeGen/CodeGenModule.cpp:1223
@@ -1208,2 +1222,3 @@
     return true;
   if (CodeGenOpts.OptimizationLevel == 0 &&
+      !GD.getDecl()->hasAttr<AlwaysInlineAttr>() &&
----------------
how about

  const FunctionDecl *F = GD.getDecl();

before the second if?

================
Comment at: test/CodeGenCXX/microsoft-abi-static-initializers.cpp:14
@@ -13,3 +13,3 @@
 // CHECK: define internal void @"__dtor_\01?s@@3US@@A"() [[NUW]] {
-// CHECK: call x86_thiscallcc void @"\01??1S@@QAE at XZ"
+// CHECK: call x86_thiscallcc void @"\01??_DS@@QAE at XZ"
 // CHECK: ret void
----------------
Should be "?1S@@...", otherwise you might get errors when linking TU compiled with different compilers, right?

================
Comment at: test/CodeGenCXX/microsoft-abi-structors.cpp:69
@@ +68,3 @@
+// DTOR3:        %[[FLAGS_VALUE:[0-9a-z._]+]] = load i32* %[[FLAGS_VAR]]
+// DTOR3:        call x86_thiscallcc void @"\01??_DC at basic@@UAE at XZ"(%"struct.basic::C"* %[[THIS:[0-9a-z]+]])
+// DTOR3-NEXT:   %[[CONDITION:[0-9]+]] = icmp eq i32 %[[FLAGS_VALUE]], 0
----------------
Should be "?1C at ..." here.

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:277
@@ -273,3 +276,3 @@
   } else if (IsDeletingDtor(CGF.CurGD)) {
-    ImplicitParamDecl *ShouldDelete
+    ImplicitParamDecl *Flags
       = ImplicitParamDecl::Create(Context, 0,
----------------
Can you please document that the least significant bit is "whether need to call operator delete at the end" and the second least significant bit is "whether a polymorphic array delete" ? 


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