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
