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

[Manu]

On Thu, Aug 15, 2019 at 1:16 AM Dukc via Digitalmars-d
<digitalmars-d at puremagic.com> wrote:
>
> 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 don't think they behave exactly the same in anyAliasSeq case.
static foreach over an AliasSeq has an `alias` iterator.
foreach ever has an alias iterator.

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

It doesn't behave exactly the same, it's quite different, and I showed
that in code a few posts back.

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

One is `alias`, one is `auto`. They're semantically VERY different,
your meta in the loop body will behave differently.

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

        void fun(ref int x)
        {
            auto y = x; // remember x

            foreach (i; AliasSeq!(x, x))
            {
                ++i;
            }
            assert(x == y); // only copies were incremented, x is unchanged

            static foreach (i; AliasSeq!(x, x))
            {
                ++i;
            }
            assert(x == y + 2); // the aliased symbol 'x' was incremented
        }

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

Compile-time lvalues are absolutely possible, I've showed code how it
works a bunch of times, including just above (difference between
foreach and static foreach).

> And remember, this clause does not apply to alias
> sequences.

It's a bit weird to say 'except' that way; static foreach primarily
applies to tuples, and foreach should probably be deprecated such that
it can't apply to tuples.

It's not technically wrong, but it's just seems a bit of a weird way
to paint that rule to me.