DIP 1014, "Hooking D's struct move semantics", is now ready for final review. This is a last chance for community feedback before the DIP is handed off to Walter and Andrei for the Formal Assessment. Please read the procedures document for details on what is expected in this review stage:

https://github.com/dlang/DIPs/blob/master/PROCEDURE.md#final-review

The current revision of the DIP for this review is located here:

https://github.com/dlang/DIPs/blob/01af4bb1e81993066bd1e7f77263002d3883fabf/DIPs/DIP1014.md

In it you'll find a link to and summary of the previous review round. This round of review will continue until 11:59 pm ET on July 11th unless I call it off before then.

Thanks in advance for your participation.

On Wednesday, 27 June 2018 at 07:13:14 UTC, Mike Parker wrote:
>
> Thanks in advance for your participation.

For those of you using the NNTP or mailing list interfaces, this is the thread to respond in. Thanks!

On Wednesday, 27 June 2018 at 07:24:05 UTC, Mike Parker wrote:
> On Wednesday, 27 June 2018 at 07:13:14 UTC, Mike Parker wrote:
>>
>> Thanks in advance for your participation.
>
> For those of you using the NNTP or mailing list interfaces, this is the thread to respond in. Thanks!

Alo!

This is great!

Just a clarification about the last paragraph phrasing

The last line: "We can further reduce this problem by calling the function opPostMove." seemed to imply that an alternate name to opPostMove would be mentioned, but am I correct in understanding that it is just saying that "naming this second function as op* will keep code breakage to a minimum" ?

Also, what should happen if someone defines an opPostMove for a class. Compile error or? Should something about that be mentioned?

Cheers
- Ali

On Wednesday, 27 June 2018 at 07:13:14 UTC, Mike Parker wrote:
> DIP 1014, "Hooking D's struct move semantics", is now ready for final review.

Structs are a low level feature that should be able to be used in any way programmer sees fit. This is just what is wrong with C# structs: In principle they're the same as D structs but disallow many things for no obvious reason, thus limiting their usability. See my question and it's answers at Stack overflow to see what I mean: https://stackoverflow.com/questions/51098690/assigning-value-to-member-of-nullable-struct-in-c-sharp

This is a feature that is likely to be useful for low level programming, but is zero cost for those who don't need it. The DIP looks well written. I'm in favour of it.

On Thursday, 5 July 2018 at 10:27:52 UTC, Dukc wrote:
> The DIP looks well written. I'm in favour of it.

However, we need to consider how well this interacts with Razvan's DIP [1]. We should consider this together with it, so the implementations do not end up messing each other.

1: https://forum.dlang.org/thread/hhdtliynfwckcgaflddi@forum.dlang.org

On Wednesday, 27 June 2018 at 07:13:14 UTC, Mike Parker wrote:
> DIP 1014, "Hooking D's struct move semantics", is now ready for final review.

after quick read:

