www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Built-in array opSliceAssign

reply Eduard Staniloiu <edi33416 gmail.com> writes:
Hello, everyone!

I have a question regarding the expected behaviour of the 
built-in array's opSliceAssign.

Given the following code:

```
import std.stdio;

struct A
{
     int x;

     ref A opAssign(A rhs)
     {
         writefln("slice_bug.opAssign: begin");

         return this;
     }
}

void main(string[] args)
{
     A[] a = [A(1), A(2), A(3)];
     A[] b = [A(2), A(3), A(4)];

     // Expecting opAssign to be called for every element in a
     a[] = b[];

     // In other words, I was under the impression that the above
     // is sintactic-sugar for
     for (size_t i = 0; i < a.lenght; ++i)
     {
         a[i] = b[i]; // This calls opAssign, as expected
     }
}
```

As I wrote in the comments above, I was expecting `a[] = b[]` to 
iterate the slices and assign the elements of b into a.

What really happens is a memcpy: as you can see from godblot [0], 
this gets lowered to a call to `_d_arraycopy`, in druntime.

I'm not sure if this is the intended behaviour, though.
I'm saying this as I've got bitten by this, because I'm doing 
reference counting inside opAssign.

IMHO, this is a bug. The code should lower to calls to opAssing 
for types that define opAssign.

I've also pasted the code on https://run.dlang.io/is/vneELO

[0] - https://godbolt.org/z/_IXCAV
Oct 25 2018
next sibling parent reply Paul Backus <snarwin gmail.com> writes:
On Thursday, 25 October 2018 at 12:25:37 UTC, Eduard Staniloiu 
wrote:
 As I wrote in the comments above, I was expecting `a[] = b[]` 
 to iterate the slices and assign the elements of b into a.

 What really happens is a memcpy: as you can see from godblot 
 [0], this gets lowered to a call to `_d_arraycopy`, in druntime.
In D, when you assign one aggregate to another, opAssign is only called for the aggregate, not for any of its elements. However, postblit constructors are called for both. Example: https://run.dlang.io/is/XfDaWw Your example will work as expected if you change the opAssign to a postblit constructor: https://run.dlang.io/is/HBbGO2
Oct 25 2018
parent reply Eduard Staniloiu <edi33416 gmail.com> writes:
On Thursday, 25 October 2018 at 12:38:44 UTC, Paul Backus wrote:
 On Thursday, 25 October 2018 at 12:25:37 UTC, Eduard Staniloiu 
 wrote:
 As I wrote in the comments above, I was expecting `a[] = b[]` 
 to iterate the slices and assign the elements of b into a.

 What really happens is a memcpy: as you can see from godblot 
 [0], this gets lowered to a call to `_d_arraycopy`, in 
 druntime.
In D, when you assign one aggregate to another, opAssign is only called for the aggregate, not for any of its elements. However, postblit constructors are called for both. Example: https://run.dlang.io/is/XfDaWw
Can you, please, give me a link to where it says this in the specs? Based on the example, I would expect that the code gets lowered to some version of `_d_arrayassign` [0]. I still think that this is problematic, as it's unexpected to the user: you're expecting the assignment operator to be called, not the postblit. I know that the compiler can and will create an opAssign if a postblit or dtor is defined, as you can write the assignment as a this._dtor; blit rhs into this. This being said, I think that if the user took the time to define opAssign, it should be called, because he might want to do something extra when an assignment occurs: an ex. having different logs "creating new obj" vs "changing existing obj".
 Your example will work as expected if you change the opAssign 
 to a postblit constructor: https://run.dlang.io/is/HBbGO2
Oct 25 2018
parent Eduard Staniloiu <edi33416 gmail.com> writes:
On Thursday, 25 October 2018 at 13:01:06 UTC, Eduard Staniloiu 
wrote:
 On Thursday, 25 October 2018 at 12:38:44 UTC, Paul Backus wrote:
 On Thursday, 25 October 2018 at 12:25:37 UTC, Eduard Staniloiu 
 wrote:
 As I wrote in the comments above, I was expecting `a[] = b[]` 
 to iterate the slices and assign the elements of b into a.

 What really happens is a memcpy: as you can see from godblot 
 [0], this gets lowered to a call to `_d_arraycopy`, in 
 druntime.
In D, when you assign one aggregate to another, opAssign is only called for the aggregate, not for any of its elements. However, postblit constructors are called for both. Example: https://run.dlang.io/is/XfDaWw
Can you, please, give me a link to where it says this in the specs? Based on the example, I would expect that the code gets lowered to some version of `_d_arrayassign` [0]. I still think that this is problematic, as it's unexpected to the user: you're expecting the assignment operator to be called, not the postblit. I know that the compiler can and will create an opAssign if a postblit or dtor is defined, as you can write the assignment as a this._dtor; blit rhs into this. This being said, I think that if the user took the time to define opAssign, it should be called, because he might want to do something extra when an assignment occurs: an ex. having different logs "creating new obj" vs "changing existing obj".
 Your example will work as expected if you change the opAssign 
 to a postblit constructor: https://run.dlang.io/is/HBbGO2
