Discussion Thread: DIP 1040--Copying, Moving, and Forwarding--Community Review Round 1

[tsbockman]

On Friday, 12 March 2021 at 01:43:01 UTC, tsbockman wrote:
> On Friday, 12 March 2021 at 01:40:22 UTC, tsbockman wrote:
>> I think I finally figure out how to make some sense out of the 
>> DIP's description. However, the lowering cannot be expressed 
>> clearly with the DIP's syntax, so I will use an alternative 
>> notation:
>>
>> void moveConstruct(ref S source) nothrow @nogc {
>>     if(source.isUnique) {
>>         ptr = &internal;
>>         internal = source.internal;
>>     } else
>>         ptr = source.ptr;
>> }
>> void moveAssign(ref S source) @trusted nothrow @nogc {
>>     S oldDest = void;
>>     oldDest.moveConstruct(this); // Move the old value to a 
>> temporary.
>>     moveConstruct(source);
>>     // Implicitly destroy the old value.
>> }
>>
>> Is this correct?

Nope, still not correct. I had a bug in the surrounding context, 
and wasn't testing enough things. Here's another attempt that 
passes a more thorough test:
     
https://gist.github.com/run-dlang/b789714c01905f091a44ee2666276433

The important bit:

/* move construction and assignment (these must be called manually
and do not use the DIP syntax, since it's not implemented yet): */
void moveConstruct(ref S source) @system nothrow @nogc {
     // @system since this must not be called by itself on an 
already-initialized object.
     if(source.isUnique) {
         ptr = &internal;
         internal = source.internal;
     } else
         ptr = source.ptr;
     source.ptr = null;
}
void moveAssign(ref S source) @trusted nothrow @nogc {
     static if(useDIPLowering) {
         // destroy after (the DIP's proposal):
         S newVal = void;
         newVal.moveConstruct(source);
         S oldVal = void;
         oldVal.moveConstruct(this);
         moveConstruct(newVal);
         // Implicitly destruct(oldVal).
     } else {
         // conditionally move and destroy before (my proposal):
         if(&source !is &this) {
             destruct(this);
             moveConstruct(source);
         }
     }
}

Key changes: the move constructor must put the source into a 
state where the destructor is a no-op, and for the move 
assignment operation to destroy the old value *after* the move, 
as required by the DIP, TWO extra moves are required. That is a 
lot of extra work and confusion just to avoid explicitly checking 
if the source and destination are the same. This seems especially 
silly given that the optimizer can probably detect the 
move-to-self case at compile time in many cases, and eliminate 
either the test, or the entire move during compilation.

Is there some other motivation for destroying after, rather than 
before, besides the self-move case?