Feedback Thread: DIP 1040--Copying, Moving, and Forwarding--Community Review Round 1

[Atila Neves]

On Friday, 5 March 2021 at 12:20:36 UTC, Mike Parker wrote:
> This is the feedback thread for the first round of Community 
> Review of DIP 1040, "Copying, Moving, and Forwarding".
>
> ===================================
> **THIS IS NOT A DISCUSSION THREAD**
>
> Posts in this thread must adhere to the feedback thread rules 
> outlined in the Reviewer Guidelines (and listed at the bottom 
> of this post).
>
> https://github.com/dlang/DIPs/blob/master/docs/guidelines-reviewers.md
>
> That document also provides guidelines on contributing feedback 
> to a DIP review. Please read it before posting here. If you 
> would like to discuss this DIP, please do so in the discussion 
> thread:
>
> https://forum.dlang.org/post/ncoqnixvllbjsxdzbyxi@forum.dlang.org
> ==================================
>
> You can find DIP 1040 here:
>
> https://github.com/dlang/DIPs/blob/a9c553b0dbab1c2983a801b5e89b51c5c33d5180/DIPs/DIP1040.md
>
> 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.
>
> At the end of this review round, the DIP will be moved into the 
> Post-Community Round 1 state. Significant revisions resulting 
> from this review round may cause the DIP manager to require 
> another round of Community Review, otherwise the DIP will be 
> queued for the Final Review.
>
> ==================================
> Posts in this thread that do not adhere to the following rules 
> will be deleted at the DIP author's discretion:
>
> * All posts must be a direct reply to the DIP manager's initial 
> post, with only two exceptions:
>
>     - Any commenter may reply to their own posts to retract 
> feedback contained in the original post
>
>     - The DIP author may (and is encouraged to) reply to any 
> feedback solely to acknowledge the feedback with agreement or 
> disagreement (preferably with supporting reasons in the latter 
> case)
>
> * Feedback must be actionable, i.e., there must be some action 
> the DIP author can choose to take in response to the feedback, 
> such as changing details, adding new information, or even 
> retracting the proposal.
>
> * Feedback related to the merits of the proposal rather than to 
> the contents of the DIP (e.g., "I'm against this DIP.") is 
> allowed in Community Review (not Final Review), but must be 
> backed by supporting arguments (e.g., "I'm against this DIP 
> because..."). The supporting arguments must be reasonable. 
> Obviously frivolous arguments waste everyone's time.
>
> * Feedback should be clear and concise, preferably listed as 
> bullet points (those who take the time to do an in-depth review 
> and provide feedback in the form of answers to the questions in 
> the documentation linked above will receive much gratitude). 
> Information irrelevant to the DIP or which is not provided in 
> service of clarifying the feedback is unwelcome.

Awesome! Now for the feedback.

I think the DIP would be helped by links to definitions that come 
after their first usage, or being restructured so that such links 
are no longer necessary. There are multiple cases, but the first 
example I ran into was "The argument is invalid after this move, 
and is not destructed.", and I wanted to write "what does 
'invalid' mean?". As it turns out it's defined later on in the 
document.

> Introduces the notion of a Moveable Reference

But then the DIP refers to these as "move refs" afterwards.

> where s is constructed into the parameter.

This confused me; it also seems to me that deleting it doesn't 
alter what the section is trying to say.

> A Move Constructor that throws is illegal.

I assume this means it'll fail to compile?

> The Move Assignment Operator is nothrow, even if nothrow is not 
> explicitly specified.

And also illegal if it happens to throw?

> An EMO is a struct that has both a Move Constructor and a Move 
> Assignment Operator

Including a compiler-generated version of either function?

> A Move Ref is a parameter that is a reference to an EMO

I eventually understood what this meant, but this confused me 
when I read it the first time. I'd reword it to mention that the 
syntax looks like a by-value parameter but ends up being passed 
by reference. It also confused me that the 2nd function had `ref` 
in there.

>  If NRVO cannot be performed, s is copied to the return value 
> on the caller's stack.

Why is it copied instead of moved?

> When the call is made to f(), a copy is made.

I guess this only applies to f(S())?

> While it appears that C++ best practice is to use rvalue 
> references instead of values for parameters

Not always, sometimes the best practice can be to pass by value 
then move if a copy has to be made anyway: 
https://stackoverflow.com/questions/16724657/why-do-we-copy-then-move