digitalmars.D - DIP 1014--Hooking D's struct move semantics--Final Review
- Mike Parker (12/12) Jun 27 2018 DIP 1014, "Hooking D's struct move semantics", is now ready for
- Mike Parker (3/4) Jun 27 2018 For those of you using the NNTP or mailing list interfaces, this
- aliak (13/18) Jun 29 2018 Alo!
- Shachar Shemesh (7/28) Jul 12 2018 This is a left over from a previous draft, where the operator was called...
- aliak00 (8/37) Jul 17 2018 A postblit on a class issues a compiler error. And an identity
- Shachar Shemesh (5/11) Jul 17 2018 That's actually a good argument. It should definitely be handled the
- Dukc (11/13) Jul 05 2018 Structs are a low level feature that should be able to be used in
- Dukc (6/7) Jul 11 2018 However, we need to consider how well this interacts with
- Johan Engelen (29/31) Jul 11 2018 after quick read:
- Shachar Shemesh (28/63) Jul 12 2018 I don't draw the same conclusions from the text as you.
- Johan Engelen (52/114) Jul 14 2018 First off: I am trying to wear a strict language lawyer hat. D
- Shachar Shemesh (48/76) Jul 14 2018 Wouldn't these comments be better suited for the language spec's PR, the...
- Jonathan M Davis (49/62) Jul 11 2018 Looks pretty good overall, though the name of the __move_post_blt arguab...
- Shachar Shemesh (44/69) Jul 12 2018 On 12/07/18 04:17, Jonathan M Davis wrote:> I'm also> not sure if going
- Dukc (3/11) Jul 13 2018 That's already thought through? Great, no need to "lockstep" the
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.
Jun 27 2018
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!
Jun 27 2018
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: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 - AliThanks in advance for your participation.For those of you using the NNTP or mailing list interfaces, this is the thread to respond in. Thanks!
Jun 29 2018
On 29/06/18 15:35, aliak wrote:On Wednesday, 27 June 2018 at 07:24:05 UTC, Mike Parker wrote:This is a left over from a previous draft, where the operator was called "opMove". It should be removed.On Wednesday, 27 June 2018 at 07:13:14 UTC, Mike Parker wrote: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" ?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!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
Jul 12 2018
On Thursday, 12 July 2018 at 10:24:40 UTC, Shachar Shemesh wrote:On 29/06/18 15:35, aliak 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. Cheers, - AliOn Wednesday, 27 June 2018 at 07:24:05 UTC, Mike Parker wrote:This is a left over from a previous draft, where the operator was called "opMove". It should be removed.On Wednesday, 27 June 2018 at 07:13:14 UTC, Mike Parker wrote: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" ?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!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
Jul 17 2018
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
Jul 17 2018
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 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.
Jul 05 2018
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
Jul 11 2018
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) 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. 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"? 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. `opPostMove` will be called (needed for user-facing documentation of the function). -Johan
Jul 11 2018
On 11/07/18 20:04, Johan Engelen wrote:On Wednesday, 27 June 2018 at 07:13:14 UTC, Mike Parker wrote: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.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) 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).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.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.`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
Jul 12 2018
On Thursday, 12 July 2018 at 10:22:33 UTC, Shachar Shemesh wrote:On 11/07/18 20:04, 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. 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."On Wednesday, 27 June 2018 at 07:13:14 UTC, Mike Parker wrote: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.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) 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).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.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.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.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 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.Why "SHOULD" and not "MUST"?Precisely for the reason you stated above. So that if you want to do something else, you may.Please don't ;-)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.Great.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`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?
Jul 14 2018
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.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.Why is that freedom needed?Why "SHOULD" and not "MUST"?Precisely for the reason you stated above. So that if you want to do something else, you may.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
Jul 14 2018
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
Jul 11 2018
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
Jul 12 2018
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.
Jul 13 2018