(would be much easier to do inline commenting, but it appears that's not supported)

### Section "Code emitted by the compiler on move"
Dangerous to talk about what "code is emitted" by the compiler. I think this DIP doesn't need that, and semantics is enough. "the compiler MUST call " should be reworded, because an _actual_ call to the function should not be mandatory, because it would limit the optimizer in eliding it or inlining it (note that it will be hard to _prevent_ the optimizer from eliding/inlining the call as currently specified by the DIP). So this should be reworded such that the semantic effect is as if the function is called. Also unspecified _when_ the function needs to be called.


### "__move_post_blt SHOULD be defined in a manner that is compatible"
What does "compatible" mean? Some things should be made more explicit, such as the order of calls for compound structs.
Why "SHOULD" and not "MUST"?


### "This MUST return true iff a struct or any of its members have an opPostMove defined."
Doesn't cover struct A containing struct B containing struct C with opPostMove. Reword e.g. into: "hasElaborateMove!S MUST return true iff `S` has an `opPostMove` defined or if hasElaborateMove!X is true for any member of S of type X.


### What is lacking is a clear list of exactly in which cases `opPostMove` will be called (needed for user-facing documentation of the function).

-Johan

On Wednesday, 27 June 2018 01:13:14 MDT Mike Parker via Digitalmars-d wrote:
> DIP 1014, "Hooking D's struct move semantics", is now ready for final review. This is a last chance for community feedback before the DIP is handed off to Walter and Andrei for the Formal Assessment. Please read the procedures document for details on what is expected in this review stage:
>
> https://github.com/dlang/DIPs/blob/master/PROCEDURE.md#final-review
>
> The current revision of the DIP for this review is located here:
>
> https://github.com/dlang/DIPs/blob/01af4bb1e81993066bd1e7f77263002d3883fab f/DIPs/DIP1014.md
>
> In it you'll find a link to and summary of the previous review round. This round of review will continue until 11:59 pm ET on July 11th unless I call it off before then.
>
> Thanks in advance for your participation.


Looks pretty good overall, though the name of the __move_post_blt arguably should be different if we're going to move to copy constructors. I'm also not sure if going to copy constructors means that we should do something different with this. It don't think that it's affected by it, but I could be missing something.

However, I don't agree that opPostMove needs to be nothrow on the basis that it might be confusing if it's not. Based on that argument, postblit constructors should be required to be nothrow, and they don't have to be nothrow. Copying and moving frequently look pretty much the same to the programmer and may even depend on how the compiler is able to optimize the code (e.g. it figures out that it can use a move instead of a copy). So, while it probably is generally better for opPostMove to be nothrow, it doesn't seem to me like it really should be required that it be nothrow. For better or worse, we don't even require that destructors be nothrow. So, it doesn't really fit for opPostMove to have to be nothrow. The motivation for it is not at all technical in nature and the social aspect of it is already contradicted by similar features which are allowed to throw.

Also, as soon as we discuss overriding what happens when an object is moved, that opens up the question of whether it should be allowed to be @disabled. What happens when someone does

@disable opPostMove();

For better or worse, @disable works on any function in D (even if it's kind of pointless for functions that aren't constructors or operators). So, it's going to need to be handled even if it's just making it an error to @disable it. However, I would argue that if we're going to take the step of allowing the programmer to hook into the move process to do custom stuff that we should also allow it to be outright disabled - the use case that comes to mind at the moment being stuff like mutexes where the object can't safely be moved, because it's not only not thread-safe, but it's probably not possible with the mutex's C API, since you handed a pointer off to it and have no control over what it does with it.

Of course, that also opens the question of what should happen with a shared opPostMove, since presumably a mutex would be in a shared object, and that would then require that we finish working out shared - though I'd argue that we would have to disallow moves on a shared object in order to be thread-safe anyway. So, maybe the mutex use case doesn't ultimately matter, since we're likely going to have to disable moves for shared anyway, but shared and mutexes aside, any use case where you hand the pointer off to something outside the object won't work with opPostMove, whereas it could be made to work if we had opPostMove and were then allowed to @disable it.

By no means do I think that this DIP should be contingent on anything to do with disallowing moving, but I do think that it if we're going to allow mucking around with moves like this rather than simply leaving it up to the compiler that we should seriously consider allowing it to be disabled - especially when opPostMove gives us a really obvious syntax for it, and while some of the motivations for this DIP are solved by it, others would really require disallowing moves.

- Jonathan M Davis

On 11/07/18 20:04, Johan Engelen wrote:
> On Wednesday, 27 June 2018 at 07:13:14 UTC, Mike Parker wrote:
>> DIP 1014, "Hooking D's struct move semantics", is now ready for final review.
> 
> after quick read:
> 
> (would be much easier to do inline commenting, but it appears that's not supported)
> 
> ### Section "Code emitted by the compiler on move"
> Dangerous to talk about what "code is emitted" by the compiler. I think this DIP doesn't need that, and semantics is enough. "the compiler MUST call " should be reworded, because an _actual_ call to the function should not be mandatory, because it would limit the optimizer in eliding it or inlining it (note that it will be hard to _prevent_ the optimizer from eliding/inlining the call as currently specified by the DIP).

I don't draw the same conclusions from the text as you.

I'm perfectly fine with specifying that nothing here is mandatory if the compiler ensures that *the end effect* is as if it happened.

AFAIK, C++ has a standing order to that effect, and it greatly simplifies documenting what you want to do.

> 
> ### "__move_post_blt SHOULD be defined in a manner that is compatible"
> What does "compatible" mean?

"Has the same effect as".

It's a little as if you're complaining about something not being explicit in one section, and again about that same thing being explicit in the next. Precisely why such standing order would be a good idea.

> Some things should be made more explicit, such as the order of calls for compound structs.

I don't think it makes sense to specify the order, except to say that member's opPostMove must be called before the instance's opPostMove (which the code already says).

> Why "SHOULD" and not "MUST"?

Precisely for the reason you stated above. So that if you want to do something else, you may.

> 
> 
> ### "This MUST return true iff a struct or any of its members have an opPostMove defined."
> Doesn't cover struct A containing struct B containing struct C with opPostMove. Reword e.g. into: "hasElaborateMove!S MUST return true iff `S` has an `opPostMove` defined or if hasElaborateMove!X is true for any member of S of type X.

Yes, I'm sorry. I worded the spec for humans.

I can suggest a recursive definition:

hasElaborateMove for a struct MUST return true iff at least one of the following is true:
* The struct has opPostMove defined
* hasElaborateMove returns true for at least one of the struct's members.

I'm fine with this definition, and it resolves your concern, but I think it is less readable.

You are free to suggest a better way of phrasing this.

> 
> 
> ### What is lacking is a clear list of exactly in which cases `opPostMove` will be called (needed for user-facing documentation of the function).

I don't think I understand this point. Can you suggest what that list might contain?

Thank you,
Shachar

On 29/06/18 15:35, aliak wrote:
> On Wednesday, 27 June 2018 at 07:24:05 UTC, Mike Parker wrote:
>> On Wednesday, 27 June 2018 at 07:13:14 UTC, Mike Parker wrote:
>>>
>>> Thanks in advance for your participation.
>>
>> For those of you using the NNTP or mailing list interfaces, this is the thread to respond in. Thanks!
> 
> Alo!
> 
> This is great!
> 
> Just a clarification about the last paragraph phrasing
> 
> The last line: "We can further reduce this problem by calling the function opPostMove." seemed to imply that an alternate name to opPostMove would be mentioned, but am I correct in understanding that it is just saying that "naming this second function as op* will keep code breakage to a minimum" ?

This is a left over from a previous draft, where the operator was called "opMove". It should be removed.

> Also, what should happen if someone defines an opPostMove for a class. Compile error or? Should something about that be mentioned?

I think nothing should happen. The function would be ignored, just like it is today. I am open to hear other ideas, however.

I'm not sure whether it should be explicitly mentioned or not.

Shachar

On 12/07/18 04:17, Jonathan M Davis wrote:> I'm also> not sure if going to copy constructors means that we should do something> different with this. It don't think that it's affected by it, but I could be> missing something.

I actually had that very same concern myself. Andrei does not seem to share it (I talked to him about it during DConf, and he even mentioned it in his talk). He seems to think the two are completely independent.

Like I said, I'm not sure I completely agree. Some of the problems postblit have will also affect us here (specifically, what happens if you want to override a member's behavior, specifically when @disabled.

What tipped the scale for me was this: This DIP is relatively simple. It requires a few changes to all three (compiler, run time and phobos), but small changes to all three. Best of all, this change does not introduce what Andrei called "magic". The changes are easily lowered to existing D code.

Switching to a move-constructor type implementation will make all of those advantages disappear in a puff of bits.

> 
> However, I don't agree that opPostMove needs to be nothrow on the basis that
> it might be confusing if it's not.

The problem here is that D moves *everywhere*, and it is often quite impossible to make it not move (believe me, I've tried).

With that said, downgrading this to a very hearty recommendation instead of a requirement is fine by me.

> Also, as soon as we discuss overriding what happens when an object is moved,
> that opens up the question of whether it should be allowed to be @disabled.
> What happens when someone does
> 
> @disable opPostMove();

What happens today is that if you actually try to move the struct, you will get a compilation error. IMHO, this is the behavior we actually want to keep.

I can actually see some uses for such a step - if you simply cannot have that struct move, a compilation error is preferable to things breaking.

With that said, the moves are so integral to D's code generation that a struct with opPostMove @disabled would not be very useful. I am of the opinion that the programmer can make up their own mind on what to do about it.

> However, I would argue that if we're going to take the step of allowing
> the programmer to hook into the move process to do custom stuff that we
> should also allow it to be outright disabled - the use case that comes to
> mind at the moment being stuff like mutexes where the object can't safely be
> moved, because it's not only not thread-safe, but it's probably not possible
> with the mutex's C API, since you handed a pointer off to it and have no
> control over what it does with it.

Like I said above, I completely agree.
> 
> Of course, that also opens the question of what should happen with a shared
> opPostMove, since presumably a mutex would be in a shared object, and that
> would then require that we finish working out shared - though I'd argue that
> we would have to disallow moves on a shared object in order to be
> thread-safe anyway.

That one is actually handled by the DIP.

opPostMove is called by the compiler when it has just moved the object. This means that, at least as far as the compiler is concerned, the object has no external references to it (or it couldn't move it).

If a struct's pointer is shared, and there are external pointers to update, it is up to the programmer to decide how to handle it (and, yes, @disable might be the only safe option).

> By no means do I think that this DIP should be contingent on anything to do
> with disallowing moving, but I do think that it if we're going to allow
> mucking around with moves like this rather than simply leaving it up to the
> compiler that we should seriously consider allowing it to be disabled -

I disagree. I think we should allow it to be disabled whether this DIP goes in or not. On the contrary: this DIP would allow cases to be handled where today they are simply not safe, no matter what you do. In other words, the cases where you'd want to disable move are only reduced by adopting this DIP.

Shachar