rvalues DIP

[12345swordy]

1.) This is way long overdue here as it been over 3 years.
2.) I will work with Andrei and Walter regarding editing the dip.

[mdparker]

First thing to do is to add your name and contact info to the authors field since you're now the point of contact.

[12345swordy]

First thing to do is to add your name and contact info to the authors field since you're now the point of contact.

Done

[dukc]

I think -preview=in is supposed to solve this problem. How is it wrong or insufficient?

[12345swordy]

There is no submitted DIP for -preview=in switch.

[dukc]

Yes, it should have one if it's going to get in the default langauge. But DIPs should consider the alternatives, regardless of whether an alternative DIP is written or not.

[12345swordy]

1.) This dip does not concern itself with the -preview=in switch, as it has different design goals.
2.) This dip is not being submitted to the dip process yet as it waiting for a response during the D Language Foundation Monthly Meeting regarding preview=in

[Geod24]

1.) This dip does not concern itself with the -preview=in switch, as it has different design goals.

If you believe so, perhaps add a note about it in the DIP. That question is likely to come up frequently.

Regarding the DIP itself, something that comes to mind is whether or not this addresses the formal review of DIP1016.

Here, the DIP Author clearly expresses two reasons why a programmer may choose to declare a function to accept ref arguments. [...]The Language Maintainers insist that any proposal allowing ref parameters to accept rvalue references must clearly define how functions which make use of ref for side effects will not accept rvalues.

Looking at the DIP, it does mention the issue, but dismisses it:

In such cases, w.price is not assignable and calls such as applyDiscount(w.price) or w.price.applyDiscount will succeed but not perform any meaningful update. A maintainer expecting such calls to fail may be surprised. We consider this is an inevitable price to pay for the gained flexibility.

Currently on L198 of the diff.

As such, it seems that this proposal is unlikely to be successful, the language maintainers having already rejected a DIP for a flaw that the current proposal considers "inevitable price".

Looking at the other objections:

hey say that with the current semantics, this function only operates on long values as it should. With the proposed semantics, the call will accept all shared integral types. Any similar proposal must address this hole in order to be accepted.

This has been addressed by the Nonassignable Requirement.

The first problem the Language Maintainers identified with this approach is that the rewrite is from an expression to a statement, rendering it invalid. The expression should be rewritten as an expression to clarify how it behaves in larger expressions.

This is discussed in the Lifetime section. However, I have two points of concerns:

• First, it includes a specification for the order of evaluation of parameter: this is a clear scope creep of the DIP;
• Second, it doesn't explicitly address how throwing constructors are to be handled: It seems however that the stricter definition address the previous review's concerns;

Another questionable decision is to have evaluation be LTR for the arguments but RTL for the CallExp. Why ?

[Geod24]

There is no submitted DIP for -preview=in switch.

As a side note: No, there is no DIP. Instead, there is a working implementation, being used in production for over a year.
That implementation has worked out many of the quirks a DIP would not have found, both in the language and associated tooling. It would be foolish, and vain, to dismiss any evidence of a working solution for the benefit of a theoretical one.

[12345swordy]

@Geod24 This is not the time nor the place to raise concerns/objections/etc regarding this dip. Please wait until this PR is merged and the Community Review Round starts. This is not the place to discuss the -preview=in switch.

I want to be explicitly clear here, I am considered to be a champion regarding this dip as it involves one of the Language maintainers. I am NOT the technical leader here, Walter Andrie and Manu are. Any changes to this dip must received approval from all 3 of them (If they are still available that is).

[Imperatorn]

There is no submitted DIP for -preview=in switch.

As a side note: No, there is no DIP. Instead, there is a working implementation, being used in production for over a year. That implementation has worked out many of the quirks a DIP would not have found, both in the language and associated tooling. It would be foolish, and vain, to dismiss any evidence of a working solution for the benefit of a theoretical one.

Actually, when one thinks about it, might be a good idea in the future to just slap a preview switch for a feature, let it out and be tested in real life.

In the functional safety universe there's a term called "proven in use", i think parts of that could apply here as well.

No hypothesis, no matter how well thought out and rigorous, stands a chance against emperical evidence.

If not made before, a preview switch could be added in conjunction with the DIP.

I think that could actually increase the overall health of the implementation.

So, maybe do that even in this case? Test it. Prove that's it useful. If not, focus on other things.

[dukc]

@Geod24 This is not the time nor the place to raise concerns/objections/etc regarding this dip. Please wait until this PR is merged and the Community Review Round starts.

Wrong. The draft review is for that just as much as the community review is. From the reviewer guidelines:

Both of the first two stages of the review process are aimed at improving the DIP's language (see the author guidelines for hints), its technical strength, and its overall quality. Personal opinions on the merit of the proposed feature are welcome in both stages as long as they are accompanied by concrete reasons that identify flaws with the proposal.

[12345swordy]

