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

[Dukc]

On Wednesday, 14 August 2019 at 18:26:21 UTC, Manu wrote:
> You need -preview=rvalueRefParam
> I pointed you to Andrei's lecture that talks about this at the 
> start
> of the thread.

Now THAT was news to me! I had flipped through the slides of that 
lecture some day but I didn't remember we had such a review 
switch. I definitely need to address that somehow. To be frank, I 
may have to recommend considering alternative #1 instead of the 
DIP if that switch is planned to be mainstream.


> It does this:
>
> alias i = tuple[0];
> ... body
> alias i = tuple[1];
> ... body
> alias i = tuple[2];
> ... body
>
> This is why the ref function arg shows ref correctly. It 
> unrolls the loop for each element of the tuple as an alias.

Yeah, but for some reason a plain `foreach` will compile over an 
`AliasSeq` with ref (if all elements are lvalues), but `static 
foreach` won't (according to my tests). Might be a bug, the error 
message seemed strange.

I suppose that `foreach` and `static foreach` are intended behave 
exactly the same in this case. Before considering your proposal 
of deprecating non-static `foreach` for this use, that is.

I am going to mention that `static foreach` over alias sequence 
behaves exactly as without the keyword. I'll also make clear that 
"no rvalues possible for `static foreach`" won't apply here -only 
over ranges / `opApply`s.

>
>> >> Same answer for the rest of examples with tuples.
>> >
>> > No, static and non-static foreach are completely different
>> > semantically.
>> > My examples may show the same outputs either way, but it's 
>> > easy
>> > to
>> > make cases which show the distinction between static and
>> > non-static.
>>
>> See above.
>
> ?

I meant that so far, I've seen no diffenece in behaviour of 
`foreach` and `static foreach` when the aggregate is an 
`AliasSeq`, except that compilation failure i talked above.

I'm not arguing that behaviour should remain the same, just that 
this appears to be the intended behaviour of them currently.

If it is not, sorry, I still don't get it.



> It should effectively be the same as:
>
> int[3] x;
> static foreach (i; AliasSeq!(x[0], x[1], x[2])) {}
>
> Which doesn't work, but this does:
>
> int x, y, z;
> static foreach (i; AliasSeq!(x, y, z)) {}
>
> This also works:
>
> int[3] x;
> static foreach (i; 0 .. 3) { ... x[i] ... }
>
> Unrolling an array is super useful, we just don't support it... 
> but we could!
>
>> [snip]
>
> x is right there, or the code couldn't compile. Our meta is 
> perfectly
> fine with symbol aliases.
> For the non-ref (ie, alias) case, sub the array expansion with 
> `(x, y,
> z)` instead of `(x[0], x[1], x[2])`. It's exactly the same 
> thing, it's
> just not supported is all.
>
> From there, whether we support static foreach fabricating a ref 
> that
> points to an alias is a different question, but it's totally 
> possible.
>
> It's not any different for the compiler than this:
>
> void fun(ref int);
> int x;
> static foreach(ref i; AliasSeq!(x, x, x)) {} // <- i is a 
> reference to a local
> fun(x); fun(x); fun(x); // <- function receives a reference to 
> local
>
> It makes ref to x in both cases.
>
>> [snip]
>
> It's just that you specifically mention static foreach and that 
> it's
> meaningless; which I'm not sure is true.
> You should either not mention static foreach (your DIP applies 
> to
> runtime foreach), or should say what static foreach should 
> support,
> and that may be "static foreach does not interact with ref", 
> but it's
> possible that it could, and could be useful in the future.
>
> I think there's just a few gaps still where things that should 
> work
> haven't been requested. Loop unrolling primarily.
> We can't alias expressions at all.

Okay, I'll say that `static foreach` with `auto ref` has no 
effect as long as compile-time lvalues are not possible, but 
should change the behaviour accordingly if they are possible in 
the future. And remember, this clause does not apply to alias 
sequences.