From 008ce0cd1ca8e0a2949cf3364087de0a0aec5997 Mon Sep 17 00:00:00 2001 From: JinShil Date: Wed, 18 Apr 2018 10:04:06 +0900 Subject: [PATCH 1/3] Optimization: Don't generate try-catch for nothrow postblits --- src/dmd/dsymbolsem.d | 136 +++++++++++++++++++++++++++---------------- 1 file changed, 85 insertions(+), 51 deletions(-) diff --git a/src/dmd/dsymbolsem.d b/src/dmd/dsymbolsem.d index 60d63a63c77b..acca36acf281 100644 --- a/src/dmd/dsymbolsem.d +++ b/src/dmd/dsymbolsem.d @@ -97,6 +97,7 @@ private extern (C++) FuncDeclaration buildPostBlit(StructDeclaration sd, Scope* stc |= sd.postblits[i].storage_class & STC.disable; } + VarDeclaration[] fieldsToDestroy; auto postblitCalls = new Statements(); // iterate through all the struct fields that are not disabled for (size_t i = 0; i < sd.fields.dim && !(stc & STC.disable); i++) @@ -116,6 +117,84 @@ private extern (C++) FuncDeclaration buildPostBlit(StructDeclaration sd, Scope* continue; assert(!sdv.isUnionDeclaration()); + // if this field's postblit is not `nothrow`, add a `scope(failure)` + // block to destroy any prior successfully postblitted fields should + // this field's postblit fail + if (fieldsToDestroy.length > 0 && !(cast(TypeFunction)sdv.postblit.type).isnothrow) + { + // create a list of destructors that need to be called + Expression[] dtorCalls; + foreach(sf; fieldsToDestroy) + { + Expression ex; + tv = sf.type.toBasetype(); + if (tv.ty == Tstruct) + { + // this.v.__xdtor() + + ex = new ThisExp(loc); + ex = new DotVarExp(loc, ex, sf); + + // This is a hack so we can call destructors on const/immutable objects. + ex = new AddrExp(loc, ex); + ex = new CastExp(loc, ex, sf.type.mutableOf().pointerTo()); + ex = new PtrExp(loc, ex); + if (stc & STC.safe) + stc = (stc & ~STC.safe) | STC.trusted; + + auto sfv = (cast(TypeStruct)sf.type.baseElemOf()).sym; + + ex = new DotVarExp(loc, ex, sfv.dtor, false); + ex = new CallExp(loc, ex); + + dtorCalls ~= ex; + } + else + { + // _ArrayDtor((cast(S*)this.v.ptr)[0 .. n]) + + uinteger_t length = 1; + while (tv.ty == Tsarray) + { + length *= (cast(TypeSArray)tv).dim.toUInteger(); + tv = tv.nextOf().toBasetype(); + } + //if (n == 0) + // continue; + + ex = new ThisExp(loc); + ex = new DotVarExp(loc, ex, sf); + + // This is a hack so we can call destructors on const/immutable objects. + ex = new DotIdExp(loc, ex, Id.ptr); + ex = new CastExp(loc, ex, sdv.type.pointerTo()); + if (stc & STC.safe) + stc = (stc & ~STC.safe) | STC.trusted; + + ex = new SliceExp(loc, ex, new IntegerExp(loc, 0, Type.tsize_t), + new IntegerExp(loc, length, Type.tsize_t)); + // Prevent redundant bounds check + (cast(SliceExp)ex).upperIsInBounds = true; + (cast(SliceExp)ex).lowerIsLessThanUpper = true; + + ex = new CallExp(loc, new IdentifierExp(loc, Id._ArrayDtor), ex); + + dtorCalls ~= ex; + } + } + fieldsToDestroy = []; + + // aggregate the destructor calls + auto dtors = new Statements(); + foreach_reverse(dc; dtorCalls) + { + dtors.push(new ExpStatement(loc, dc)); + } + + // put destructor calls in a `scope(failure)` block + postblitCalls.push(new OnScopeStatement(loc, TOK.onScopeFailure, new CompoundStatement(loc, dtors))); + } + // perform semantic on the member postblit in order to // be able to aggregate it later on with the rest of the // postblits @@ -184,62 +263,17 @@ private extern (C++) FuncDeclaration buildPostBlit(StructDeclaration sd, Scope* postblitCalls.push(new ExpStatement(loc, ex)); // combine in forward order /* https://issues.dlang.org/show_bug.cgi?id=10972 - * When the following field postblit calls fail, + * When subsequent field postblit calls fail, * this field should be destructed for Exception Safety. */ - if (!sdv.dtor) - continue; - sdv.dtor.functionSemantic(); - - tv = structField.type.toBasetype(); - if (tv.ty == Tstruct) - { - // this.v.__xdtor() - - ex = new ThisExp(loc); - ex = new DotVarExp(loc, ex, structField); - - // This is a hack so we can call destructors on const/immutable objects. - ex = new AddrExp(loc, ex); - ex = new CastExp(loc, ex, structField.type.mutableOf().pointerTo()); - ex = new PtrExp(loc, ex); - if (stc & STC.safe) - stc = (stc & ~STC.safe) | STC.trusted; - - ex = new DotVarExp(loc, ex, sdv.dtor, false); - ex = new CallExp(loc, ex); - } - else + if (sdv.dtor) { - // _ArrayDtor((cast(S*)this.v.ptr)[0 .. n]) - - uinteger_t length = 1; - while (tv.ty == Tsarray) - { - length *= (cast(TypeSArray)tv).dim.toUInteger(); - tv = tv.nextOf().toBasetype(); - } - //if (n == 0) - // continue; - - ex = new ThisExp(loc); - ex = new DotVarExp(loc, ex, structField); - - // This is a hack so we can call destructors on const/immutable objects. - ex = new DotIdExp(loc, ex, Id.ptr); - ex = new CastExp(loc, ex, sdv.type.pointerTo()); - if (stc & STC.safe) - stc = (stc & ~STC.safe) | STC.trusted; - - ex = new SliceExp(loc, ex, new IntegerExp(loc, 0, Type.tsize_t), - new IntegerExp(loc, length, Type.tsize_t)); - // Prevent redundant bounds check - (cast(SliceExp)ex).upperIsInBounds = true; - (cast(SliceExp)ex).lowerIsLessThanUpper = true; + sdv.dtor.functionSemantic(); - ex = new CallExp(loc, new IdentifierExp(loc, Id._ArrayDtor), ex); + // keep a list of fields that need to be destroyed in case + // of a future postblit failure + fieldsToDestroy ~= structField; } - postblitCalls.push(new OnScopeStatement(loc, TOK.onScopeFailure, new ExpStatement(loc, ex))); } void checkShared() From 4427a382f61cf764624b3e31d13b772790376803 Mon Sep 17 00:00:00 2001 From: JinShil Date: Wed, 18 Apr 2018 16:43:49 +0900 Subject: [PATCH 2/3] Add field postblit exception safety test --- test/runnable/xpostblit.d | 59 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/test/runnable/xpostblit.d b/test/runnable/xpostblit.d index b364c4cff63a..8f8a59d093f7 100644 --- a/test/runnable/xpostblit.d +++ b/test/runnable/xpostblit.d @@ -70,8 +70,67 @@ void test2() @safe @nogc nothrow assert(Counter.cnt == 1); } +/**************************************************************** + This test is intended to verify the exception safety of field + postblits +*/ +string trace = ""; + +struct FieldThrow +{ + string name; + this(string n) + { + name = n; + } + + bool throwExcept; + this(this) + { + if (throwExcept) + { + throw new Exception(""); + } + } + + ~this() { trace ~= name ~ ".dtor"; } +} + +struct S +{ + auto f1 = FieldThrow("f1"); + FieldThrow[2] f2f3= [FieldThrow("f2"), FieldThrow("f3")]; + auto f4 = FieldThrow("f4"); +} + +void test3() +{ + trace = ""; + + S s1; + + // Cause `s1.f4`'s postblit to throw + s1.f4.throwExcept = true; + + try + { + // `s`'s postblit will be a combination of `f1`, `f2f3`, and `f4`'s + // postblit in that order. However, `f4`'s postblit will throw, + // causing `s1.f2f3` and `s1.f1`'s destructors to execute in that + // order + S s2 = s1; + } + catch(Exception ex){ } + + // Confirm the field destructors were called and were called in the + // corrrect order + assert(trace == "f3.dtor" ~ "f2.dtor" ~ "f1.dtor"); +} +/****************************************************************************/ + void main() { test1(); test2(); + test3(); } From 8a33fb6c692d0b2c1f03ceeb65c28beaebd932a0 Mon Sep 17 00:00:00 2001 From: JinShil Date: Tue, 24 Apr 2018 10:29:07 +0900 Subject: [PATCH 3/3] Add test for issue 18493 --- test/runnable/betterc.d | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/runnable/betterc.d b/test/runnable/betterc.d index be4573eb9497..b1f151d0ff15 100644 --- a/test/runnable/betterc.d +++ b/test/runnable/betterc.d @@ -36,3 +36,19 @@ extern (C) void test17605() a = 1; } +/*******************************************/ +// https://issues.dlang.org/show_bug.cgi?id=18493 + +struct S18493 +{ + this(this) nothrow { } // Since this is attributed with `nothrow` there should be no error about using + // try-catch with -betterC + ~this() { } +} + +struct S18493_2 +{ + S18493 s1; + S18493 s2; +} +