Bug 10789 – Struct destructor erroneously called

The attached program simulates a simple reference counted struct. 'fun' is supposed to return a newly initialized S with a count of 1. Instead it calls the destructor dropping the count to zero and then returns a copy of the initialized struct. Leaving out the 'if' statment and returning the fresh 'S' directly does not exibit this behavior. This issue is critical as it has a high probability to indroduce hard to detect/track down bugs when automatic reference counting is used. Tested on DMD 2.063.2/Win32 and /Win64 and DMD master/Win32.

Created attachment 1241 Reproduction case

Reduced: extern(C) int printf(const char*, ...); struct S { static int count; this(int ignoreme) { int oldcount = count; printf("%X ctor %d=>%d\n", &this, oldcount, ++count); } ~this() { int oldcount = count; printf("%X dtor %d=>%d\n", &this, oldcount, --count); } this(this) { int oldcount = count; printf("%X postblit %d=>%d\n", &this, oldcount, ++count); } } S fun() { S s1 = S(42), s2 = S(24); if (true) return s1; else return s2; } void main() { S s = fun(); } In case of if(true) compiler does not insert postblit call.

I digged a little in the DMD sources and found a commit by Kenji Hara that at least affects this example and has a commented out code block that looks a little suspicious : https://github.com/D-Programming-Language/dmd/commit/b4bc25d72e601436f3999abc5c9c31e464385052#L4R1241 Changing "#if 0//DMDV2" back to "#if DMDV2" inserts a proper postblit call, but then the returned t has its initialized field set to false. This does not happen with the "#if 0" AFAICS. Unfortunately I know far to less about the mechanics at work there to make an informed attempt to fix this.

(In reply to comment #3) > I digged a little in the DMD sources and found a commit by Kenji Hara that at > least affects this example and has a commented out code block that looks a > little suspicious : > https://github.com/D-Programming-Language/dmd/commit/b4bc25d72e601436f3999abc5c9c31e464385052#L4R1241 > > Changing "#if 0//DMDV2" back to "#if DMDV2" inserts a proper postblit call, but > then the returned t has its initialized field set to false. This does not > happen with the "#if 0" AFAICS. Unfortunately I know far to less about the > mechanics at work there to make an informed attempt to fix this. Sounds like a regression.

Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/4ca445bb2564997b80d5c00c6dfc1daeff1e30af fix Issue 10789 - Struct destructor erroneously called https://github.com/D-Programming-Language/dmd/commit/cfffc9b02fed9366babb6712cba7d6f26c18df6e Merge pull request #2523 from 9rnsr/fix10789 [REG2.061] Issue 10789 - Struct destructor erroneously called

The original test case still fails on DMD HEAD: --- 0018FD74 this() 0 0018FD75 this(this) 1 0018FD74 ~this() 2 0018FD9C ~this() 1 core.exception.AssertError@app(47): Assertion failure --- This is due to the last destructor running on an uninitialized instance (initialized == false).

(In reply to comment #7) > The original test case still fails on DMD HEAD: > > --- > 0018FD74 this() 0 > 0018FD75 this(this) 1 > 0018FD74 ~this() 2 > 0018FD9C ~this() 1 > core.exception.AssertError@app(47): Assertion failure > --- > > This is due to the last destructor running on an uninitialized instance > (initialized == false). To me it looks like that the original test case contains a bug. In S.this(this), `initialized` field is incorrectly set to false. It will stop to decrement S.count at the destruction of the copied objects. Therefore the last assertion in main fails because S.count == 1.

Sorry, you are absolutely right. The "initialized = false" was supposed to go to the destructor instead to test if the destructor is called twice on the same instance. I'm still seeing a similar issue in my project, but I can't reproduce it with the fixed this(this).