On 08.03.21 15:01, Imperatorn wrote:
> On Monday, 8 March 2021 at 11:28:53 UTC, Timon Gehr wrote:
>> On 08.03.21 11:21, Walter Bright wrote:
>>>> [...]
>>>
>>> That's exactly how the @live analysis is currently implemented. I didn't know it was called a fixed point analysis.
>>
>> The general framework is also called abstract interpretation, where the merge operation is called a join of abstract elements. There's also a meet operation that can introduce information from conditionals into the state. (E.g. if you want to do value range propagation this can be useful.) In general, abstract interpretation does not converge to a fixed point in finite time, then you need to introduce some sort of widening operator. In practice, this is often the main challenge to make it work.
> 
> Just curious, is there any connection here to symbolic execution?

Most program analysis is abstract interpretation in some way as it is, well, abstract.

You can consider symbolic execution as an abstract domain consisting of symbolic formulas such that the concretization function γ maps each symbolic formula to the set of program states that satisfy it. Possible choices for meet and join are symbolic conjunction and disjunction.

Then, a fully precise abstract transformer f^# is a symbolic transformer in the sense f[γ(φ)] = γ(f^#(φ)). If your symbolic execution does not fully model some aspects of the program semantics you will get the ordinary soundness condition f[γ(φ)] ⊆ γ(f^#(φ)).

Widening for this domain is invariant synthesis and if you do it (automated or manually), you get something that's getting close to what's usually called a program verifier instead of a static analyzer.

On Monday, 8 March 2021 at 17:42:16 UTC, Timon Gehr wrote:
> On 08.03.21 15:01, Imperatorn wrote:
>> On Monday, 8 March 2021 at 11:28:53 UTC, Timon Gehr wrote:
>>> [...]
>> 
>> Just curious, is there any connection here to symbolic execution?
>
> Most program analysis is abstract interpretation in some way as it is, well, abstract.
>
> You can consider symbolic execution as an abstract domain consisting of symbolic formulas such that the concretization function γ maps each symbolic formula to the set of program states that satisfy it. Possible choices for meet and join are symbolic conjunction and disjunction.
>
> Then, a fully precise abstract transformer f^# is a symbolic transformer in the sense f[γ(φ)] = γ(f^#(φ)). If your symbolic execution does not fully model some aspects of the program semantics you will get the ordinary soundness condition f[γ(φ)] ⊆ γ(f^#(φ)).
>
> Widening for this domain is invariant synthesis and if you do it (automated or manually), you get something that's getting close to what's usually called a program verifier instead of a static analyzer.

Interesting. That sounds quite powerful. But what about undecidability? Wait... The abstract semantics is a super set here right? Hmm, so that's why that works.

This is actually kinda cool.

Do you have any good books on the subject to recommend?

On Monday, 8 March 2021 at 10:27:07 UTC, Walter Bright wrote:
> What the DIP should make clear, however, is that changing copies to moves should be regarded as implementation dependent, i.e. it is a mistake if the user attempts to rely on a particular sequence of moves and copies. Much like relying on this for NRVO is a mistake. NRVO has always been an optional optimization, and if it happens or not is difficult to determine just by looking at the code. In practice, the only problem that has cropped up has been pressure to find more cases where NRVO can be applied, so I think we're good with labeling the copy=>move semantics as an implementation-dependent optimization.

That seems like the best approach. It allows for thing like fixed point analysis to bail in case it needs too many iterations. In this case, the list of cases presented should be presented as examples rather than spec.

On Monday, 8 March 2021 at 10:21:37 UTC, Walter Bright wrote:
> Ok, good idea, although to be clear this is implicit in the Description.
>

I assumed so, but IMO this is very much adding to the rationale/requirement part of the DIP, because this will be forgotten or overlooked by most readers. When it is in there then it is easy to point further discussions that break this assumption to it. Without it, this has to be explained again and again.

On Monday, 8 March 2021 at 21:50:19 UTC, deadalnix wrote:
> On Monday, 8 March 2021 at 10:21:37 UTC, Walter Bright wrote:
>> Ok, good idea, although to be clear this is implicit in the Description.
>
I would like to remind everyone of the Feedback Thread rules. All posts must be a reply to my original post and must provide actionable feedback. DIP authors can reply to feedback, but there should be no replies to their posts unless they ask for clarity. Discussions belong in the discussion thread.

I’ll let all the posts above this one stand, but I will delete any further such posts.

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

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".

From the DIP:
> A Move Assignment Operator is a struct member assignment operator that moves, rather than copies, the argument corresponding to its first parameter into the constructed object. After the move is complete, the destructor is called on the original contents of the constructed object. The argument is invalid after this move, and is not destructed.

This paragraph seems garbled to me.

- What is "the constructed object"? It's ambiguous: presumably both `this` and the source argument to the move `opAssign` were initialized at some time in the past, otherwise why call the move `opAssign` instead of the move constructor? Generally, the phrase "the constructed object" is terribly confusing and for clarity should be replaced throughout the DIP with less ambiguous phrasing, such as "the source of the move" or "the destination of the move".

- "After the move is complete, the destructor is called on the original contents of the constructed object." This makes no sense: the original contents of the destination of the move cannot be destroyed *after* the move, as it has already been overwritten by the move, and is gone. It would have to either be destroyed *before* the move, or copied to a temporary first, which seems like a pointless waste.

- If the destination is to be destroyed before the move, then there seems to be no need for the separate move `opAssign` at all; the compiler should instead just call the destructor on the destination, if it has already been initialized, before calling the move constructor.

- If there is some good reason for the distinct move `opAssign` that I have missed, it should be explained clearly in the DIP.

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".

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.

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.

There's no discussion how a move constructor will interact with a possible `opPostMove` in a struct or one of it's fields (DIP1014).

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.

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.

> 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.

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.