From 403d5f0148abe60ac1836da9038c18cebd8b8644 Mon Sep 17 00:00:00 2001 From: Rainer Schuetze Date: Sat, 16 Jun 2018 15:01:10 +0200 Subject: [PATCH] fix Issue 18966 - extern(C++) constructor should match C++ semantics assigning vtable set vtbl after exlicit or implicit call to super() in C++ constructors --- src/dmd/aggregate.h | 2 +- src/dmd/dclass.d | 17 ++++++++++- src/dmd/expressionsem.d | 29 +++++++++++++++++- src/dmd/tocsym.d | 8 +++-- src/dmd/toobj.d | 14 ++++----- test/runnable/cppa.d | 48 ++++++++++++++++++++++++++++++ test/runnable/extra-files/cppb.cpp | 9 ++++++ test/runnable/extra-files/cppb.h | 9 ++++++ 8 files changed, 123 insertions(+), 13 deletions(-) diff --git a/src/dmd/aggregate.h b/src/dmd/aggregate.h index 16f25ea1ccee..e91278a7d452 100644 --- a/src/dmd/aggregate.h +++ b/src/dmd/aggregate.h @@ -324,7 +324,7 @@ class ClassDeclaration : public AggregateDeclaration void addLocalClass(ClassDeclarations *); // Back end - Symbol *vtblsym; + Dsymbol *vtblsym; ClassDeclaration *isClassDeclaration() { return (ClassDeclaration *)this; } void accept(Visitor *v) { v->visit(this); } diff --git a/src/dmd/dclass.d b/src/dmd/dclass.d index b2349548f2c7..a57edfbfa1e0 100644 --- a/src/dmd/dclass.d +++ b/src/dmd/dclass.d @@ -973,7 +973,22 @@ extern (C++) class ClassDeclaration : AggregateDeclaration } // Back end - Symbol* vtblsym; + Dsymbol vtblsym; + + final Dsymbol vtblSymbol() + { + if (!vtblsym) + { + auto vtype = Type.tvoidptr.immutableOf(); + auto var = new VarDeclaration(loc, vtype, Identifier.idPool("__vtbl"), null, STC.immutable_ | STC.static_); + var.addMember(null, this); + var.isdataseg = 1; + var.linkage = LINK.d; + var.semanticRun = PASS.semanticdone; // no more semantic wanted + vtblsym = var; + } + return vtblsym; + } override final inout(ClassDeclaration) isClassDeclaration() inout { diff --git a/src/dmd/expressionsem.d b/src/dmd/expressionsem.d index 04c40846d6fc..9eff953b2799 100644 --- a/src/dmd/expressionsem.d +++ b/src/dmd/expressionsem.d @@ -3105,6 +3105,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor return f; } + bool isSuper = false; if (exp.e1.op == TOK.dotVariable && t1.ty == Tfunction || exp.e1.op == TOK.dotTemplateDeclaration) { UnaExp ue = cast(UnaExp)exp.e1; @@ -3250,7 +3251,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor auto ad = sc.func ? sc.func.isThis() : null; auto cd = ad ? ad.isClassDeclaration() : null; - const bool isSuper = exp.e1.op == TOK.super_; + isSuper = exp.e1.op == TOK.super_; if (isSuper) { // Base class constructor call @@ -3588,6 +3589,32 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor } result = Expression.combine(argprefix, exp); + + if (isSuper) + { + auto ad = sc.func ? sc.func.isThis() : null; + auto cd = ad ? ad.isClassDeclaration() : null; + if (cd && cd.classKind == ClassKind.cpp) + { + // if super is defined in C++, it sets the vtable pointer to the base class + // so we have to rewrite it, but still return 'this' from super() call: + // (auto tmp = super(), this.__vptr = __vtbl, tmp) + __gshared int superid = 0; + char[20] buf; + sprintf(buf.ptr, "__super%d", superid++); + auto tmp = copyToTemp(0, buf.ptr, result); + Loc loc = exp.loc; + Expression tmpdecl = new DeclarationExp(loc, tmp); + + auto dse = new DsymbolExp(loc, cd.vtblSymbol()); + auto ase = new AddrExp(loc, dse); + auto pte = new DotIdExp(loc, new ThisExp(loc), Id.__vptr); + auto ate = new AssignExp(loc, pte, ase); + + Expression e = new CommaExp(loc, new CommaExp(loc, tmpdecl, ate), new VarExp(loc, tmp)); + result = e.expressionSemantic(sc); + } + } } override void visit(DeclarationExp e) diff --git a/src/dmd/tocsym.d b/src/dmd/tocsym.d index f96ea8078722..b967db885d30 100644 --- a/src/dmd/tocsym.d +++ b/src/dmd/tocsym.d @@ -549,7 +549,7 @@ Classsym *fake_classsym(Identifier id) Symbol *toVtblSymbol(ClassDeclaration cd) { - if (!cd.vtblsym) + if (!cd.vtblsym || !cd.vtblsym.csym) { if (!cd.csym) toSymbol(cd); @@ -559,9 +559,11 @@ Symbol *toVtblSymbol(ClassDeclaration cd) auto s = toSymbolX(cd, "__vtbl", SCextern, t, "Z"); s.Sflags |= SFLnodebug; s.Sfl = FLextern; - cd.vtblsym = s; + + auto vtbl = cd.vtblSymbol(); + vtbl.csym = s; } - return cd.vtblsym; + return cd.vtblsym.csym; } /********************************** diff --git a/src/dmd/toobj.d b/src/dmd/toobj.d index 6fcc1662deb1..042478e9b101 100644 --- a/src/dmd/toobj.d +++ b/src/dmd/toobj.d @@ -419,13 +419,13 @@ void toObjFile(Dsymbol ds, bool multiobj) */ dtbv.size(0); } - cd.vtblsym.Sdt = dtbv.finish(); - cd.vtblsym.Sclass = scclass; - cd.vtblsym.Sfl = FLdata; - out_readonly(cd.vtblsym); - outdata(cd.vtblsym); + cd.vtblsym.csym.Sdt = dtbv.finish(); + cd.vtblsym.csym.Sclass = scclass; + cd.vtblsym.csym.Sfl = FLdata; + out_readonly(cd.vtblsym.csym); + outdata(cd.vtblsym.csym); if (cd.isExport()) - objmod.export_symbol(cd.vtblsym,0); + objmod.export_symbol(cd.vtblsym.csym,0); } override void visit(InterfaceDeclaration id) @@ -1264,7 +1264,7 @@ private void genClassInfoForClass(ClassDeclaration cd, Symbol* sinit) // vtbl[] dtb.size(cd.vtbl.dim); if (cd.vtbl.dim) - dtb.xoff(cd.vtblsym, 0, TYnptr); + dtb.xoff(cd.vtblsym.csym, 0, TYnptr); else dtb.size(0); diff --git a/test/runnable/cppa.d b/test/runnable/cppa.d index 161f5eb2f7bc..f08f782b4bc2 100644 --- a/test/runnable/cppa.d +++ b/test/runnable/cppa.d @@ -1441,6 +1441,53 @@ void test18928() assert(s.x == 5); } +/****************************************/ +// https://issues.dlang.org/show_bug.cgi?id=18966 + +extern(C++): +class Base18966 +{ + this() @safe nothrow; + ~this(); + void vf(); + int x; +} + +class Derived18966 : Base18966 +{ + override void vf() { x = 200; } +} + +class Explicit18966 : Base18966 +{ + this() @safe { super(); } + override void vf() { x = 250; } +} + +class Implicit18966 : Base18966 +{ + this() nothrow {} + override void vf() { x = 300; } +} + +void test18966() +{ + Derived18966 d = new Derived18966; + assert(d.x == 10); + d.vf(); + assert(d.x == 200); + + Explicit18966 e = new Explicit18966; + assert(e.x == 10); + e.vf(); + assert(e.x == 250); + + Implicit18966 i = new Implicit18966; + assert(i.x == 10); + i.vf(); + assert(i.x == 300); +} + /****************************************/ void main() @@ -1488,6 +1535,7 @@ void main() test15589(); test15589b(); test18928(); + test18966(); printf("Success\n"); } diff --git a/test/runnable/extra-files/cppb.cpp b/test/runnable/extra-files/cppb.cpp index a88721de35bf..6f07fbde8f43 100644 --- a/test/runnable/extra-files/cppb.cpp +++ b/test/runnable/extra-files/cppb.cpp @@ -918,3 +918,12 @@ CC18928* newCC18928() { return new CC18928(); } + +/****************************************/ +// https://issues.dlang.org/show_bug.cgi?id=18966 +Base18966::Base18966() { x = 10; } +Base18966::~Base18966() {} +void Base18966::vf() +{ + x = 100; +} diff --git a/test/runnable/extra-files/cppb.h b/test/runnable/extra-files/cppb.h index a78d79205500..0aec779dc16e 100644 --- a/test/runnable/extra-files/cppb.h +++ b/test/runnable/extra-files/cppb.h @@ -56,3 +56,12 @@ struct Cpp15589Struct ~Cpp15589Struct(); int s; }; + +class Base18966 +{ +public: + Base18966(); + virtual ~Base18966(); + virtual void vf(); + int x; +};