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

[Manu]

On Wed, Aug 14, 2019 at 8:40 AM Dukc via Digitalmars-d
<digitalmars-d at puremagic.com> wrote:
>
> 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)`

You need -preview=rvalueRefParam
I pointed you to Andrei's lecture that talks about this at the start
of the thread.

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

Yes, I recall. But static foreach exists now, so I feel like it should
be deprecated in favour of static foreach.

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

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.

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

Okay, good. There was a time where array literals use to allocate
using the GC...

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

Right, sorry, I wasn't sure the current state of array literals. There
was a time when they allocated memory.

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

It's not being read, just referenced. This can/should work, it just doesn't.

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!

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

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.

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

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.

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

I was just pointing out that I think this text should change:
"It should be allowed in static foreach, but with no effect, as
elements of compile-time aggregates can never be lvalues."

Elements CAN be lvalues in static foreach, that's what I was trying to show:

int x, y, z;
static foreach (i; AliasSeq!(x, y, z)) {} // <- compiles in today's
language, all elements are lvalues.

I'm saying, if you allow `auto ref` in static foreach, it should do
the thing (capture lvalues by ref).
I think you should change it to say it is not allowed in static
foreach, at least until we work out if/how ref in static foreach
should work.
I think it should work naturally; ref's to symbol aliases is possible,
and I think the uniformity would be sensible, and I agree it's outside
this DIP, which is why you should change that text.