On Friday, 20 July 2018 at 10:31:48 UTC, Seb wrote:
> On Friday, 20 July 2018 at 10:08:03 UTC, Dgame wrote:
>> On Friday, 20 July 2018 at 09:39:47 UTC, Nicholas Wilson wrote:
>>> On Friday, 20 July 2018 at 09:03:18 UTC, Dukc wrote:
>>>> appending something (like .byRef or byRef!long, the latter making an implicit type conversion)
>>>
>>> That can't work: either it returns an expired stack temporary (*very* bad), or allocates with no way to deallocate (bad).
>>
>> What about something like this?
>>
>> ----
>> import std.stdio;
>>
>> ref T byRef(T)(T value) {
>>     static T _val = void;
>>     _val = value;
>>
>>     return _val;
>> }
>>
>> void foo(ref int a) {
>>     writeln("A = ", a);
>> }
>>
>> void main() {
>>     foo(42.byRef);
>>     foo(23.byRef);
>> }
>> ----
>
> That can't work, consider e.g.:
>
> foo(10.byRef, 20.byRef); // calls foo with (20, 20)
>
> https://run.dlang.io/is/lazeu2

True.. But this could work (but is way more uglier): https://run.dlang.io/is/rKs2yQ

On Friday, 20 July 2018 at 05:16:53 UTC, Mike Parker wrote:
> This is the feedback thread for the first round of Community Review for DIP 1016, "ref T accepts r-values":
>
> https://github.com/dlang/DIPs/blob/725541d69149bc85a9492f7df07360f8e2948bd7/DIPs/DIP1016.md
>
> All review-related feedback on and discussion of the DIP should occur in this thread. The review period will end at 11:59 PM ET on August 3, or when I make a post declaring it complete.
>
> At the end of Round 1, if further review is deemed necessary, the DIP will be scheduled for another round. Otherwise, it will be queued for the Final Review and Formal Assessment by the language maintainers.
>
> Please familiarize yourself with the documentation for the Community Review before participating.
>
> https://github.com/dlang/DIPs/blob/master/PROCEDURE.md#community-review
>
> Thanks in advance to all who participate.

First of all, I'm very glad to see Manu's persistence turn to fruition and by incorporating all the feedback so far, I think we have a very solid proposal.

One question:

> It is important that `T` be defined as the argument type [..]

I think this should say that `T` is defined as the *parameter* type. This is what I would expect to happen for `void fun(ref int x, ref long y, ref double z)`:

```
fun(short(1), byte(2), int(3));
```

=>

```
{
    int    __fun_temp0 = void;
    long   __fun_temp1 = void;
    double __fun_temp2 = void;

    fun(
        __fun_temp0 := short(1),
        __fun_temp1 := byte(2),
        __fun_temp2 := int(3)
    );
}
```

One other subtle point is that the order of declaration of the temporaries determines their lifetime, so we must be careful with that. In fact I think that we should define it so declaration and initialization are not separated and simply make the order of declaration match the function argument evaluation order.

On Friday, 20 July 2018 at 10:43:54 UTC, Dgame wrote:
> On Friday, 20 July 2018 at 10:31:48 UTC, Seb wrote:
>> On Friday, 20 July 2018 at 10:08:03 UTC, Dgame wrote:
>>> On Friday, 20 July 2018 at 09:39:47 UTC, Nicholas Wilson wrote:
>>>> On Friday, 20 July 2018 at 09:03:18 UTC, Dukc wrote:
>>>>> appending something (like .byRef or byRef!long, the latter making an implicit type conversion)
>>>>
>>>> That can't work: either it returns an expired stack temporary (*very* bad), or allocates with no way to deallocate (bad).
>>>
>>> What about something like this?
>>>
>>> ----
>>> import std.stdio;
>>>
>>> ref T byRef(T)(T value) {
>>>     static T _val = void;
>>>     _val = value;
>>>
>>>     return _val;
>>> }
>>>
>>> void foo(ref int a) {
>>>     writeln("A = ", a);
>>> }
>>>
>>> void main() {
>>>     foo(42.byRef);
>>>     foo(23.byRef);
>>> }
>>> ----
>>
>> That can't work, consider e.g.:
>>
>> foo(10.byRef, 20.byRef); // calls foo with (20, 20)
>>
>> https://run.dlang.io/is/lazeu2
>
> True.. But this could work (but is way more uglier): https://run.dlang.io/is/rKs2yQ


Putting the missing syntax sugar aside and performance overhead due to unneeded copies, this won't work either.
Consider for example that A could be a non-copyable type:

