On Thursday, 24 January 2019 at 23:59:30 UTC, Walter Bright wrote:
> On 1/24/2019 1:03 PM, kinke wrote:
>> (bool __gate = false;) , ((A __pfx = a();)) , ((B __pfy = b();)) , __gate = true , f(__pfx, __pfy);
>
> There must be an individual gate for each of __pfx and pfy. With the rewrite above, if b() throws then _pfx won't be destructed.

There is no individual gate, there's just one to rule the caller-destruction of all temporaries. That's the current state, and there's no need for that to change. I was trying to say that a rewrite as expression, as requested as part of the assessment, clearly isn't enough, as the dtor expressions aren't visible this way, and neither is the scoping (when the dtor expression of `__pfx` comes into play etc.).

On 1/24/2019 11:53 PM, Nicholas Wilson wrote:
> That the conflation of pass by reference to avoid copying and mutation is not only deliberate but also mitigated by @disable.

The first oddity about @disable is it is attached to the foo(int), not the foo(ref int). If I wanted to know if foo(ref int) takes rvalue references, I'd have to go looking for the existence of another function. This is one of those cases where it's hard to prove a negative, as other functions can be introduced by mixins. This is a strong usability negative.

Next, the @disable applies to the entire parameter list. However, overload selection is done by looking at each parameter. The DIP says:

"The DIP author responded that ideas to improve this are welcome, but that he cannot imagine a use case."

I can guarantee that the use case of more than one reference parameter will come up. The workarounds the DIP suggests are simply awful.

Let's look at what C++ does for rvalue references:

    http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2027.html

The syntax is attached to the parameter declaration of the function it applies to, not some other function, and not every parameter:

    int foo(T&& t);  // C++ rvalue ref

There are no weird workarounds, at least for that aspect. There are indeed unlikable things about the C++ rules, but the DIP needs to pay more attention to how C++ does this, and justify why D differs. Particularly because D will likely have to have some mechanism of ABI compatibility with C++ functions that take rvalue references.

This is not a small problem.

A further problem is implicit conversions, which the DIP ignores by only talking about ints.

    void bar(int);
    void foo(ref int);
    enum short s = 10;
    bar(s); // compiles
    foo(s); // currently fails to compile

Should `s` be promoted to an int temporary, then pass the temporary by reference? I can find no guidance in the DIP. What if `s` is a uint (i.e. the implicit conversion is a type paint and involves no temporary)?

Here's a discussion of Rust and rvalue references which may offer insight:

https://www.reddit.com/r/rust/comments/3ko5pm/explaining_cs_rvalue_references_from_a_rust/


> That the DIP applies to statements, not expressions.

The DIP should not invent its own syntax, give no explanation of it, and have the reader guess. (It did explain the :=, but not the use of { } and statements.) And, even if one did a mental rewrite, the semantics of the statement version are simply wrong. (For example, if 'fun' was actually a function pointer returned by another function, and that other function threw an exception - then the destructor would be run on an uninitialized variable.)


> That the construction order issue is trivially fixable, by specifying the same behaviour as the non ref case modulo ref.

It should never have gotten this far without giving a precise explanation of how exception safety is achieved when faced with multiple parameters. In the past I've done a lot of work on exception safety, and it isn't trivial once one goes beyond trivial cases.

All that criticism aside, I'd like to see rvalue references in D. But the DIP needs significant work.

On Friday, 25 January 2019 at 11:56:58 UTC, Walter Bright wrote:
> It should never have gotten this far without giving a precise explanation of how exception safety is achieved when faced with multiple parameters.

The pot calling the kettle black. DIP1000? DIP1017?

Again, all that requires is a minor revision. It is not DIP breaking.

On Friday, 25 January 2019 at 12:03:36 UTC, Nicholas Wilson wrote:
> On Friday, 25 January 2019 at 11:56:58 UTC, Walter Bright wrote:
>> It should never have gotten this far without giving a precise explanation of how exception safety is achieved when faced with multiple parameters.
>
> The pot calling the kettle black. DIP1000? DIP1017?

Or DIP1008: https://issues.dlang.org/show_bug.cgi?id=19463

On 1/25/19 5:57 AM, kinke wrote:
> On Thursday, 24 January 2019 at 23:59:30 UTC, Walter Bright wrote:
>> On 1/24/2019 1:03 PM, kinke wrote:
>>> (bool __gate = false;) , ((A __pfx = a();)) , ((B __pfy = b();)) , __gate = true , f(__pfx, __pfy);
>>
>> There must be an individual gate for each of __pfx and pfy. With the rewrite above, if b() throws then _pfx won't be destructed.
> 
> There is no individual gate, there's just one to rule the caller-destruction of all temporaries. That's the current state, and there's no need for that to change. I was trying to say that a rewrite as expression, as requested as part of the assessment, clearly isn't enough, as the dtor expressions aren't visible this way, and neither is the scoping (when the dtor expression of `__pfx` comes into play etc.).

