Fix issue 15589 - extern(C++) virtual destructors are not put in vtbl[]

src/dmd/aggregate.h

@@ -285,6 +285,7 @@ class ClassDeclaration : public AggregateDeclaration
     TypeInfoClassDeclaration *vclassinfo;       // the ClassInfo object for this ClassDeclaration
     bool com;                           // true if this is a COM class (meaning it derives from IUnknown)
     bool stack;                         // true if this is a scope class
+    int cppDtorVtblIndex;               // slot reserved for the virtual destructor [extern(C++)]
     bool inuse;                         // to prevent recursive attempts
     bool isActuallyAnonymous;           // true if this class has an identifier, but was originally declared anonymous
                                         // used in support of https://issues.dlang.org/show_bug.cgi?id=17371

src/dmd/clone.d

@@ -15,6 +15,7 @@ module dmd.clone;
 import core.stdc.stdio;
 import dmd.aggregate;
 import dmd.arraytypes;
+import dmd.dclass;
 import dmd.declaration;
 import dmd.dscope;
 import dmd.dstruct;
@@ -883,6 +884,34 @@ extern (C++) FuncDeclaration buildDtor(AggregateDeclaration ad, Scope* sc)
             e = Expression.combine(ex, e); // combine in reverse order
         }
 
+        /* extern(C++) destructors call into super to destruct the full hierarchy
+        */
+        ClassDeclaration cldec = ad.isClassDeclaration();
+        if (cldec && cldec.classKind == ClassKind.cpp && cldec.baseClass && cldec.baseClass.dtor)
+        {
+            // WAIT BUT: do I need to run `cldec.baseClass.dtor` semantic? would it have been run before?
+            cldec.baseClass.dtor.functionSemantic();
+
+            stc = mergeFuncAttrs(stc, cldec.baseClass.dtor);
+            if (!(stc & STC.disable))
+            {
+                // super.__xdtor()
+
+                Expression ex = new SuperExp(loc);
+
+                // This is a hack so we can call destructors on const/immutable objects.
+                // Do it as a type 'paint'.
+                ex = new CastExp(loc, ex, cldec.baseClass.type.mutableOf());
+                if (stc & STC.safe)
+                    stc = (stc & ~STC.safe) | STC.trusted;
+
+                ex = new DotVarExp(loc, ex, cldec.baseClass.dtor, false);
+                ex = new CallExp(loc, ex);
+
+                e = Expression.combine(e, ex); // super dtor last
+            }
+        }
+
         /* Build our own "destructor" which executes e
          */
         if (e || (stc & STC.disable))

src/dmd/cppmanglewin.d

@@ -603,7 +603,7 @@ private:
             // <flags> ::= Y <calling convention flag>
             buf.writeByte('Y');
         }
-        const(char)* args = mangleFunctionType(cast(TypeFunction)d.type, d.needThis(), d.isCtorDeclaration() || d.isDtorDeclaration());
+        const(char)* args = mangleFunctionType(cast(TypeFunction)d.type, d.needThis(), d.isCtorDeclaration() || d.isDtorDeclaration(), d.isDtorDeclaration() !is null);
         buf.writestring(args);
     }
 
@@ -1144,7 +1144,7 @@ private:
         cur.accept(this);
     }
 
-    const(char)* mangleFunctionType(TypeFunction type, bool needthis = false, bool noreturn = false)
+    const(char)* mangleFunctionType(TypeFunction type, bool needthis = false, bool noreturn = false, bool noargs = false)
     {
         scope VisualCPPMangler tmp = new VisualCPPMangler(this);
         // Calling convention
@@ -1204,7 +1204,7 @@ private:
             rettype.accept(tmp);
             tmp.flags &= ~MANGLE_RETURN_TYPE;
         }