---
struct A {
    int i;
    @disable this(this);
}
---

With this DIP it should be possible to do `foo(A(1))` because only a reference is passed around.

On Friday, 20 July 2018 at 05:16:53 UTC, Mike Parker wrote:
> Thanks in advance to all who participate.

"It has been noted that is it possible"
switch: it <-> is

On Friday, 20 July 2018 at 05:16:53 UTC, Mike Parker wrote:
> This is the feedback thread for the first round of Community Review for DIP 1016, "ref T accepts r-values":
>
> https://github.com/dlang/DIPs/blob/725541d69149bc85a9492f7df07360f8e2948bd7/DIPs/DIP1016.md
>
> All review-related feedback on and discussion of the DIP should occur in this thread. The review period will end at 11:59 PM ET on August 3, or when I make a post declaring it complete.
>
> At the end of Round 1, if further review is deemed necessary, the DIP will be scheduled for another round. Otherwise, it will be queued for the Final Review and Formal Assessment by the language maintainers.
>
> Please familiarize yourself with the documentation for the Community Review before participating.
>
> https://github.com/dlang/DIPs/blob/master/PROCEDURE.md#community-review
>
> Thanks in advance to all who participate.

I like this too. Solid work!

One minor point: the DIP could mentioned structs with disabled postblits (it currently only tangentially touches it by stating that no copy operation should happen).
It would be great to make sure that this will work:

---
struct A
{
    int i;
    @disable this(this);
}

void fun(ref A x);
void test()
{
    fun(A(42));
}
---

On 7/20/18 1:16 AM, Mike Parker wrote:
> This is the feedback thread for the first round of Community Review for DIP 1016, "ref T accepts r-values":
> 
> https://github.com/dlang/DIPs/blob/725541d69149bc85a9492f7df07360f8e2948bd7/DIPs/DIP1016.md 
> 
> 
> All review-related feedback on and discussion of the DIP should occur in this thread. The review period will end at 11:59 PM ET on August 3, or when I make a post declaring it complete.
> 
> At the end of Round 1, if further review is deemed necessary, the DIP will be scheduled for another round. Otherwise, it will be queued for the Final Review and Formal Assessment by the language maintainers.
> 
> Please familiarize yourself with the documentation for the Community Review before participating.
> 
> https://github.com/dlang/DIPs/blob/master/PROCEDURE.md#community-review
> 
> Thanks in advance to all who participate.

Looks pretty good to me.

There is very little discussion of the original problem (the reason why ref does not bind to rvalues). I don't know if this will satisfy Walter.

A couple points to make there:

1. `this` has been binding as ref to rvalues since structs received `this` by ref (yes, they were pointers at one point), and there really have not been averse effects.

2. The case is made to allow return ref pipeline functionality, but does not address the other cases. A strawman to discuss is a function which takes a value-type by reference, and returns void. It's possible the function squirrels away a copy of the value somewhere, but unlikely (why would it take it by ref in that case?). I'd say 99% of the time the point is to modify the parameter for use after the function ends. But in the rvalue case, you will lose it immediately. So in effect a call that seemingly has some effect will have none.

I would point out that 1 here serves as an example of why 2 isn't critical -- you can easily get into this exact situation WITHOUT the DIP if the function in question is a member function. And people have stumbled across such issues, fixed the problems, and moved on. I would say from personal experience the occurrence of such bugs is pretty rare.

I would also point out that the issues with generic code that have been pointed out already apply here as well -- generic code might NOT CARE if the function has any effect, but having to do metacrobatics to call it the "right way" or avoid calling it on purpose would be painful to write (as has been demonstrated).

One further point, on the default arguments: you can have a default argument be ref, so make sure that section does not read like it *always* has to create a temporary.

-Steve

On Friday, July 20, 2018 05:16:53 Mike Parker via Digitalmars-d wrote:
> This is the feedback thread for the first round of Community Review for DIP 1016, "ref T accepts r-values":
>
> https://github.com/dlang/DIPs/blob/725541d69149bc85a9492f7df07360f8e2948bd 7/DIPs/DIP1016.md
>
> All review-related feedback on and discussion of the DIP should occur in this thread. The review period will end at 11:59 PM ET on August 3, or when I make a post declaring it complete.
>
> At the end of Round 1, if further review is deemed necessary, the DIP will be scheduled for another round. Otherwise, it will be queued for the Final Review and Formal Assessment by the language maintainers.
>
> Please familiarize yourself with the documentation for the Community Review before participating.
>
> https://github.com/dlang/DIPs/blob/master/PROCEDURE.md#community-review
>
> Thanks in advance to all who participate.

