Bug 19430 – wrong code for `this =`, corrupted memory issue

import core.atomic; import core.stdc.stdio; struct S { int i; this(int i) { this.i = i; puts("c"); } this(this) { puts("p"); } ~this() { puts("d"); } this(int r, int e) { this = sum(r, e); } static S sum(int r, int e) { return S(r + e); } } void main() { auto c = S(1, 2); } output: ================= c d d ================= 0 - postblits, 1 - constructor, 2 destructors

I don't see wrong code here. `this = ` is `this.opAssign(rhs)`. That opAssign in this case is implicit, copy is omitted. Add void opAssign(S rhs) { i = rhs.i; puts("a"); } the output will be c a d d

The number of the constructor (including postblits) must match the number of destructors. Assume you have a reference counted member. Then the code in the issue will cause a corrupted memory error.

You forgot a `puts` in the two-argument constructor. There are exactly two instances being constructed. One by returning from `sum`, the other one is `c` in main. First "d" in output is from assignment, second is on exiting main. All seems quite legit, i.e. WAD.

Inter(In reply to Stanislav Blinov from comment #3) > You forgot a `puts` in the two-argument constructor. There are exactly two > instances being constructed. One by returning from `sum`, the other one is > `c` in main. First "d" in output is from assignment, second is on exiting > main. All seems quite legit, i.e. WAD. Ah, yes. In the same time, I am sure there is definitely an issue. Will try to find better code example

I'll close this for now, please reopen as needed.

(In reply to Stanislav Blinov from comment #5) > I'll close this for now, please reopen as needed. import core.atomic; import core.stdc.stdio; import core.stdc.stdlib; struct S { static shared int* ptr; this(int i) { ptr = cast(shared int*) malloc(4); *ptr = 1; } pragma(inline, false) this(this) { if (ptr && *ptr) atomicOp!"+="(*ptr, 1); } ~this() { if (ptr && *ptr) atomicOp!"-="(*ptr, 1); } this(int r, int e) { this = sum(r, e); } static S sum(int r, int e) { return S(r + e); } } void main() { auto s = S(1, 2); import std.stdio; writeln(*s.ptr); } ================ output: 0 expected: 1

What's going on is (pseudo-code) this: c = S(S.init).opAssign(S.sum(1, 2)); The opAssign is implicitly generated. Two instances are constructed in-place, one is 'main.s', the other is the argument to opAssign, no copies are made, no postblits are called. But then the argument of opAssign is getting destructed, and decrements count to 0. So 0 is the correct output here. For it to be 1, you *need* an explicit opAssign: void opAssign(S rhs) { if (ptr && *ptr) atomicOp!"+="(*ptr, 1); } If it were a C++ std::shared_ptr kind of reference count (i.e. not a static counter), then you'd need this kind of opAssign: void opAssign(S rhs) { import std.algorithm.mutation : swap; swap(ptr, rhs.ptr); }

Working example: https://run.dlang.io/is/dnxQNx

Ok, requires more research

To be fair, calling assignment on an instance that hasn't been yet constructed (i.e. constructor didn't return) isn't the best of ideas. I know Phobos is doing this in places, but it's something that should really, really be avoided.

I am starting to dig deeper since I have more code that uses RC with D. The first one issue is https://issues.dlang.org/show_bug.cgi?id=19774 The second one related to generic `=` operator, still needs to be reduced.