I think the point of the DIP is not to lower expressions. It makes no sense to, they have to be statements (just like all temporaries live until the end of a statement).

-Steve

On Friday, 25 January 2019 at 11:56:58 UTC, Walter Bright wrote:
> All that criticism aside, I'd like to see rvalue references in D. But the DIP needs significant work.

I haven't participated to writing this DIP, but I personally appreciate this level of feedback.

I think it would have been more appreciated if the original answer had that level of detail.

Otherwise, I don't think this should be an all-or-nothing situation. It would make sense to bump the DIP back one stage or two for minor adjustments, and if the author decide that they need to make major changes to get past the problems you mention, require these changes to go through the entire process again (through another DIP).

On 1/25/2019 2:57 AM, kinke wrote:
> On Thursday, 24 January 2019 at 23:59:30 UTC, Walter Bright wrote:
>> On 1/24/2019 1:03 PM, kinke wrote:
>>> (bool __gate = false;) , ((A __pfx = a();)) , ((B __pfy = b();)) , __gate = true , f(__pfx, __pfy);
>>
>> There must be an individual gate for each of __pfx and pfy. With the rewrite above, if b() throws then _pfx won't be destructed.
> 
> There is no individual gate, there's just one to rule the caller-destruction of all temporaries.

What happens, then, when b() throws?

On Friday, 25 January 2019 at 11:56:58 UTC, Walter Bright wrote:
> On 1/24/2019 11:53 PM, Nicholas Wilson wrote:
>> That the conflation of pass by reference to avoid copying and mutation is not only deliberate but also mitigated by @disable.
>
> The first oddity about @disable is it is attached to the foo(int), not the foo(ref int). If I wanted to know if foo(ref int) takes rvalue references, I'd have to go looking for the existence of another function. This is one of those cases where it's hard to prove a negative, as other functions can be introduced by mixins. This is a strong usability negative.
>
> Next, the @disable applies to the entire parameter list. However, overload selection is done by looking at each parameter. The DIP says:
>
> "The DIP author responded that ideas to improve this are welcome, but that he cannot imagine a use case."
>
> I can guarantee that the use case of more than one reference parameter will come up. The workarounds the DIP suggests are simply awful.
>
> Let's look at what C++ does for rvalue references:
>
>     http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2027.html
>
> The syntax is attached to the parameter declaration of the function it applies to, not some other function, and not every parameter:
>
>     int foo(T&& t);  // C++ rvalue ref
>
> There are no weird workarounds, at least for that aspect. There are indeed unlikable things about the C++ rules, but the DIP needs to pay more attention to how C++ does this, and justify why D differs. Particularly because D will likely have to have some mechanism of ABI compatibility with C++ functions that take rvalue references.
>
> This is not a small problem.
>
> A further problem is implicit conversions, which the DIP ignores by only talking about ints.
>
>     void bar(int);
>     void foo(ref int);
>     enum short s = 10;
>     bar(s); // compiles
>     foo(s); // currently fails to compile
>
> Should `s` be promoted to an int temporary, then pass the temporary by reference? I can find no guidance in the DIP. What if `s` is a uint (i.e. the implicit conversion is a type paint and involves no temporary)?
>
> Here's a discussion of Rust and rvalue references which may offer insight:
>
> https://www.reddit.com/r/rust/comments/3ko5pm/explaining_cs_rvalue_references_from_a_rust/
>
>
>> That the DIP applies to statements, not expressions.
>
> The DIP should not invent its own syntax, give no explanation of it, and have the reader guess. (It did explain the :=, but not the use of { } and statements.) And, even if one did a mental rewrite, the semantics of the statement version are simply wrong. (For example, if 'fun' was actually a function pointer returned by another function, and that other function threw an exception - then the destructor would be run on an uninitialized variable.)
>
>
>> That the construction order issue is trivially fixable, by specifying the same behaviour as the non ref case modulo ref.
>
> It should never have gotten this far without giving a precise explanation of how exception safety is achieved when faced with multiple parameters. In the past I've done a lot of work on exception safety, and it isn't trivial once one goes beyond trivial cases.
>
> All that criticism aside, I'd like to see rvalue references in D. But the DIP needs significant work.

For future reference, this is what a formal review should be. I'd also rather your exact words than some summarization of them.

