March 11, 2021
On 11/3/21 10:01, Walter Bright wrote:
> On 3/11/2021 12:42 AM, Walter Bright wrote:
>> Constructing from an rvalue essentially is move construction.
> 
> I forgot to mention that the new semantics only apply to EMO objects, which require both a move constructor and a move assignment operator. The move constructor is new syntax. Therefore, it shouldn't break existing code.

But we also read in the DIP, as it has already been mentioned:

> If a Move Constructor is not defined for a struct that has a Move Assignment Operator, a default Move Constructor is defined and implemented as a move for each of its fields, in lexical order.

It's not clear if a struct would be considered an EMO if either the Move Assignment Operator or the Move Constructor are defined by default.

If that's the case, any struct with an identity assignment operator would be silently "upgraded" to EMO, thus potentially breaking existing code: the original identity assignment might even throw, which according to the DIP will no longer be allowed.
March 11, 2021
On Thursday, 11 March 2021 at 04:17:48 UTC, tsbockman wrote:
> Also, doesn't this mean that every move of an object with a destructor requires a copy?

No, only if said object was initialized first.

> Does the copy constructor get  called in such cases?

Why? That would not allow for the removal of the destruction, in fact now you'd have 2 object to destroy instead of 1.

> If so, what benefit do moves have over copies?

They do not create a new object to destroy.

March 11, 2021
On Thursday, 11 March 2021 at 08:42:32 UTC, Walter Bright wrote:
> Constructing from an rvalue essentially is move construction.

No, you need to destroy that rvalue.

You'll note that C++ move constructor still require the old object to be in a "null" state so that its destruction can effectively be a noop.

When a constructor is involved, you create a new object, and need to do something with the existing ones.

This is why I was suggesting to recycle the postblit instead as a "postmove" constructor.
March 11, 2021
On 05.03.21 13:19, Mike Parker wrote:
> This is the discussion thread for the first round of Community Review of DIP 1040, "Copying, Moving, and Forwarding":
> 
> https://github.com/dlang/DIPs/blob/a9c553b0dbab1c2983a801b5e89b51c5c33d5180/DIPs/DIP1040.md 
> 
> 
> ...

I haven't had time yet to contribute a thorough review, but one thing that stands out to me is that there seems to be no discussion of interaction with `const`/`immutable`, etc. Given that that's a source of unsoundness for postblit, maybe it deserves some explicit consideration?
March 11, 2021
On Friday, 5 March 2021 at 12:19:54 UTC, Mike Parker wrote:
> This is the discussion thread for the first round of Community Review of DIP 1040, "Copying, Moving, and Forwarding":
>
> [...]

How does it handle move constructors when a class have struct type variable that is being alias this?

-Alex
March 11, 2021
On 3/11/2021 2:19 AM, deadalnix wrote:
> On Thursday, 11 March 2021 at 08:42:32 UTC, Walter Bright wrote:
>> Constructing from an rvalue essentially is move construction.
> 
> No, you need to destroy that rvalue.


Why? There can be no other uses of the rvalue, so why not simply move it?
March 11, 2021
On Thursday, 11 March 2021 at 08:36:18 UTC, Walter Bright wrote:
> On 3/10/2021 8:17 PM, tsbockman wrote:
>> Wouldn't it make more sense to just skip the move operation entirely when the source and destination are the same? Or are there circumstances in which that cannot be determined, even at runtime?
>
> The idea is that the move assignment operation takes care of this, and makes it "as if" the move was done, then the destruction.

Is it the responsibility of the explicit body of the custom move assignment operator to do the destruction, or the responsibility of the compiler to insert a call to the destructor somewhere? If the compiler inserts the call, is this done inside the move assignment operator, or at each call site?

My concern is how to make code like this work correctly:

/////////////////////////////////////////////////
module app;

ptrdiff_t netAllocCount = 0;
void* malloc(size_t size) @system nothrow @nogc {
    import core.stdc.stdlib : malloc;
    void* ptr = malloc(size);
    netAllocCount += int(ptr !is null);
    return ptr;
}
void free(void* ptr) @system nothrow @nogc {
    import core.stdc.stdlib : free;
    netAllocCount -= int(ptr !is null);
    free(ptr);
}

struct S {
private:
    struct Data {
        uint data;
        int refCount = 1;
    }

    Data* ptr;
    Data internal;

public:
    bool isUnique() const pure @safe nothrow @nogc {
        return (ptr is &internal); }
    ref inout(uint) data() return inout pure @safe nothrow @nogc {
        return ptr.data; }

