DIP 1022---foreach auto ref---Community Review Round 2

[Timon Gehr]

On 19.10.19 15:16, Mike Parker wrote:
> This is the feedback thread for the second round of Community Review for 
> DIP 1022, "foreach auto ref":
> 
> https://github.com/dlang/DIPs/blob/089816bc47ee3d1df06d10aa2af943d3e70e6161/DIPs/DIP1022.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 November 2, or 
> when I make a post declaring it complete.
> 
> At the end of Round 2, if further review is deemed necessary, the DIP 
> will be scheduled for another round of Community Review. Otherwise, it 
> will be queued for the Final Review and Formal Assessment.
> 
> Anyone intending to post feedback in this thread is expected to be 
> familiar with the reviewer guidelines:
> 
> https://github.com/dlang/DIPs/blob/master/docs/guidelines-reviewers.md
> 
> *Please stay on topic!*
> 
> Thanks in advance to all who participate.

- I think it is important that the lines marked (1) and (2) in the code 
below have equivalent validity:

---
struct Iota{
     int s,e;
     void popFront(){ s++; }
     @property bool empty(){ return s>=e; }
     @property int front(){ return s; }
     version(ASSIGNABLE_FRONT) @property void front(int i){ s=i; }
}

void foo(ref int){}

void main(){
     foo(Iota(0,5).front);      // (1)
     foreach(ref x;Iota(0,5)){} // (2)
}
---

As far as I can tell, with the current state of the language, this is 
what this DIP attempts to propose. Given the precedent of `auto ref` 
functions, the newly introduced syntax also makes sense, so I am 
generally in favor of this DIP. (With the understanding that the 
behavior may or may not change again once we introduce some way to pass 
rvalues by `ref`.)

- The DIP claims that "It also proposes that current usages of `ref` may 
be replaced with `auto ref` to retain the current behavior."

This is not correct:

foreach(ref i;iota(0,5)){ // current behavior:
     static assert(__traits(isRef,i));
}
foreach(auto ref i;iota(0,5)){ // new behavior:
     static assert(!__traits(isRef,i));
}


- The DIP fails to address the following cases:

import std.stdio;
void main(){
     foreach(ref x;0..5){ // ???
         if(x==3) x=5;
         writeln(x);
     }
     foreach(auto ref x;0..5){ // ???
         if(x==3) x=5;
         writeln(x);
     }
}

void foo(ref int x){}
void main(){
     [1,2,3][1]=2; // currently: error
     foo([1,2,3][1]); // currently: ok
     foreach(ref x;[1,2,3]){ // ???
         x=2;
     }
     foreach(auto ref x;[1,2,3]){ // ???
         x=2;
     }
}

I think both of these should be discussed explicitly.