Accidentally sent to early. One extra reason as to why, imho, this implicit call to the postblit is evil, is that a lot of code will probably break when we'll transition to the CopyConstructor. See RazvanN's PR [0]. This is actually how I stumbled upon this, as I am using his branch with my repo. [0] - https://github.com/dlang/dmd/pull/8688
Oct 25 2018
prev sibling parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 25 October 2018 at 12:25:37 UTC, Eduard Staniloiu 
wrote:
 IMHO, this is a bug. The code should lower to calls to opAssing 
 for types that define opAssign.
The spec doesn't exactly say it uses memset, but it does imply it: https://dlang.org/spec/arrays.html#array-copying talking about "aggressive parallel code optimizations than possible with the serial semantics of C" and "copying" rather than "assigned" etc. but indeed postblit lets this work.
Oct 25 2018
parent reply Eduard Staniloiu <edi33416 gmail.com> writes:
On Thursday, 25 October 2018 at 12:55:38 UTC, Adam D. Ruppe wrote:
 On Thursday, 25 October 2018 at 12:25:37 UTC, Eduard Staniloiu 
 wrote:
 IMHO, this is a bug. The code should lower to calls to 
 opAssing for types that define opAssign.
The spec doesn't exactly say it uses memset, but it does imply it: https://dlang.org/spec/arrays.html#array-copying talking about "aggressive parallel code optimizations than possible with the serial semantics of C" and "copying" rather than "assigned" etc. but indeed postblit lets this work.
You are right. Thank you! I guess I never read/understood it like this. I expected it to use opAssign as that is what's the most natural and intuitive decision for me. I take it that this is the expected behaviour, then.
Oct 25 2018
parent reply Stanislav Blinov <stanislav.blinov gmail.com> writes:
On Thursday, 25 October 2018 at 13:22:36 UTC, Eduard Staniloiu 
wrote:

 The spec doesn't exactly say it uses memset, but it does imply 
 it:

 https://dlang.org/spec/arrays.html#array-copying

 talking about "aggressive parallel code optimizations than 
 possible with the serial semantics of C" and "copying" rather 
 than "assigned" etc.

 but indeed postblit lets this work.
You are right. Thank you! I guess I never read/understood it like this. I expected it to use opAssign as that is what's the most natural and intuitive decision for me. I take it that this is the expected behaviour, then.
I don't think interpreting the spec like that is correct. It says "...it means that the *contents* of the array are the target of the *assignment*...", and further, in the examples: s[1..2] = t[0..1]; // same as s[1] = t[0] s[0..2] = t[1..3]; // same as s[0] = t[1], s[1] = t[2] The current behavior of the compiler is quite the opposite of those "same as" above. Consider: struct S { disable this(this); // can't implicitly copy, i.e. pass lvalue to a function void opAssign(ref S rhs) { /* ... */ } // but can explicitly copy } void main() { S x, y; x = y; // works S[10] a; a[0 .. 3] = y; // doesn't compile, i.e. NOT "same as a[0] = y, a[1] = y, a[2] = y" a[0 .. 3] = a[3 .. 6]; // doesn't compile either, i.e. NOT "same as a[0] = a[3], a[1] = a[4], a[2] = a[5]" } I've filed an issue about this around a month ago: https://issues.dlang.org/show_bug.cgi?id=19274
Oct 25 2018
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Thursday, 25 October 2018 at 19:56:18 UTC, Stanislav Blinov 
wrote:
 The current behavior of the compiler is quite the opposite of 
 those "same as" above.
Yeah, I guess I am maybe selectively reading the spec in light of the implementation... but I think the examples are just sloppy. Or maybe we have a buggy implementation but idk which is more useful.
Oct 25 2018
parent Eduard Staniloiu <edi33416 gmail.com> writes:
On Thursday, 25 October 2018 at 21:00:46 UTC, Adam D. Ruppe wrote:
 On Thursday, 25 October 2018 at 19:56:18 UTC, Stanislav Blinov 
 wrote:
 The current behavior of the compiler is quite the opposite of 
 those "same as" above.
Yeah, I guess I am maybe selectively reading the spec in light of the implementation... but I think the examples are just sloppy. Or maybe we have a buggy implementation but idk which is more useful.
I still hold my believe that if opAssign is defined then that should be used. In my humble opinion, the current way might/could be faster, but it's not correct.
Oct 30 2018