While finalizing the LDC 2.067 merge, Martin and I stumbled over major DMD codegen issues for structs with destructors as function arguments. [1]
Consider this program:
---
import core.stdc.stdio;
struct Foo {
const char* name;
bool throwInDtor;
this(const char* name, bool throwInDtor) {
printf("Constructing %s\n", name);
this.name = name;
this.throwInDtor = throwInDtor;
}
~this() {
printf("Destroying %s\n", name);
if (throwInDtor)
throw new Exception("dtor " ~ name[0]);
}
}
Foo make(const char* name, bool doThrow) {
if (doThrow)
throw new Exception("make()");
return Foo(name, false);
}
void foo(Foo a, Foo b, Foo c, Foo d) { throw new Exception("foo()!"); }
void main() {
foo(Foo("a", true), make("b", true), Foo("c", false), make("d", true));
}
---
Depending on the arguments to the Foo constructor and make, we can get different test cases for exception chaining and destruction of temporaries (DMD 2.068.0, OS X x86_64):
---
1) foo(Foo("a", true), make("b", false), Foo("c", true), make("d", false))
Constructing a
Constructing b
Constructing c
Constructing d
Destroying a
Destroying b
Destroying c
Destroying d
[email protected](27): foo()!
[email protected](17): dtor a
[email protected](17): dtor c
2) foo(Foo("a", true), make("b", false), Foo("c", true), make("d", true))
Constructing a
Constructing b
Constructing c
[email protected](23): make()
3) foo(Foo("a", false), make("b", false), Foo("c", false), make("d", true))
Constructing a
Constructing b
Constructing c
[email protected](23): make()
4) foo(Foo("a", false), make("b", true), Foo("c", false), make("d", true))
Constructing a
[email protected](23): make()
5) foo(Foo("a", true), make("b", true), Foo("c", false), make("d", true))
Constructing a
[email protected](23): make()
---
1) is correct, although the order of destruction is somewhat unexpected (given that they are destroyed by the callee, this is probably not wrong, though).
However, the destructors in 2) to 5) are never called.
[1] Our issue is at https://github.com/ldc-developers/ldc/issues/1010#issuecomment-129596359
Comment #1 by bugzilla — 2015-08-11T09:58:36Z
Throwing in a destructor is a nightmare, it makes my brain hurt just trying to figure out what 'should' happen. I've proposed before that destructors should be nothrow.
Comment #2 by dfj1esp02 — 2015-08-11T11:40:08Z
Imagine resource management with reference counting, then destructors can throw due to environmental issues.
(stdio.File destructor throws)
Comment #3 by code — 2015-08-11T12:59:52Z
Note that all those cases do not (yet) fail because of the throwing dtors. It's that the dtors aren't even invoked in the first place.
Comment #4 by kinke — 2015-08-11T18:06:55Z
Well, what 'should' happen looks pretty straight-forward to me - the dtors of all live temporaries are to be called in any case, and any exceptions thrown by dtors are to be linked to an exception chain. That's what D already does for params/locals, as shown by case 1.
I just find it sad that such a horrible bug can make it into a 2nd fresh release. It's literally one of the first things I was thinking about when tackling proper destruction of complex expression trees for LDC. We definitely need more testing, and define test cases and expected outcomes before implementing new, non-trivial features.
Comment #5 by code — 2015-08-11T18:24:58Z
*** Issue 14902 has been marked as a duplicate of this issue. ***
Comment #6 by kinke — 2015-08-11T19:06:00Z
---
import core.stdc.stdio;
struct Struct {
int a = 6;
~this() {
printf("dtor\n");
throw new Exception("big bang");
}
}
int foo(bool doThrow) {
printf("foo()\n");
if (doThrow)
throw new Exception("bla");
return 1;
}
void main()
{
bool doThrow = true;
int r = Struct().a + foo(doThrow);
}
---
DMD 2.068.0, Win32:
foo()
dtor
dtor
dtor
...
crash
Works as expected if there's only a single throw (either in foo() or in dtor).
Comment #7 by code — 2015-08-13T12:23:19Z
This patch should fix the issue where no dtors are run at all:
—————
--- a/dmd2/expression.c
+++ b/dmd2/expression.c
@@ -1889,10 +1889,7 @@ bool functionParameters(Loc loc, Scope *sc, TypeFunction *tf,
}
}
}
- if (anythrow && i == lastthrow)
- {
- appendToPrefix = false;
- }
+
if (appendToPrefix) // don't need to add to prefix until there's something to destruct
{
Identifier *idtmp = Identifier::generateId("__pfx");
@@ -1901,8 +1898,14 @@ bool functionParameters(Loc loc, Scope *sc, TypeFunction *tf,
tmp->semantic(sc);
/* Modify the destructor so it only runs if gate==false
+ * We immediately set the gate to true after the last throwing argument
+ * has been constructed, so we can directly fold away the check.
*/
- if (tmp->edtor)
+ if (i == lastthrow)
+ {
+ tmp->edtor = NULL;
+ }
+ else if(tmp->edtor)
{
Expression *e = tmp->edtor;
e = new OrOrExp(e->loc, new VarExp(e->loc, gate), e); // (gate || destructor)
—————
Previously, the gate was set to true before the last throwing argument, which is obviously wrong if it indeed throws.
Don't have time to turn this into a proper DMD PR myself at the moment, though.