    // normal construction and assignment
    this(bool unique) @trusted nothrow @nogc {
        construct(unique); }
    private void construct(bool unique) @trusted nothrow @nogc {
        pragma(inline, false); // Don't let inlining make things work by accident.
        if(unique) {
            ptr = &internal;
        	internal = Data.init;
        } else {
            ptr = cast(Data*) malloc(size_t.sizeof * 2);
            (*ptr) = Data.init;
        }
    }
    ref typeof(this) opAssign(bool unique) return @safe nothrow @nogc {
        destruct(this);
        construct(unique);
        return this;
    }

    // copy construction
    @disable this(this);
    this(ref typeof(this) source) pure @trusted nothrow @nogc {
        if(source.isUnique) {
            ptr = &internal;
            internal = source.internal;
        } else {
            ptr = source.ptr;
            ptr.refCount += 1;
        }
    }

    // move construction and assignment (using the DIP's misleading syntax):
    this(S source) pure @trusted nothrow @nogc {
        if(source.isUnique) { // This works, because source is really by reference.
            ptr = &internal;
            internal = source.internal;
        } else
            ptr = source.ptr;
    }
    void opAssign(S source) @trusted nothrow @nogc {
        // What goes here, and what is the full lowering of a move assignment call?
    }

    // destructor
    ~this() @safe nothrow @nogc {
        destruct(this); }
    private void destruct(ref S s) @trusted nothrow @nogc {
        pragma(inline, false); // Don't let inlining make things work by accident.
        if(!s.isUnique) {
            s.ptr.refCount -= 1;
            if(s.ptr.refCount <= 0)
                free(s.ptr);
        }
        s.ptr = null;
    }
}

void main() @safe {
    import std.stdio : writeln;
    {
        S a = true, b = false, c = b;
        a.data = 1;
        b.data = 2;
        a = b;
        c.data = 3;
        assert(a.data == 3);
    }
    assert(netAllocCount == 0);
}
/////////////////////////////////////////////////

Syntax aside, I think the way it should be done is this:

// Lower the `a = b;` line in main, above, to this:
if(&b !is &a) { // Moving a variable into itself is a no-op.
    destroy(a);
    /* Just call the move constructor; there is no point
    to the move assignment operator as its body would be identical: */
    a.__ctor(b);
}

I've tested this algorithm, and it seems to work as intended (although of course it has to be expressed somewhat differently without the DIP). But, that's clearly not what you have in mind. I just can't figure out what you think the alternative is:

// Maybe this is your proposed lowering for the `a = b;` line in main, above?
{
    S oldDest = a; // You all say no copy constructor call is necessary here, but...
    a.opAssign(b); // (Body is the same as that of the move constructor.)
    // ...the implicit destroy(oldDest); at scope exit crashes if oldDest was only blit.
}

You can put the lowering logic inside of the move opAssign instead, but it doesn't change the fundamental problem. If no actual general-purpose lowering is possible that accurately reflects the intended semantics of, "After the move is complete, the destructor is called on the original contents of the constructed object," then the DIP's description of the semantics is simply incorrect and should be replaced with something more rigorous.
March 11, 2021
On Thursday, 11 March 2021 at 10:09:25 UTC, deadalnix wrote:
> On Thursday, 11 March 2021 at 04:17:48 UTC, tsbockman wrote:
>> Also, doesn't this mean that every move of an object with a destructor requires a copy?
>
> No, only if said object was initialized first.
>
>> Does the copy constructor get  called in such cases?
>
> Why? That would not allow for the removal of the destruction, in fact now you'd have 2 object to destroy instead of 1.
>
>> If so, what benefit do moves have over copies?
>
> They do not create a new object to destroy.

I have written an example to clarify what I'm asking about:
    https://forum.dlang.org/post/igagderdongxdhvfjihj@forum.dlang.org
March 11, 2021
On Thursday, 11 March 2021 at 21:48:44 UTC, tsbockman wrote:
> I have written an example to clarify what I'm asking about:
>     https://forum.dlang.org/post/igagderdongxdhvfjihj@forum.dlang.org

Here's a version that can be compiled and successfully run with DMD today, without the DIP, in case anyone else wants to play around with it:
    https://gist.github.com/run-dlang/247bea2c2815e794a1a52012bc70ef9f

Set `enum useDIPLowering = true;` at the top to see the problem I'm poking at.
March 11, 2021
On 3/11/2021 10:43 AM, Timon Gehr wrote:
> I haven't had time yet to contribute a thorough review, but one thing that stands out to me is that there seems to be no discussion of interaction with `const`/`immutable`, etc. Given that that's a source of unsoundness for postblit, maybe it deserves some explicit consideration?

All the problems with const/immutable revolved around postblit. That's why this proposal has zero reliance on postblit.