-        if (!type.parameters || !type.parameters.dim)
+        if (!type.parameters || !type.parameters.dim || noargs)
         {
             if (type.varargs == 1)
                 tmp.buf.writeByte('Z');

src/dmd/dclass.d

@@ -214,6 +214,9 @@ extern (C++) class ClassDeclaration : AggregateDeclaration
     /// true if this is a scope class
     bool stack;
 
+    /// if this is a C++ class, this is the slot reserved for the virtual destructor
+    int cppDtorVtblIndex = -1;
+
     /// to prevent recursive attempts
     private bool inuse;
 

src/dmd/dsymbolsem.d

@@ -3735,7 +3735,17 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
         if (dd.ident == Id.dtor && dd.semanticRun < PASS.semantic)
             ad.dtors.push(dd);
         if (!dd.type)
-            dd.type = new TypeFunction(null, Type.tvoid, false, LINK.d, dd.storage_class);
+        {
+            Parameters* params = null;
+            if (ad.classKind == ClassKind.cpp && !Target.cppDeletingDestructor)
+            {
+                // Windows doesn't use a deleting destructor, it takes an argument instead!
+                Parameter param = new Parameter(STC.undefined_, Type.tuns32, new Identifier("del"), new IntegerExp(Loc.initial, 0, Type.tuns32));
+                params = new Parameters;
+                (*params).push(param);
+            }
+            dd.type = new TypeFunction(params, Type.tvoid, false, LINK.d, dd.storage_class);
+        }
 
         sc = sc.push();
         sc.stc &= ~STC.static_; // not a static destructor
@@ -4767,6 +4777,27 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
         {
             auto s = (*cldec.members)[i];
             s.dsymbolSemantic(sc2);
+
+            if (cldec.classKind == ClassKind.cpp)
+            {
+                const DtorDeclaration dtor = s.isDtorDeclaration();
+                if (dtor && cldec.cppDtorVtblIndex == -1)
+                {
+                    if (cldec.baseClass && cldec.baseClass.cppDtorVtblIndex != -1)
+                    {
+                        // override the base virtual
+                        cldec.cppDtorVtblIndex = cldec.baseClass.cppDtorVtblIndex;
+                    }
+                    else if (!dtor.isFinal())
+                    {
+                        // reserve the dtor slot for the destructor (which we'll create later)
+                        cldec.cppDtorVtblIndex = cast(int)cldec.vtbl.dim;
+                        cldec.vtbl.push(s);
+                        if (Target.cppDeletingDestructor)
+                            cldec.vtbl.push(s); // deleting destructor uses a second slot
+                    }
+                }
+            }
         }
 
         if (!cldec.determineFields())
@@ -4854,6 +4885,20 @@ private extern(C++) final class DsymbolSemanticVisitor : Visitor
 
         cldec.dtor = buildDtor(cldec, sc2);
 
+        if (cldec.classKind == ClassKind.cpp && cldec.cppDtorVtblIndex != -1)
+        {
+            // now we've built the aggregate destructor, we'll make it virtual and assign it to the reserved vtable slot
+            cldec.dtor.vtblIndex = cldec.cppDtorVtblIndex;
+            cldec.vtbl[cldec.cppDtorVtblIndex] = cldec.dtor;
+
+            if (Target.cppDeletingDestructor)
+            {
+                // TODO: create a C++ compatible deleting destructor (call out to `operator delete`)
+                //       for the mooment, we'll call the non-deleting destructor and leak
+                cldec.vtbl[cldec.cppDtorVtblIndex + 1] = cldec.dtor;
+            }
+        }
+
         if (auto f = hasIdentityOpAssign(cldec, sc2))
         {
             if (!(f.storage_class & STC.disable))

src/dmd/func.d

@@ -3372,8 +3372,9 @@ extern (C++) final class DtorDeclaration : FuncDeclaration
 
     override bool isVirtual() const
     {
-        // false so that dtor's don't get put into the vtbl[]
-        return false;
+        // D dtor's don't get put into the vtbl[]
+        // this is a hack so that extern(C++) destructors report as virtual, which are manually added to the vtable
+        return vtblIndex != -1;
     }
 
     override bool addPreInvariant()

src/dmd/target.d

@@ -60,6 +60,7 @@ struct Target
         bool cppExceptions;       /// set if catching C++ exceptions is supported
         char int64Mangle;         /// mangling character for C++ int64_t
         char uint64Mangle;        /// mangling character for C++ uint64_t
+        bool cppDeletingDestructor; /// target C++ ABI uses deleting destructor
     }
 
     /**
@@ -115,6 +116,9 @@ struct Target
         ptrsize = 4;
         classinfosize = 0x4C; // 76
 
+        // all systems but windows use deleting destructors
+        cppDeletingDestructor = true;
+
         /* gcc uses int.max for 32 bit compilations, and long.max for 64 bit ones.
          * Set to int.max for both, because the rest of the compiler cannot handle
          * 2^64-1 without some pervasive rework. The trouble is that much of the
@@ -148,6 +152,7 @@ struct Target
             realpad = 0;
             realalignsize = 2;
             reverseCppOverloads = true;
+            cppDeletingDestructor = false;
             c_longsize = 4;
             if (ptrsize == 4)
             {

src/dmd/target.h

@@ -45,6 +45,7 @@ struct Target
     static bool cppExceptions;          // set if catching C++ exceptions is supported
     static char int64Mangle;            // mangling character for C++ int64_t
     static char uint64Mangle;           // mangling character for C++ uint64_t
+    static bool cppDeletingDestructor;  // target C++ ABI uses deleting destructor
 
     template <typename T>
     struct FPTypeProperties

test/runnable/cppa.d

@@ -1264,6 +1264,49 @@ void test16536()
     version(OSX) assert(pass16536(123) == 123);
 }
 
+/****************************************/
+// 15589 - extern(C++) virtual destructors are not put in vtbl[]
+
+extern(C++)
+{
+    class A15589
+    {
+        extern(D) static int[] dtorSeq;
+        struct S
+        {
+            this(int x) { this.x = x; }
+            ~this() { dtorSeq ~= x; }
+            int x;
+        }
+        int foo() { return 100; } // shift dtor to slot 1
+        ~this() { dtorSeq ~= 10; }
+        S s1 = S(1);
+        S s2 = S(2);
+    }
+    class B15589 : A15589
+    {
+        int bar() { return 200;} // add an additional function AFTER the dtor at slot 2
+        ~this() { dtorSeq ~= 20; }
+        S s3 = S(3);
+    }
+
+    void test15589b(A15589 p);
+}
+
+void test15589()
+{
+    A15589 c = new B15589;
+    assert(A15589.dtorSeq == null);
+    assert(c.foo() == 100);
+    assert((cast(B15589)c).bar() == 200);
+    c.__xdtor(); // virtual dtor call
+    assert(A15589.dtorSeq[] == [ 20, 3, 10, 2, 1 ]); // destroyed full hierarchy!
+
+    A15589.dtorSeq = null;
+    test15589b(c);
+    assert(A15589.dtorSeq[] == [ 20, 3, 10, 2, 1 ]); // destroyed full hierarchy!
+}
+
 /****************************************/
 
 void main()
@@ -1308,6 +1351,7 @@ void main()
     test15372();
     test15802();
     test16536();
+    test15589();
 
     printf("Success\n");
 }

