August 15, 2019
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.


August 15, 2019
On Thu, Aug 15, 2019 at 1:16 AM Dukc via Digitalmars-d <digitalmars-d@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.
August 17, 2019
On Friday, 16 August 2019 at 05:31:38 UTC, Manu wrote:
> On Thu, Aug 15, 2019 at 1:16 AM Dukc via Digitalmars-d
>> 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
>         }
>

Yes, this works. Disregard all what I have said about `static foreach` over an `AliasSeq`, I obviously need to think it completely again.

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

Yeah, as an `AliasSeq`, but I meant ranges and structs with an `opApply()` here. I know I didn't say it that way in the DIP, but I already agreed to change it.

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

I'll try to avoid describing it this way in the DIP, it's just that it was easiest to answer quote-by-quote in this theard and that led to my articulation of "except".

August 17, 2019
On Sat, Aug 17, 2019 at 12:58 AM Dukc via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
>
> On Friday, 16 August 2019 at 05:31:38 UTC, Manu wrote:
> > On Thu, Aug 15, 2019 at 1:16 AM Dukc via Digitalmars-d
> >> 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
> >         }
> >
>
> Yes, this works. Disregard all what I have said about `static foreach` over an `AliasSeq`, I obviously need to think it completely again.
>
> >>
> >> 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).
> >
>
> Yeah, as an `AliasSeq`, but I meant ranges and structs with an `opApply()` here. I know I didn't say it that way in the DIP, but I already agreed to change it.
>
> >> 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.
>
> I'll try to avoid describing it this way in the DIP, it's just that it was easiest to answer quote-by-quote in this theard and that led to my articulation of "except".

👍

I think we're on the same page now.
I'll stop hassling and let you make those tweaks :)

August 18, 2019
On Saturday, 17 August 2019 at 08:08:44 UTC, Manu wrote:
> 👍
>
> I think we're on the same page now.
> I'll stop hassling and let you make those tweaks :)

Thanks again. You revealed a lot of stuff.
August 23, 2019
I just got an email that this review is over. The changes I'm going to make are large enough that my plan is request another community review round after I've done my revisions.

Thanks you Mike, and thanks for all reviewers.

1 2 3 4
Next ›   Last »