On Thursday, 12 July 2018 at 10:38:52 UTC, Shachar Shemesh wrote:
> 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.

That's already thought through? Great, no need to "lockstep" the DIPs then and one curve less to worry about.

On Thursday, 12 July 2018 at 10:22:33 UTC, Shachar Shemesh wrote:
> 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.

First off: I am trying to wear a strict language lawyer hat. D spec is already very much ill specced which is _very_ problematic for language and compiler development. I am not attacking the proposal in order to kill it. I am merely commenting on points that I feel should be improved.

My point was to remove things like "compiler" and "emitted code" from the DIP / spec. In this case, the simple rewording can be: "When moving a struct's instance, an implicit call to __move_post_blt is inserted with both new and old instances' addresses as arguments."


>> 
>> ### "__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.

Being explicit about generated machine is not good in a language spec. Being explicit about the semantic meaning of something _is_.  ;-)
"compatible" is very vague. It may mean that just the function signature should match.
"has the same semantic effect" would be a much better description of what you want.

>> 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).

OK, so make _that_ explicit. I think there is value in prescribing the precise order of moves (like for construction of members), such that reasoning about code becomes easier.
If you want the same semantic effect (as I wrote above), then the text should say that the ordering is relaxed.

>> Why "SHOULD" and not "MUST"?
>
> Precisely for the reason you stated above. So that if you want to do something else, you may.

Why is that freedom needed? The freedom is already provided by user-defined opPostMove? I think the implicit call to __move_post_blt is a MUST, like calls to dtors.


>> 
>> ### "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.

Please don't ;-)

> 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.

Great.

>> 
>> 
>> ### 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?

I think the language spec doesn't say when a "move" is performed? So I don't know when exactly the opPostMove is called. Things that come to mind:
* exiting from struct ctor
* std.algorithm.mutation.move
Or is it enough to define what a "move" is ? (didn't check but I guess the DIP already explains that)
(D's "move" is different from C++'s right? D's move after exiting a struct's constructor doesn't lead to a destructor call, but D's std.algorithm.mutation.move _does_ call the destructor of the moved source object.)


I now realize that the DIP is a mix between language semantic changes (opPostMove) and implementation suggestions/details ("__move_post_blt"). I think it would have been clearer to split the two in the DIP (it's valuable to have implementation suggestions in addition to spec changes), but by now that's too late. Part of my comments stem from this mixed treatment. After this DIP is accepted, I think the language spec should not state anything related to __move_post_blt, because that's just an implementation detail. If that's the case, then most of my comments are no longer important because they comment on suggestions instead of spec items.

Cheers,
  Johan

On 14/07/18 15:56, Johan Engelen wrote:
> First off: I am trying to wear a strict language lawyer hat. D spec is already very much ill specced which is _very_ problematic for language and compiler development. I am not attacking the proposal in order to kill it. I am merely commenting on points that I feel should be improved.

Wouldn't these comments be better suited for the language spec's PR, then?

I'm asking seriously. There is nothing in this DIP (or just about any other one) that can go, unmodified, into the specs. If that's the DIP's purpose, then so be it. If not, then I don't see the value.

> 
> OK, so make _that_ explicit. I think there is value in prescribing the precise order of moves (like for construction of members), such that reasoning about code becomes easier.

I disagree.

The member variables at the point of the move are all:
1. Fully initialized.
2. Either move agnostic or know how to handle their own move
3. Oblivious to one another

Obviously, you might wish to spite me by creating a case where any one of the above is not true, but the compiler has every right to assume all three points above are true. In your hypothetical spite case, your best course of action is to leave the members with no opPostMove at all, and handle their move in the containing struct's opPostMove, in which case you are fully defined.

Assuming all three are correct, the order in which they are moved makes no difference.

> If you want the same semantic effect (as I wrote above), then the text should say that the ordering is relaxed.

I have no objection to explicitly stating that exact move order of members is undefined.

> 
>>> Why "SHOULD" and not "MUST"?
>>
>> Precisely for the reason you stated above. So that if you want to do something else, you may.
> 
> Why is that freedom needed?

Because compiler implementers might have a good reason to do something besides this. For example, a compiler writer might choose to place the actual moving code inside __move_post_blt, and I don't think we should stop her.

> The freedom is already provided by user-defined opPostMove?

Different audiences. opPostMove serves the D programmer, __move_post_blt serves the compiler.

> 
> I think the language spec doesn't say when a "move" is performed?

I think Walter sees that as an advantage, but I'm not sure.

Either way, the current language spec says structs must have semantics that remain correct even if the struct suddenly changes the memory address it resides in. The specs + DIP 1014 say that the above is true, or the struct must supply an opPostMove that fixes the semantics post-move.

In both cases, *when* the move takes place is irrelevant.

> Or is it enough to define what a "move" is ? (didn't check but I guess the DIP already explains that)
Only implicitly.

> (D's "move" is different from C++'s right?
Yes.

> D's move after exiting a struct's constructor doesn't lead to a destructor call, but D's std.algorithm.mutation.move _does_ call the destructor of the moved source object.)

Depends on which version of move you're referring to.

For example, moveEmplace does not.

I think the correct way to phrase this is to say that D's move *never* calls a destructor, but if the move's destination had a valid object in it, that one gets destructed.

In a way, C++'s move is the same, except the actual moving of the data from the source location to the destination one is up to the programmer, and accordingly, so is destructing. Since, logically, a C++ move operator always copies, it also has to destruct the source.

Technically, however, it doesn't always. A move assignment operator typically just swaps the content of the structs (i.e. - moves the source to the destination and the destination to the source), and lets the usual rvalue elimination code destruct it.

Shachar

On Thursday, 12 July 2018 at 10:24:40 UTC, Shachar Shemesh wrote:
> 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

A postblit on a class issues a compiler error. And an identity opAssign on a class also issues a compiler error. So I'm not sure how this would be different. And the page In https://dlang.org/spec/operatoroverloading.html also explicitly mentions differences between ops on classes or structs.

Cheers,
- Ali

On 17/07/18 16:29, aliak00 wrote:
> 
> A postblit on a class issues a compiler error. And an identity opAssign on a class also issues a compiler error. So I'm not sure how this would be different. And the page In https://dlang.org/spec/operatoroverloading.html also explicitly mentions differences between ops on classes or structs.

That's actually a good argument. It should definitely be handled the same way the corresponding opAssign is handled.

Thanks,
Shachar