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