The draft review is for that just as much as the community review is.

I considered the time window for the draft review be already over for over 3 years ago. This should be submitted as part of the dip process a long time ago. Only for it to be forgotten over time. Well better late then never as they say. Until then, wait until this is either merge or get I get notification from @WalterBright that this dip is canceled. Until then, I am not making changes to this dip without approval from the other 3.

[dukc]

Address the reviews given here by committing your changes to another branch, and from that create a pull request to rvalues branch in your own fork. Then ask the technical DIP leaders review that PR.

[12345swordy]

Address the reviews given here...

I am NOT responsible for addressing any reviews that is brought up here. That is up for the other three to decided.

[dukc]

Let them to create the PRs to 12345swordy:rvalues then.

[12345swordy]

Let them to create the PRs to 12345swordy:rvalues then.

You can freely contact them, by emailing them and they can email me back so that we can discuss our next plan of action regarding the review.

[dukc]

Since it's them who are deciding what goes to the DIP, they will have to follow this discussion to see what should be addressed.

Ping @WalterBright @TurkeyMan @andralex

#218 (comment) fairly much sums up what there is to address so far.

[12345swordy]

@dukc Stop spamming this thread, this is NOT the place to discuss your issues with this draft.
Post your grievous here: https://forum.dlang.org/post/yqrqtrsqcjdatjjdsubv@forum.dlang.org

[mdparker]

@12345swordy I suggest you take a look at this:

https://github.com/dlang/DIPs/blob/master/docs/process-reviews.md#draft-review

this is NOT the place to discuss your issues with this draft.

This DIP was in Draft Review the moment you submitted the PR. That means it is the place for people to raise their issues with this DIP.

I am NOT responsible for addressing any reviews that is brought up here.

You submitted the DIP, and that means you have taken on the responsibility of addressing any issues raised at every stage of the review process, including the Draft Review. You can do so directly yourself or by asking one of the original authors to do so, but it absolutely is your responsibility.

[12345swordy]

This DIP was in Draft Review the moment you submitted the PR. That means it is the place for people to raise their issues with this DIP.

I have already address this objection already:

"I considered the time window for the draft review be already over for over 3 years ago. This should be submitted as part of the dip process a long time ago. Only for it to be forgotten over time. Well better late then never as they say. Until then, wait until this is either merge or get I get notification from @WalterBright that this dip is canceled. Until then, I am not making changes to this dip without approval from the other 3."

I considered the draft review to be done a long time ago. You may disagree due to technicality, but I am not going to responded to any issues brought here as this should have been submited a long time ago. Other dip authors can address the issues if they wish.

You submitted the DIP, and that means you have taken on the responsibility of addressing any issues raised at every stage of the review process, including the Draft Review.

Which I considered to been done over 3 years ago. I am not going address any issues that is post here, as I deemed it to be completely unnecessarily as I am not the one who originally wrote this and just made minor tweeks to it, for this to be accepted.

If this PR is not being merge, because the lack of response to the issues that is brought up, then I will gladly ping the other 3 until they responded to said issues.

[mdparker]

I have already address this objection already:

I saw that. And my comment was to inform you that your understanding of the process is flawed. As described in the authoring process documentation, while the DIP is in the author's forked repository, it's considered to be in the development stage. The author can solicit feedback forever, but that is not the Draft Review stage of the DIP process.

I am not the one who originally wrote this and just made minor tweeks to it, for this to be accepted

This is your DIP now. You've submitted it.

If this PR is not being merge, because the lack of response to the issues that is brought up, then I will gladly ping the other 3 until they responded to said issues.

You are not required to address all feedback during Draft Review, but it is a courtesy to respond. I may ask you to address some of the feedback before merging, but when this DIP is merged mostly depends on other factors. I just wanted you to understand that this is in Draft Review and you are responsible for it, so please stop demanding that people not participate in the process.

And please be aware that once the Community Review begins, you will be completely responsible for addressing review feedback if the original authors are unavailable. If you aren't willing to do that, then I will need to hold the DIP back until you can make arrangements for one of them to be available for the two weeks of CR.

[12345swordy]

@mdparker given that this dip has been supersedes by -preview=in, do I close this PR or merge this in with the status set to "supersedes"?

[mdparker]

It never got out of Draft, so we just close it. Only DIPs that get merged have a status other than Draft.

[dukc]

@12345swordy Just to be sure, did you receive a confirmation from them that they decided to drop this DIP? If you got the idea from the monthly meeting summary, that's not quite a confirmation yet. -preview=in isn't yet at the default language after all.

[mdparker]

@dukc He got the confirmation from me in a separate email a week or so ago. The discussion at the meeting was to determine the fate of -preview=in. Kill it, or let it stand. They decided to let it stand. And it will accept rvalue references. Mathias will write a DIP for it and it will almost certainly be approved.

[dukc]

That sounds like it. My sympathies for this paper getting rejected.