On Thursday, 18 March 2021 at 10:01:12 UTC, Walter Bright wrote:
> On 3/16/2021 11:11 AM, tsbockman wrote:
>> You just have different results in mind from what your wrote in the DIP.
>
> The desired result is clear - two objects exist before the move assignment, and one afterwards. If both objects are the same instance, then it should be a no-op.
>
> The wording could be improved - want to make a stab at it?

Sure.

Old wording:
> A Move Assignment Operator is a struct member assignment operator
> that moves, rather than copies, the argument corresponding to its
> first parameter into the constructed object. After the move is
> complete, the destructor is called on the original contents of
> the constructed object. The argument is invalid after this move,
> and is not destructed.

Proposed new wording:
> A Move Assignment Operator is a struct member assignment operator
> that moves, rather than copies, the source argument into the
> destination argument. The source argument corresponds to the first
> parameter, and the destination argument corresponds to the implicit
> `this` parameter.
>
> If both arguments refer to the same object, the Move Assignment
> Operator shall have no effect, and that object remains valid.
>
> Otherwise, the effect shall be to destroy the old contents of the
> destination and to move the contents of the source into the
> destination. The source is invalid after the Move Assignment
> Operator returns, and is not destroyed.
>
> The Move Assignment Operator implementation may perform the
> destruction and move in any order, however it must ensure that
> the source is not destroyed when the old contents of the
> destination are destroyed, even if the source is indirectly owned
> by the old contents of the destination.

The final paragraph is the tricky part: I thought about the "destroy after" thing some more, and realized that while it is neither necessary nor helpful when the source and destination refer to the exact same object, it *is* helpful when the old destination may *indirectly* own the source.

For example, what if there is a singly-linked list where each `next` link is a unique smart pointer? When removing the current `head` of the list, we may wish to move `head.next` into `head`. If `head` is destroyed first, it will wrongly destroy the rest of the list before the move is complete, including `head.next`.

However, requiring that the destruction of the old destination be performed after the move of source to destination doesn't fix this problem by itself; an additional step is required: set `source` to `null` as part of the move.

So, requiring a specific order is undesirable, because it doesn't fix the problem without also requiring the existence of a `null` state for every movable type. (My earlier code example uses a `null` state.)

Also, requiring a specific order is undesirable because it is not the only possible solution to the indirect ownership problem; for example, the move assignment operator could scan the old destination's indirections and just skip destroying any that would invalidate the source.

Thus, I propose we formally leave the details up to each move assignment operator's implementer, and limit the language spec to forbidding the problem itself, rather than require semantic equivalence to a specific solution. Types without problematic indirections can destroy before, types with problematic indirections and a `null` state can destroy after, and those rare types that don't fit into either category can do something more creative.

On Thursday, 18 March 2021 at 19:53:49 UTC, deadalnix wrote:
> On Wednesday, 17 March 2021 at 17:14:04 UTC, Q. Schroll wrote:
>> It might be a stupid question, but why have move assignment in the first place? In C++, there's the copy-and-swap idiom[1]. Maybe it's obvious why it does not apply in D, but if using a swap function makes implementing a copy assignment and move assignment trivial, why not requiring opSwap instead of opAssign for an elaborate move object?
>>
>
> This isn't a stupid question, this is THE question. It is easy to assume things are necessary because other went there and did it, but I find that questioning these assumptions is how the greatest design ideas came up.

My suggestion isn't really don't do ever implement opAssign (the copy one and the move one) manually, but rather: Leave it up to the compiler, except you know you really cannot use copy and swap.