I am completely against allowing ref to accept rvalues without some sort of attribute indicating that it should be allowed to (e.g. @rvalue ref). Allowing ref to accept rvalues goes completely against the idea that ref is for passing an object so that it can be mutated and have its result affect the caller. With this DIP, we'd likely start seeing folks using ref all over the place even when it has nothing to do with having the function mutating its arguments, and that's not only error-prone, but it obfuscates what ref was originally intended for.

auto ref was already introduced to solve this problem. The problem of course is that it requires that the function be templated, and while that's often desirable, it's not always a viable option (e.g. it doesn't work with virtual functions). So, I'm fine with adding something akin to auto ref which is intended to solve the non-templated case with semantics similar to those described in the DIP, but I think that it would be a huge mistake to make normal ref accept rvalues.

IMHO, having it be an error to pass an rvalue by ref is often a very valuable behavior, and I do _not_ want to lose that. I would be fine if this DIP proposed an attribute such as @rvalue to enable ref to accept rvalues, but I very much hope that this DIP is not accepted so long as it allows ref to accept rvalues without such an attribute.

- Jonathan M Davis

On Friday, 20 July 2018 at 13:21:11 UTC, Jonathan M Davis wrote:
>
> auto ref was already introduced to solve this problem. The problem of course is that it requires that the function be templated, and while that's often desirable, it's not always a viable option (e.g. it doesn't work with virtual functions). So, I'm fine with adding something akin to auto ref which is intended to solve the non-templated case with semantics similar to those described in the DIP, but I think that it would be a huge mistake to make normal ref accept rvalues.
>

I'm in favour of this DIP.

However, after this DIP... what is the point of the old "auto ref"? If you want to turn your function into a template it's trivial.

What if the current semantics of "auto ref" was redefined to what this DIP proposes, i.e. non-templated rvalue ref.

On Friday, 20 July 2018 at 05:16:53 UTC, Mike Parker wrote:
> This is the feedback thread for the first round of Community Review for DIP 1016, "ref T accepts r-values":
>
> https://github.com/dlang/DIPs/blob/725541d69149bc85a9492f7df07360f8e2948bd7/DIPs/DIP1016.md
>
> All review-related feedback on and discussion of the DIP should occur in this thread. The review period will end at 11:59 PM ET on August 3, or when I make a post declaring it complete.
>
> At the end of Round 1, if further review is deemed necessary, the DIP will be scheduled for another round. Otherwise, it will be queued for the Final Review and Formal Assessment by the language maintainers.
>
> Please familiarize yourself with the documentation for the Community Review before participating.
>
> https://github.com/dlang/DIPs/blob/master/PROCEDURE.md#community-review
>
> Thanks in advance to all who participate.

I like this proposal. It fits nicely in the C++ interface story and clears the complexity of defining non-template APIs that work with (const) ref inputs.
Kudos for Manu for pushing this through.

On Friday, July 20, 2018 15:18:33 Daniel N via Digitalmars-d wrote:
> On Friday, 20 July 2018 at 13:21:11 UTC, Jonathan M Davis wrote:
> > auto ref was already introduced to solve this problem. The problem of course is that it requires that the function be templated, and while that's often desirable, it's not always a viable option (e.g. it doesn't work with virtual functions). So, I'm fine with adding something akin to auto ref which is intended to solve the non-templated case with semantics similar to those described in the DIP, but I think that it would be a huge mistake to make normal ref accept rvalues.
>
> I'm in favour of this DIP.
>
> However, after this DIP... what is the point of the old "auto ref"? If you want to turn your function into a template it's trivial.
>
> What if the current semantics of "auto ref" was redefined to what this DIP proposes, i.e. non-templated rvalue ref.

When auto ref is used, the refness of the arguments is forwarded, which can be critical for stuff like emplace. It also avoids any temporaries, because if you pass an rvalue, it just instantiates the template so that the parameter isn't ref, resulting in a move, which is almost certainly more efficent than what this DIP proposes. So, we would definitely not want to change what auto ref means for templated functions.

We could choose to implement auto ref for non-templated functions using the semantics proposed for ref in this DIP, but that would mean that auto ref did different things for templated and non-templated code, which would be connfusing and potentially problematic when trying to do something like templatize an existing function. Also, it would mean that templates could not use the new semantics, whereas if we added a new attribute such as @rvalue, then both templated and non-templated functions could have functions accept rvalues by ref using temporaries in those cases that the programmer wants that behavior instead of auto ref (e.g. to reduce template bloat).

- Jonathan M Davis