March 20, 2021 Re: Feedback Thread: DIP 1040--Copying, Moving, and Forwarding--Community Review Round 1 | ||||
---|---|---|---|---|
| ||||
Posted in reply to Mike Parker | The Move Constructor is defined as: this(S s) { ... } // move constructor It's been pointed out (not sure which post) that this conflicts with an Rvalue Constructor, which has the same syntax: this(S s) { ... } // rvalue constructor Note that an Rvalue Constructor is *not* considered to be a Copy Constructor: this(ref S s) { ... } // copy constructor So the syntax is already in use. Need to resolve the conflict. --------- The Rvalue Constructor and Copy Constructor are currently not allowed to coexist: this(S); this(ref S); // error, can't have both an rvalue and copy constructor which suggests a straightforward resolution: it's an Rvalue Constructor if there's no Copy Constructor, and a Move Constructor if there is. While this sounds confusing, I suspect in practice the MC is simply a more advanced RC. |
March 20, 2021 Re: Feedback Thread: DIP 1040--Copying, Moving, and Forwarding--Community Review Round 1 | ||||
---|---|---|---|---|
| ||||
Posted in reply to deadalnix | On 3/18/2021 4:22 PM, deadalnix wrote:
> On Friday, 5 March 2021 at 12:20:36 UTC, Mike Parker wrote:
>> [...]
>
> Looking at how the discussion is going, this seems to me like this DIP should be split into two DIPs because it's really doing two things. One of these things seems to generally be agreed upon, while the other is quite intensively discussed.
>
> 1/ The transformation of copy/destruct couples into move if the object isn't used in between the copy and the destruction. This seems to be fairly uncontroversial and the proposal is sound.
>
> 2/ The addition of new move constructor/assignment operators. There seems to be a lot of discussion around these, and I am myself not convinced that the design is sound.
>
> Splitting this DIP in two would allow to move forward with 1/ relatively quickly and provide value to D user right away, while providing time and space for 2/ to ironed out.
While this is an attractive idea, (1) without some help from the programmer via adding the Move functions seems likely to cause problems.
|
March 20, 2021 Re: Feedback Thread: DIP 1040--Copying, Moving, and Forwarding--Community Review Round 1 | ||||
---|---|---|---|---|
| ||||
Posted in reply to Elronnd | On 3/17/2021 11:14 PM, Elronnd wrote:
>> If a Move Constructor is not defined for a struct that has a Move Assignment Operator, a default Move Constructor is defined and implemented as a move for each of its fields, in lexical order.
>
> I think the order should be undefined. Forcing the moves to happen in lexical order may inhibit some optimizations; and code which relies on the order of implicit moves is fragile anyway and shouldn't be encouraged. If the order is important for legitimate reasons, then those reasons should be made explicit in the code with an explicit move constructor.
The trouble with that is the user may inadvertently rely on the implementation-defined order, and his code will break when using another implementation.
The optimization advantage, if any, would be slight and not worth the risk.
|
March 20, 2021 Re: Feedback Thread: DIP 1040--Copying, Moving, and Forwarding--Community Review Round 1 | ||||
---|---|---|---|---|
| ||||
Posted in reply to Dukc | On 3/16/2021 10:19 AM, Dukc wrote: > It is labourous for the reader to get the difference between move and copy constructors, which is that copy constructors have a `ref` parameter. It'd be easier if this was explicitly mentioned. There'll be some more text added to deal with Rvalue Constructors, that should take care of this. > I don't like the term "move assignment operator". The operator is the same assignment operator we already have, no need to rename it. Just talk about how identity assignments are handled when the argument can be moved. You can call that a move assignment, but leave the trailing "operator" out, since there's no new operator. It's good to distinguish between a move and a copy. > There's no discussion how a move constructor will interact with a possible `opPostMove` in a struct or one of it's fields (DIP1014). https://github.com/dlang/DIPs/blob/master/DIPs/accepted/DIP1014.md Although #DIP1014 was approved, this DIP replaces it. That should be covered in the text. > As for all the stuff about EMOs, I don't like that at all. We already allow the compiler to move a struct instead of copying it, if it can detect a last use. And also we have the possibility to manually elide copying via `std.algorithm.move`. If I recall TDPL book correctly, returning a value or passing a Rvalue to a function is already guaranteed to move. And those rules are for all structs, not just for those that have some fancy operator overloads. > > I also don't like all the rules about detecting last uses. Those are optimizations, not something the language spec should complicate itself for. > > There is a breaking change that hasn't been mentioned. An identity assignment with no `ref` for the argument, or an `auto ref` argument, is going to become a move assignment. Thus it will break if it throws something. Only if the move assignment is defined, so it shouldn't be breaking. > All in all, I like the idea of move constructor - it is the same improvement over `opPostMove` that copy constructors are over postblits. But I cannot say the same about rest of the DIP. |
March 20, 2021 Re: Feedback Thread: DIP 1040--Copying, Moving, and Forwarding--Community Review Round 1 | ||||
---|---|---|---|---|
| ||||
Posted in reply to Mike Parker | On Friday, 5 March 2021 at 12:20:36 UTC, Mike Parker wrote:
>
> The review period will end at 11:59 PM ET on March 19, or when I make a post declaring it complete. Feedback posted to this thread after that point may be ignored.
This review round is complete. Thanks to everyone who participated.
The DIP authors may reply to unanswered feedback, but I ask that others please refrain from leaving any more posts in this thread.
|
March 23, 2021 Re: Feedback Thread: DIP 1040--Copying, Moving, and Forwarding--Community Review Round 1 | ||||
---|---|---|---|---|
| ||||
Posted in reply to Mike Parker Attachments:
| On Fri, Mar 5, 2021 at 10:25 PM Mike Parker via Digitalmars-d < digitalmars-d@puremagic.com> wrote:
> This is the feedback thread for the first round of Community Review of DIP 1040, "Copying, Moving, and Forwarding".
I'm really late to the party here, and there's a lot to catch up, so maybe I'm repeating discussion that already happened... but I think there's a few points that need consideration.
First, I like this DIP, I think this is good for D. That said, I also care deeply about C++ compatibility, and this DIP introduces some serious issues.
1. This changes the extern(C++) ABI, and it's a major breaking change, yet it's not mentioned in the breaking changes section; every by-val constructor call will have an ABI change, and C++ compatibility will completely explode.
2. By-val declarations matching f(T&&) is not a great default. It's relatively uncommon in C++ for these arguments to exist unless it makes specific sense, and many cases where it is present C++ uses 'universal references', that infer rval-ness.
3. ...and even when they do exist, this DIP has different ABI semantics! The C++ `T&&` function still expects the caller to destruct the argument, but in D, the caller will never destruct a moved argument. You talk about this, saying that it's a requirement to pay special attention to the semantic difference, but I don't think that's reasonable. You're going to have endless reports of issues from people that don't read or don't *understand* the difference, and complain their program is crashing.
4. Is it that C++'s criteria for an EMO matches your definition, or will there emerge cases where a differing EMO assessment will produce a mismatch in calling convention?
I think there are more problems I can imagine if I dive into the weeds, but at least these need to be addressed in the DIP.
My off-the-cuff thoughts are:
1 & 2. Rather than applying EMO semantics to extern(C++) functions by
default, and using @value to nominate the default behaviour; since you've
accepted an attribute just for C++ disambiguation, why not assume by-val
semantics as default, and require an @rval-ref attribute instead to select
the C++ move ABI?
3. Since your EMO semantics don't actually apply to C++, that is,
destruction responsibility does NOT carry into the call, I don't think EMO
semantics as described actually apply to C++ at all, and shouldn't be
attempted. We should use a separate solution for C++ compat (like the
attribute I mention above) and EMO semantics have no impact on C++.
So, in the presence of extern(C++), f(T arg) behaves 'traditionally' (as
you describe when applying `@value`), and maybe something like f(T @rvalref
arg) introduces the C++ rules (together with some attention to
overloading). EMO semantics as described have no applicability to C++.
Your C++ compatibility section needs a complete do-over as far as I'm
concerned. They don't improve the situation for C++ compatibility, they
make it worse.
Perhaps it should be tackled as a separate matter, and in that event, I
think the simplest thing to do is to assume f(T) for EMO remains by-val for
extern(C++), that is "EMO calling only applies to extern(D) code", and then
we make an @rvalref attribute that applies only to extern(C++) for the sake
of matching calling convention in a future DIP?
I might be interested in attempting a DIP that adds an extern(C++)-only rval-ref attribute on top of this DIP; It needs more work. Existing work (for instance, last SAOC) was taken from the perspective that it solves move semantics in D at the same time as interfacing C++. With this DIP addressing elaborate move in D, I think a new approach which ONLY addresses interfacing C++ is an easier sell, and keeps the feature much more isolated.
|
March 23, 2021 Re: Feedback Thread: DIP 1040--Copying, Moving, and Forwarding--Community Review Round 1 | ||||
---|---|---|---|---|
| ||||
Posted in reply to Manu | On Tuesday, 23 March 2021 at 00:35:31 UTC, Manu wrote:
> On Fri, Mar 5, 2021 at 10:25 PM Mike Parker via Digitalmars-d < digitalmars-d@puremagic.com> wrote:
>
>> [...]
>
>
> I'm really late to the party here, and there's a lot to catch up, so maybe I'm repeating discussion that already happened... but I think there's a few points that need consideration.
>
> [...]
Thank you for the feedback, I'm still mulling over it, but the idea of having move semantics for D that we can make the best they can possibly be, and then a solution for C++ compatibility seems like a good one. I'm not a huge fan of magic quasi-UDA things on parameters personally.
One thing I have put to Walter and would like more opinions on, is being a little more like Rust and less like C++ by having more violent move semantics that (often described as destructive) bypass the .init process described here. Although I'd like it, I'm not convinced of how practical it is, but it would greatly simplify analysis of the program both for a static analyzer (presumably the successor to ob.d) and for the programmers brain (e.g. think about how many value categories C++ has, and how use of move semantics in C++ can end up introducing a subtle new state to objects).
|
March 23, 2021 Re: Feedback Thread: DIP 1040--Copying, Moving, and Forwarding--Community Review Round 1 | ||||
---|---|---|---|---|
| ||||
Posted in reply to Manu | On Tuesday, 23 March 2021 at 00:35:31 UTC, Manu wrote:
> I'm really late to the party here, and there's a lot to catch up, so maybe I'm repeating discussion that already happened... but I think there's a few points that need consideration.
>
The extern(C++) case has not been discussed much so far. It goes without saying that it needs to be as close as C++ as it can be. I assumed the DIP to not apply to extern(C++).
|
March 24, 2021 Re: Feedback Thread: DIP 1040--Copying, Moving, and Forwarding--Community Review Round 1 | ||||
---|---|---|---|---|
| ||||
Posted in reply to deadalnix Attachments:
| On Tue, Mar 23, 2021 at 10:05 PM deadalnix via Digitalmars-d < digitalmars-d@puremagic.com> wrote:
> On Tuesday, 23 March 2021 at 00:35:31 UTC, Manu wrote:
> > I'm really late to the party here, and there's a lot to catch up, so maybe I'm repeating discussion that already happened... but I think there's a few points that need consideration.
> >
>
> The extern(C++) case has not been discussed much so far. It goes
> without saying that it needs to be as close as C++ as it can be.
> I assumed the DIP to not apply to extern(C++).
>
The DIP mentions C++ compat, and it's bad. I think it would be best if the whole DIP apply only to extern(D), and we'll address C++ compat separately.
|
March 24, 2021 Re: Feedback Thread: DIP 1040--Copying, Moving, and Forwarding--Community Review Round 1 | ||||
---|---|---|---|---|
| ||||
Posted in reply to Manu | On Wednesday, 24 March 2021 at 00:14:27 UTC, Manu wrote:
> On Tue, Mar 23, 2021 at 10:05 PM deadalnix via Digitalmars-d < digitalmars-d@puremagic.com> wrote:
>
>> On Tuesday, 23 March 2021 at 00:35:31 UTC, Manu wrote:
>> > I'm really late to the party here, and there's a lot to catch up, so maybe I'm repeating discussion that already happened... but I think there's a few points that need consideration.
>> >
>>
>> The extern(C++) case has not been discussed much so far. It goes
>> without saying that it needs to be as close as C++ as it can be.
>> I assumed the DIP to not apply to extern(C++).
>>
>
> The DIP mentions C++ compat, and it's bad. I think it would be best if the whole DIP apply only to extern(D), and we'll address C++ compat separately.
I think this is the right approach.
|
Copyright © 1999-2021 by the D Language Foundation