> Doing this has major issue: it require all movable structs to have a null state (as in C++) or make other unsavory tradeofs (see https://forum.dlang.org/post/bkfqchwpnonngjrtybbe@forum.dlang.org for a more thorough explanation).
>
> Nevertheless, if the struct naturally has a null state, this is indeed a very good way to do it.

I was never sure if that was a good or bad decision D made, but doesn't D require every type to have a null value (.init)? Or do you mean something else?

On 3/18/2021 11:12 AM, Timon Gehr wrote:
> In condensed form, I think the main complaint is this. Let's start with a struct:
> 
> struct S{
>      T field0;
>      this(S r){
>          field0=move(r.field0);
>      }
> }
> 
> 
> Now someone adds a new field, but forgets to update the move constructor:
> 
> struct S{
>      T field0, field1;
>      this(S r){
>          field0=move(r.field0);
>      }
> }
> 
> field1 is now leaked: _Its destructor will never run_. And this can happen in @safe code. (Ignoring the issue that field1 of the moved object will be the init value.)

Thanks, I understand it now.

> This design is error-prone. postblit does not have this issue, because fields that are not explicitly referred to are moved correctly by default.
> 
> Hence the suggestion in my previous post to perhaps require all fields to be initialized in a move constructor and similar thoughts about opAssign.
> This mitigates the risk, but unfortunately it does not eliminate it. (It is furthermore possible that such an error would be annoying in some cases.)

Not sure how it doesn't eliminate the risk. The compiler already does some flow analysis in constructors (to implement restrictions about when constructor calls can occur).

On 3/18/2021 12:50 PM, deadalnix wrote:
> So, what do we want to do with move constructors anyways? Can't we just move the struct field by field recursively and be done with it? Yes, and I'd argue there is a problem if this isn't enough for 95% of the cases. Which leads to the two use cases I was able to identify:
>   - Non movable struct. It is important that such a struct doesn't move. For instance, when the struct is some sort of header or a larger data segment. Another example is a struct that represent some kind of guard that needs to see its construction/destruction done in order. This can be achieved by disabling the move constructor, whatever the move constructor is defined as. It is fairly easy to realize such use case, the move constructor simply needs to exist at all.

Ok.

>   - Movable struct that require some form of bookkeeping. For these cases, a postblit would work with one exception: interior pointers.

This bookkeeping was the motivation for #DIP1014:

"For example, D structs also may not use the constructor/destructor to register themselves with a global registry that keeps track of all instances in the system, e.g. via a linked list. This also severely limits the ability to store delegates that reference the struct instance from outside the struct."

https://github.com/dlang/DIPs/blob/master/DIPs/accepted/DIP1014.md

Marking the struct as immovable would resolve this problem, too.

> What I refers as interior pointers are struct containing pointer to elements which are within the struct itself. While this idiom exist, it is vanishingly rare and becoming rarer over time. The main reason for this is that memory has become slower, computation faster, and pointer larger, which in turn lead people to use "relative pointers", namely pointer defined as an offset from this. Unless is is expected that the struct may be more than 4GB in size - which is always the case, then it's all good. The extra addition required is well worth the memory saved (and increase hit rate in the cache that result from it). See https://www.youtube.com/watch?v=G3bpj-4tWVU for instance on how the swift runtime started using such techniques.

I didn't know this. This is good info.


> I'll be blunt, once these techniques are known, I've actually never encountered a case of interior pointers that would not be solved by disabling move altogether. I'm not pretending it doesn't exist, but I've never seen it. It simply doesn't make sense to sacrifice any of the above mentioned requirement for it, even it turns out this is really needed, because, well, this is the edge case of the edge case, and while enabling it might be an option, throwing away thing which are good in the general case for it just doesn't make sense.
> 
> I suspect that even then, making the struct unmovable and then definition custom method to move it manually would do the trick just fine. But just in case, here is what I propose: simply add an intrinsic, such as `void* __pre_move_address()` that can be called in the postblit, returning the address of the premove object. Any object using it would, of course, discard 4/ and not be usable as a value and instead always be passed by reference at the ABI level. This is the least constraining requirement to break, because it impact exclusively performances and never correctness like 1/ or 5/ would. However, considering it is possible to it custom once you disable move, I strongly suspect the bang is not worth the effort.

This is more or less what DIP1014 proposed.

One problem unaddressed is, for moveable structs, what if there *are* interior pointers, and they wind up pointing to a defunct object? The current compiler never moves structs, so this problem never occurs.

Some off the top of my head possibilities:

1. never move structs in @safe code (or at least disable moving structs that contain pointers). Unfortunately, ref counted objects must have a payload pointer in them.

2. add a runtime check on field pointer assignments

3. add an invariant() runtime check on field pointer values

4. insist that field pointers be marked with @system (there's another DIP for marking variables as @system)

P.S. the original reason for not allowing interior pointers is so a compacting garbage collector could be used.

On Sat, Mar 20, 2021 at 02:20:06AM -0700, Walter Bright via Digitalmars-d wrote:
> One problem unaddressed is, for moveable structs, what if there *are* interior pointers, and they wind up pointing to a defunct object? The current compiler never moves structs, so this problem never occurs.
[...]

Is there a typo somewhere here?  I'm *pretty* sure the current compiler *does* move structs in some cases, and that has caused problems in the past where structs that store pointers to themselves will end up with dangling pointers after, e.g., being returned from a function.


T

-- 
May you live all the days of your life. -- Jonathan Swift

On 3/20/2021 8:12 AM, H. S. Teoh wrote:
> Is there a typo somewhere here?  I'm *pretty* sure the current compiler
> *does* move structs in some cases, and that has caused problems in the
> past where structs that store pointers to themselves will end up with
> dangling pointers after, e.g., being returned from a function.

It doesn't. Even in the case of NRVO, it doesn't actually move them.

The main reason it doesn't is because data flow analysis is necessary to see if it can be moved (no pointers to the source object). In order to determine "last use", DFA is used.

On Sunday, 21 March 2021 at 00:18:43 UTC, Walter Bright wrote:
> On 3/20/2021 8:12 AM, H. S. Teoh wrote:
>> Is there a typo somewhere here?  I'm *pretty* sure the current compiler
>> *does* move structs in some cases, and that has caused problems in the
>> past where structs that store pointers to themselves will end up with
>> dangling pointers after, e.g., being returned from a function.
>
> It doesn't. Even in the case of NRVO, it doesn't actually move them.
>
> The main reason it doesn't is because data flow analysis is necessary to see if it can be moved (no pointers to the source object). In order to determine "last use", DFA is used.

idk for GDC, but LDC will, because LLVM will.

On Sunday, 21 March 2021 at 01:33:56 UTC, deadalnix wrote:
> On Sunday, 21 March 2021 at 00:18:43 UTC, Walter Bright wrote:
>> On 3/20/2021 8:12 AM, H. S. Teoh wrote:
>>> Is there a typo somewhere here?  I'm *pretty* sure the current compiler
>>> *does* move structs in some cases, and that has caused problems in the
>>> past where structs that store pointers to themselves will end up with
>>> dangling pointers after, e.g., being returned from a function.
>>
>> It doesn't. Even in the case of NRVO, it doesn't actually move them.
>>
>> The main reason it doesn't is because data flow analysis is necessary to see if it can be moved (no pointers to the source object). In order to determine "last use", DFA is used.
>
> idk for GDC, but LDC will, because LLVM will.

Godbolt example?

On Sunday, 21 March 2021 at 07:51:45 UTC, Max Haughton wrote:
> Godbolt example?

https://godbolt.org/z/eK7dYx

You'll note that there are no loads in the generated code.