test/runnable/extra-files/cppb.cpp

@@ -519,7 +519,7 @@ void test14200b(float a, int b, double c) {};
 
 namespace std {
     namespace N14956 {
-	struct S14956 { };
+    struct S14956 { };
     }
 }
 
@@ -762,7 +762,7 @@ int foo15372(int value)
 
 void test15372b()
 {
-	int t = foo15372<int>(1);
+    int t = foo15372<int>(1);
 }
 
 /****************************************/
@@ -793,7 +793,7 @@ class Foo15802
 
 void test15802b()
 {
-	int t = Foo15802<int>::boo(1);
+    int t = Foo15802<int>::boo(1);
 }
 
 
@@ -806,3 +806,34 @@ uint64_t pass16536(uint64_t a)
     return a;
 }
 #endif
+
+/****************************************/
+// 15589 - extern(C++) virtual destructors are not put in vtbl[]
+
+class A15589
+{
+public:
+    struct S
+    {
+    public:
+        int x;
+    };
+    virtual int foo();
+    virtual ~A15589();
+    S s1;
+    S s2;
+};
+class B15589 : public A15589
+{
+public:
+    virtual int bar();
+    virtual ~B15589();
+    S s3;
+};
+
+void test15589b(A15589 *p)
+{
+    assert(p->foo() == 100);
+    assert(((B15589*)p)->bar() == 200);
+    p->~A15589();
+}