On 26/01/2019 10:00 AM, Rubn wrote:
> On Friday, 25 January 2019 at 11:56:58 UTC, Walter Bright wrote:
>> On 1/24/2019 11:53 PM, Nicholas Wilson wrote:
>>> That the conflation of pass by reference to avoid copying and mutation is not only deliberate but also mitigated by @disable.
>>
>> The first oddity about @disable is it is attached to the foo(int), not the foo(ref int). If I wanted to know if foo(ref int) takes rvalue references, I'd have to go looking for the existence of another function. This is one of those cases where it's hard to prove a negative, as other functions can be introduced by mixins. This is a strong usability negative.
>>
>> Next, the @disable applies to the entire parameter list. However, overload selection is done by looking at each parameter. The DIP says:
>>
>> "The DIP author responded that ideas to improve this are welcome, but that he cannot imagine a use case."
>>
>> I can guarantee that the use case of more than one reference parameter will come up. The workarounds the DIP suggests are simply awful.
>>
>> Let's look at what C++ does for rvalue references:
>>
>> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2027.html
>>
>> The syntax is attached to the parameter declaration of the function it applies to, not some other function, and not every parameter:
>>
>>     int foo(T&& t);  // C++ rvalue ref
>>
>> There are no weird workarounds, at least for that aspect. There are indeed unlikable things about the C++ rules, but the DIP needs to pay more attention to how C++ does this, and justify why D differs. Particularly because D will likely have to have some mechanism of ABI compatibility with C++ functions that take rvalue references.
>>
>> This is not a small problem.
>>
>> A further problem is implicit conversions, which the DIP ignores by only talking about ints.
>>
>>     void bar(int);
>>     void foo(ref int);
>>     enum short s = 10;
>>     bar(s); // compiles
>>     foo(s); // currently fails to compile
>>
>> Should `s` be promoted to an int temporary, then pass the temporary by reference? I can find no guidance in the DIP. What if `s` is a uint (i.e. the implicit conversion is a type paint and involves no temporary)?
>>
>> Here's a discussion of Rust and rvalue references which may offer insight:
>>
>> https://www.reddit.com/r/rust/comments/3ko5pm/explaining_cs_rvalue_references_from_a_rust/ 
>>
>>
>>
>>> That the DIP applies to statements, not expressions.
>>
>> The DIP should not invent its own syntax, give no explanation of it, and have the reader guess. (It did explain the :=, but not the use of { } and statements.) And, even if one did a mental rewrite, the semantics of the statement version are simply wrong. (For example, if 'fun' was actually a function pointer returned by another function, and that other function threw an exception - then the destructor would be run on an uninitialized variable.)
>>
>>
>>> That the construction order issue is trivially fixable, by specifying the same behaviour as the non ref case modulo ref.
>>
>> It should never have gotten this far without giving a precise explanation of how exception safety is achieved when faced with multiple parameters. In the past I've done a lot of work on exception safety, and it isn't trivial once one goes beyond trivial cases.
>>
>> All that criticism aside, I'd like to see rvalue references in D. But the DIP needs significant work.
> 
> For future reference, this is what a formal review should be. I'd also rather your exact words than some summarization of them.

So in other words, a formal review should include somebody acting as an 'interviewer' prompting questions. If that is the case, I do think it would be a good idea.

On Friday, 25 January 2019 at 19:08:55 UTC, Walter Bright wrote:
> On 1/25/2019 2:57 AM, kinke wrote:
>> On Thursday, 24 January 2019 at 23:59:30 UTC, Walter Bright wrote:
>>> On 1/24/2019 1:03 PM, kinke wrote:
>>>> (bool __gate = false;) , ((A __pfx = a();)) , ((B __pfy = b();)) , __gate = true , f(__pfx, __pfy);
>>>
>>> There must be an individual gate for each of __pfx and pfy. With the rewrite above, if b() throws then _pfx won't be destructed.
>> 
>> There is no individual gate, there's just one to rule the caller-destruction of all temporaries.
>
> What happens, then, when b() throws?

`__pfx` goes out of scope, and is dtor expression (cleanup/finally) is run as part of stack unwinding. Rewritten as block statement:

{
  bool __gate = false;
  A __pfx = a();
  try {
    B __pfy = b(); // may throw
    __gate = true;
    return f(__pfx, __pfy); // move args to callee
  } finally {
    __gate || __pfx.__xdtor(); // only destruct if not moved to callee
  }
}

With this DIP, the g() call (both rvalue args passed by ref) would now become:

{
  A __pfx = a();
  try {
    B __pfy = b(); // may throw
    try {
      return g(__pfx, __pfy); // pass by ref
    } finally {
      __pfy.__xdtor(); // always destructed by caller
    }
  } finally {
    __pfx.__xdtor();
  }
}