www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - copying a struct containing a struct

reply eskimo <jfanatiker gmx.at> writes:
I hit a strange problem, which seems to be a compiler bug, but I kind of
doubt it, because it is really serious and I did not find anything in
bugzilla. So here is the example code, that behaves really not as
expected and in my understanding just plain and seriously wrong:

---

import std.stdio;

struct Test {
    this(this) {
        writeln("postblit called for: ", &this);
    }
    ref Test opAssign(ref Test other) {
        writeln("opAssign called for: ", &this);
        return this;
    }

    ~this() {
        writeln("Destructed: ", &this);
    }
    int buf;

}
struct TTest {
    Test inner;
}
void main() {
    TTest t1;
    TTest t2;
    writeln("t1: ", &t1.inner);
    writeln("t2: ", &t2.inner);
    t1=t2;
}
---

I would have expected, that opAssign is called for t1 and the
destructors of the two structs. The output:
---
./test1.d
t1: BFF57114
t2: BFF57118
postblit called for: BFF5711C
Destructed: BFF570F4
Destructed: BFF57118
Destructed: BFF57114
---

So instead of opAssign postblit gets called and not even for one of the
two created structs but for some temporary (BFF5711C)? This alone would
be already wrong behaviour, but to make matters worse, for this
temporary not even the destructor is called, instead the destructor of
some other unknown object gets called: BFF570F4.

Does someone have a clue what is going on? Is there a bug report I
missed? 

Btw. if you replace t1=t2; with t1.inner=t2.inner; everything works as
expected.
Nov 16 2012
parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Friday, 16 November 2012 at 21:02:36 UTC, eskimo wrote:

 Btw. if you replace t1=t2; with t1.inner=t2.inner; everything 
 works as
 expected.


What you are seeing is move semantics in action:

When you call t1 = t2, it calls the compiler generated:
"opAssign(TTest other)"

As you can see, pass by value, so a new TTest is created by 
postblit, in the scope of main: "BFF5711C". This new copy is then 
*moved* into the scope of opAssign. At the end of opAssign, it is 
the moved object that is destroyed: "BFF570F4". As for the 
original object, since the compiler knows it was moved, it 
doesn't call the destroyer on it.

Then, at the end of the function, your t1 and t2 are destroyed.

--------
The fact that "opAssign called for: " is not printed is, AFAIK, a 
HUGE and old standing bug: The fields of the struct are bit 
copied (!)

Frankly, I have no idea why it isn't fixed yet...

To "bypass" this "bug", simply define an opAssign yourself.
Nov 16 2012
next sibling parent reply "Rob T" <rob ucora.com> writes:
On Friday, 16 November 2012 at 22:24:09 UTC, monarch_dodra wrote:

 --------
 The fact that "opAssign called for: " is not printed is, AFAIK, 
 a HUGE and old standing bug: The fields of the struct are bit 
 copied (!)

 Frankly, I have no idea why it isn't fixed yet...

 To "bypass" this "bug", simply define an opAssign yourself.


This should be a compiler error or at the very least issue a 
warning.

It seems to me that overridden sub-struct opAssign needs to be 
chained together, and it's a compiler error if there's a break in 
the chain, i.e., if a sub-level struct has non-default opAssign, 
then the parent must have one too, else it's an error.


--rt
Nov 16 2012
parent reply eskimo <jfanatiker gmx.at> writes:
On Fri, 2012-11-16 at 23:45 +0100, Rob T wrote:

 This should be a compiler error or at the very least issue a 
 warning.
 
 It seems to me that overridden sub-struct opAssign needs to be 
 chained together, and it's a compiler error if there's a break in 
 the chain, i.e., if a sub-level struct has non-default opAssign, 
 then the parent must have one too, else it's an error.
 


The default generated should simply call the user specified ones of any
sub struct.

Where can I find this bug? When searching for opAssign I can't find it.
I would like to vote for it.
Nov 16 2012
parent reply "Rob T" <rob ucora.com> writes:
On Friday, 16 November 2012 at 23:35:47 UTC, eskimo wrote:

 The default generated should simply call the user specified 
 ones of any
 sub struct.


I agree that is a more ideal solution. Conceptually, there's 
supposed to be a default opAssign anyway, so it should work as 
you would expect when a nested struct's opAssign is overridden.


 Where can I find this bug? When searching for opAssign I can't 
 find it.
 I would like to vote for it.


I don't see one for this bug either, probably why it's not fixed. 
You should create it.

--rt
Nov 16 2012
parent eskimo <jfanatiker gmx.at> writes:
On Sat, 2012-11-17 at 04:20 +0100, Rob T wrote:

 I don't see one for this bug either, probably why it's not fixed. 
 You should create it.
 
 


I just verified, the compiler swaps the struct contents, so it actually
does the right thing if you demand that structs are relocatable. So in
effect there seems to be no bug.

Best regards,

Robert
Nov 17 2012
prev sibling parent eskimo <jfanatiker gmx.at> writes:
 "opAssign(TTest other)"


Thanks for highlighting this, now it is at least clear what happens. 


 --------
 The fact that "opAssign called for: " is not printed is, AFAIK, a 
 HUGE and old standing bug: The fields of the struct are bit 
 copied (!)


I now re-read the section in TDPL about move semantics:
	First off, D objects must be relocatable, ...

Well now I know why I got a problem, mine are not (not in the example
but in https://github.com/eskimor/phobos/blob/new_signal/std/signals.d
).

I register a delegate to a struct method in the postblit constructor, so
it is not relocatable, that is why things break so horrible. I
deregister the delegate in the destructor, but because of the move
semantics the deregistering is done for the wrong struct.

So in fact the only solution is to forbid copying the struct at all,
which might be the better solution anyway.

Thanks a lot! This really helped. 

Best regards,

Robert
Nov 16 2012