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

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

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.

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

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

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.

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

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.

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.