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

[Dukc]

On Tuesday, 13 August 2019 at 23:34:32 UTC, Manu wrote:
>
> struct S
> {
>   int front();
>   ...
> }
>
> void takesRef(ref int);
>
> {
>   S s;
>
>   takesRef(s.front()); // <- fine, temporary is created and 
> passed by ref
>
>   foreach(ref i; s) // <- error: no temporary is created; we 
> have
> decided to emit an error where an rval is given to a loop 
> counter
>   { ... }
> }
>
> This is the special case I'm talking about.

Nope, does not pass: `Error: function onlineapp.takesRef(ref int 
integer) is not callable using argument types (int)`


>> > Okay, here's some experiment cases:
>> >
>> >   int x = 1;
>> >   static foreach (i; AliasSeq!(10, x, 20))
>> >       pragma(msg, __traits(isRef, i));
>> >
>> >> false false false
>
> Iterating a tuple is a compile-time expansion, which is 
> distinct from
> a runtime expansion.
> The result is `false false false` either way, but static and
> non-static are very different operations.
>
> `foreach` will iterate an array of int's, `static foreach` will 
> expand the tuple and `i` will be an alias of each element.
>

No. `foreach` over an alias sequence will be unrolled at compile 
time, and behave like a `static foreach` does with CTFEd arrays. 
That was the way we did what `static foreach` does now, remember?

But I have to confess I'm not sure what `static foreach` does 
over an alias sequence. I ran a few tests, and some compiled 
despite my expectations, but didn't get a clear picture what it 
does. I think I need to read `static foreach` DIP again.

>> 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.

>> Yes, these are the cases I meant with `static foreach`. I 
>> think `[10, 20, 30]` is a rvalue, and if the latter of these 
>> two examples compiles, it should not.
>
> Actually, because of D's 'weird shit' law, the array is not an 
> rvalue.
> (I think...?)

It is. I tested that: `Error: constant value 10 cannot be ref`

>
>> > As above, we could allow this by creating temporaries the 
>> > same as function arguments... but we've decided not to allow 
>> > ref iterators from rvalues.
>> >
>> >   static foreach (auto ref i; [10, 20, 30])
>> >       pragma(msg, __traits(isRef, i));
>> >
>> >> false false false ??
>>
>> Yes, exactly what's supposed to happen.
>
> Right... what's your point?
> Incidentally, I'm not sure this will hold; because array 
> literals are
> not rvalues, so I think it might be `true true true` here...

Because the loop won't work with `ref`, it falls back to 
iteration-by-copy. But you're right that if the literals were not 
rvalues, that would result in `true true true`.

>> > I guess the whole array is an rvalue, so then the loop 
>> > counter would take copies of the elements?
>> >
>> > It gets interesting when you do this:
>> >
>> >   int[3] x = [10, 20, 30];
>> >   static foreach (ref i; x)
>> >       pragma(msg, __traits(isRef, i));
>> >
>> >> true true true
>
> Why not? It's an alias... it should compile just fine.

x is a runtime static array. Tested that also: `Error: variable x 
cannot be read at compile time`

>
>> >
>> > And:
>> >
>> >   int[3] x = [10, 20, 30];
>> >   static foreach (auto ref i; x)
>> >       pragma(msg, __traits(isRef, i));
>
> `x` is not an enum, it's a local.
> The cases I propose to consider are exactly what I wrote, not 
> some other thing.

You are right it is not an enum, like I also just said. But 
that's exactly the reason why it cannot compile, `ref` (or `auto 
ref`) or no. `static foreach` requires the values of the loop 
aggregate to be available at compile time, not just it's length.

I might well support an argument that this shouldn't be the case, 
but as it is, my DIP should not need to discuss that case.

>
> For instance:
>
> int fun(ref int x)
> {
>   static foreach (i; AliasSeq!(10, x, 30))  // <- outputs 
> `false true false`
>     pragma(msg, __traits(isRef, i));
>
>   foreach (i; AliasSeq!(10, x, 30)) // <- outputs `false false 
> false` (!!!)
>     pragma(msg, __traits(isRef, i));
> }
>
> [snip]
> I actually think this form of runtime foreach should be 
> deprecated, if
> you want to do this with runtime foreach, wrap the tuple in `[ 
> ]`.

An interesting idea for another potential DIP.

>
>
> I'm not strictly talking about what does actually compile, I'm 
> talking about what makes semantic sense, and would be uniform 
> semantically with no bizarre cases. Some cases might not work, 
> but should.

Perhaps, in some cases. But again, it's outside the scope of this 
DIP.