www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - DIP 1022--foreach auto ref--Community Review Round 1

reply Mike Parker <aldacron gmail.com> writes:
This is the feedback thread for the first round of Community 
Review for DIP 1022, "foreach auto ref":

https://github.com/dlang/DIPs/blob/8f3c24df2acc0f3554875a9decf6314508b72d8a/DIPs/DIP1022.md

All review-related feedback on and discussion of the DIP should 
occur in this thread. The review period will end at 11:59 PM ET 
on Aug 22, or when I make a post declaring it complete.

At the end of Round 1, if further review is deemed necessary, the 
DIP will be scheduled for another round of Community Review. 
Otherwise, it will be queued for the Final Review and Formal 
Assessment.

Anyone intending to post feedback in this thread is expected to 
be familiar with the reviewer guidelines:

https://github.com/dlang/DIPs/blob/master/docs/guidelines-reviewers.md

I will be moving (copying, deleting, and pasting in a new thread) 
posts that do not adhere to the reviewer guidelines. So please 
keep all comments in this thread focused on DIP 1022.

Thanks in advance to all who participate.
Aug 08 2019
next sibling parent reply Dukc <ajieskola gmail.com> writes:
On Thursday, 8 August 2019 at 12:10:09 UTC, Mike Parker wrote:

 This is the feedback thread for the first round of Community 
 Review for DIP 1022, "foreach auto ref":


Author here, AMA/Destroy!
Aug 08 2019
next sibling parent reply Exil <Exil gmall.com> writes:
On Thursday, 8 August 2019 at 12:13:35 UTC, Dukc wrote:

 On Thursday, 8 August 2019 at 12:10:09 UTC, Mike Parker wrote:

 This is the feedback thread for the first round of Community 
 Review for DIP 1022, "foreach auto ref":


 Author here, AMA/Destroy!


Not sure if "foreach( ref" should be deprecated. Doesn't make 
sense to me with other uses of "auto ref", like in function 
parameters. They still allow you to use ref. If you did remove 
it, then I'm not sure how opApply would work in that case. If you 
want to have both ref and copy, or even just two different types. 
You can't select the proper ref type because of auto.
Aug 08 2019
parent reply Dukc <ajieskola gmail.com> writes:
On Thursday, 8 August 2019 at 12:35:45 UTC, Exil wrote:

 Not sure if "foreach( ref" should be deprecated.


I'm not quite sure what you meant, but if you're afraid that the 
DIP will deprecate foreach-ref in whole, no. It will only 
deprecate foreach-ref with rvalues. Auto ref will work exactly as 
ref does now.
Aug 08 2019
parent reply Exil <Exil gmall.com> writes:
On Thursday, 8 August 2019 at 12:44:23 UTC, Dukc wrote:

 On Thursday, 8 August 2019 at 12:35:45 UTC, Exil wrote:

 Not sure if "foreach( ref" should be deprecated.


 I'm not quite sure what you meant, but if you're afraid that 
 the DIP will deprecate foreach-ref in whole, no. It will only 
 deprecate foreach-ref with rvalues. Auto ref will work exactly 
 as ref does now.


Like I said in the rest of the paragraph. It restricts it 
unncecssarily if.you deprecate it. You can't specify the type if 
you only allow "auto ref". There's no reason to deprecate it. So 
no, it does not work exactly as ref does now cause it won't work 
at all in some instances such as when using opApply.
Aug 08 2019
parent Dukc <ajieskola gmail.com> writes:
On Thursday, 8 August 2019 at 14:04:30 UTC, Exil wrote:

 Like I said in the rest of the paragraph. It restricts it 
 unncecssarily if.you deprecate it. You can't specify the type 
 if you only allow "auto ref". There's no reason to deprecate 
 it. So no, it does not work exactly as ref does now cause it 
 won't work at all in some instances such as when using opApply.


Seems that this one needs clarification. Type should be able to 
be specified between `auto ref` and the identifier it applies to. 
Perhaps I need to include a grammar specification.
Aug 08 2019
prev sibling next sibling parent reply Andrea Fontana <nospam example.com> writes:
On Thursday, 8 August 2019 at 12:13:35 UTC, Dukc wrote:

 On Thursday, 8 August 2019 at 12:10:09 UTC, Mike Parker wrote:

 This is the feedback thread for the first round of Community 
 Review for DIP 1022, "foreach auto ref":


 Author here, AMA/Destroy!


Just a note: the patch I submitted aimed to fix a "lifecycle" 
problem, I wasn't trying to fix the behaviour. foreach+ref with 
rvalues was creating a copy of each element without calling its 
dtor.

Good work, I hope this dip will be approved!
Aug 08 2019
parent Dukc <ajieskola gmail.com> writes:
On Thursday, 8 August 2019 at 12:36:07 UTC, Andrea Fontana wrote:

 Just a note: the patch I submitted aimed to fix a "lifecycle" 
 problem, I wasn't trying to fix the behaviour. foreach+ref with 
 rvalues was creating a copy of each element without calling its 
 dtor.

 Good work, I hope this dip will be approved!


Um, I understood that this is what you PR finally did, but didn't 
you intend to disable foreach-ref with rvalues at some point 
(when Andrei made the comment I mentioned)?
Aug 08 2019
prev sibling next sibling parent reply jmh530 <john.michael.hall gmail.com> writes:
On Thursday, 8 August 2019 at 12:13:35 UTC, Dukc wrote:

 On Thursday, 8 August 2019 at 12:10:09 UTC, Mike Parker wrote:

 This is the feedback thread for the first round of Community 
 Review for DIP 1022, "foreach auto ref":


 Author here, AMA/Destroy!


My sense is that there should be a very simple example up top 
that makes clear what the problem with the current behavior 
(simpler than the DynamicArray example later).
Aug 08 2019
parent Dukc <ajieskola gmail.com> writes:
On Thursday, 8 August 2019 at 15:13:27 UTC, jmh530 wrote:

 My sense is that there should be a very simple example up top 
 that makes clear what the problem with the current behavior 
 (simpler than the DynamicArray example later).


Perhaps. The dynamic array example would be more practical, while 
the top example would focus on being just dead simple.
Aug 08 2019
prev sibling parent reply Manu <turkeyman gmail.com> writes:
On Thu, Aug 8, 2019 at 5:15 AM Dukc via Digitalmars-d
<digitalmars-d puremagic.com> wrote:

 On Thursday, 8 August 2019 at 12:10:09 UTC, Mike Parker wrote:

 This is the feedback thread for the first round of Community
 Review for DIP 1022, "foreach auto ref":


 Author here, AMA/Destroy!


This DIP contains obvious improvement, and should absolutely move
forward, but there are some bugs in this DIP.

"This DIP proposes that foreach loops with the ref keyword applied to
the element variable should only be legal when the elements of the
range have memory addresses."

We create temporaries for ref-args now
(https://www.youtube.com/watch?v=aRvu2JGGn6E), so capturing a ref loop
counter is fine in all cases. But, your DIP is still important, that's
just not the reason.

"It also proposes that current usages ref may be replaced with auto
ref to retain the current behavior."

I'm not sure what you mean by 'replaced', auto ref would simply make
the auto-ref semantic available to foreach loops, which is useful.
'auto ref' in this context only really has anything to do with move
semantics... but I can't see that mentioned anywhere in your DIP.

"This is to ensure that foreach will iterate by reference, while still
allowing iteration over a range of non-copyable elements without
explicit need to adapt the code[1]."

I don't understand this sentence at all... what does it mean?

"...then if one or more elements of aggregate are rvalues[2], a
deprecation message must be emitted including the suggestion to
annotate loopVariable1 with the auto ref keyword instead."

This is wrong, it shouldn't be deprecated; it was only just recently
fixed after a decade of really hard work! (again, see the youtube link
above)

"[...]"

Lots of text... I mean, `auto ref` should be supported in foreach
certainly, but your rationale is all wrong. This DIP is mostly wrong,
even though the thing it wants is correct, the DIP just isn't sure why
it wants it.
Aug 09 2019
parent reply Dukc <ajieskola gmail.com> writes:
On Friday, 9 August 2019 at 18:17:50 UTC, Manu wrote:

 [snip]


Wow! It seems that were on totally different pages here! I 
definitely have to check your link and think long and deeply 
about your comment before I know how to even begin my response. 
I'll likely make it tomorrow.

Looks like an opening for an interesting debate, thanks for 
popping up!
Aug 09 2019
parent reply Manu <turkeyman gmail.com> writes:
On Fri, Aug 9, 2019 at 2:00 PM Dukc via Digitalmars-d
<digitalmars-d puremagic.com> wrote:

 On Friday, 9 August 2019 at 18:17:50 UTC, Manu wrote:

 [snip]


 Wow! It seems that were on totally different pages here! I
 definitely have to check your link and think long and deeply
 about your comment before I know how to even begin my response.
 I'll likely make it tomorrow.

 Looks like an opening for an interesting debate, thanks for
 popping up!


Sorry if that sounded blunt.

Let me try and be more clear; by comparison, we can consider C++,
which has value, ref, and 'auto ref' foreach loops.
All 3 are valid operations, and we have the first 2, but not the 3rd.

C++: for (auto i : things)
D: foreach (i; things)

C++: for (const auto& i : things)
D: foreach (ref i; things) // <- this didn't used to work, but ref
FINALLY accepts rvalues by creating a temporary. I suspect you may be
confusing this case with your auto-ref proposal.

C++: for (auto&& i : things)
D: foreach (auto ref i; things)  // <- you are proposing this, and I
agree, we want this. I've wanted it many times.

I suspect from reading that you are trying to solve the second case
with this DIP, but it's already solved by Andrei's work (which he
presented at dconf).
What this DIP should actually solve though, in the 3rd case. We need that.
Aug 09 2019
parent reply Dukc <ajieskola gmail.com> writes:
On Friday, 9 August 2019 at 21:55:45 UTC, Manu wrote:

 Sorry if that sounded blunt.


No, not at all. I was just surprised that you questioned my core 
resoning, yet wanted the dip to move forward.


 Let me try and be more clear; by comparison, we can consider 
 [Snip]
 
 I suspect from reading that you are trying to solve the second 
 case
 with this DIP, but it's already solved by Andrei's work (which 
 he
 presented at dconf).
 What this DIP should actually solve though, in the 3rd case. We 
 need that.


Hmm, you want to retain the current behaviour of `ref`? This 
confuses me, because `auto ref` I'm proposing has exactly the 
same semantics as `ref` does now, yet you also want it to be 
included.

My guess is that you're thinking the benefit of `auto ref` to be 
allowing to handle rvalue ranges without becoming a pointer 
internally, thus being more efficient in at least some cases. The 
rest of my answer assumes that was your point.

I disagree. I am not trying to target efficiency issues a all. If 
my problem was that, I'd simply propose foreach `ref` to use 
rvalue copies directly, not via internal pointers. Unlike in C++, 
refs and the values they point to are indistinguishable from D 
user perspective, so that is just an implementation detail that 
would probably get through even without a DIP. Unless it's 
already done.

The problem I'm targeting is that currently the compiler will not 
complain, if the user, when expecting to mutate range elements, 
iterates by `ref` over foreach. But since I know there's need for 
the notion I'm deprecating, I also suggest `auto ref` for the 
behaviour.

I know this is a kind of a jerk change, as it forces to change 
code. But I thought that the migration path is so simple (just 
changing `ref` to `auto ref` where you get deprecation messages) 
that avoiding it (see alternative 1 of the DIP) is not worth 
having a language inconsistency.
Aug 10 2019
next sibling parent Dukc <ajieskola gmail.com> writes:
On Saturday, 10 August 2019 at 08:09:08 UTC, Dukc wrote:

 The problem I'm targeting is that currently the compiler will 
 not complain, if the user, when expecting to mutate range 
 elements, iterates by `ref` over foreach.


Meant: iterates by `ref` `foreach` over a range of rvalues
Aug 10 2019
prev sibling next sibling parent Dukc <ajieskola gmail.com> writes:
On Saturday, 10 August 2019 at 08:09:08 UTC, Dukc wrote:

 The problem I'm targeting is that currently the compiler will 
 not complain, if the user, when expecting to mutate range 
 elements, iterates by `ref` over foreach.


Meant: iterates by `ref` `foreach` over a range of rvalues
Aug 10 2019
prev sibling parent reply Manu <turkeyman gmail.com> writes:
On Sat, Aug 10, 2019 at 1:11 AM Dukc via Digitalmars-d
<digitalmars-d puremagic.com> wrote:

 On Friday, 9 August 2019 at 21:55:45 UTC, Manu wrote:

 Sorry if that sounded blunt.


 No, not at all. I was just surprised that you questioned my core
 resoning, yet wanted the dip to move forward.


 Let me try and be more clear; by comparison, we can consider
 [Snip]

 I suspect from reading that you are trying to solve the second
 case
 with this DIP, but it's already solved by Andrei's work (which
 he
 presented at dconf).
 What this DIP should actually solve though, in the 3rd case. We
 need that.


 Hmm, you want to retain the current behaviour of `ref`? This
 confuses me, because `auto ref` I'm proposing has exactly the
 same semantics as `ref` does now, yet you also want it to be
 included.

 My guess is that you're thinking the benefit of `auto ref` to be
 allowing to handle rvalue ranges without becoming a pointer
 internally, thus being more efficient in at least some cases. The
 rest of my answer assumes that was your point.

 I disagree. I am not trying to target efficiency issues a all. If
 my problem was that, I'd simply propose foreach `ref` to use
 rvalue copies directly, not via internal pointers. Unlike in C++,
 refs and the values they point to are indistinguishable from D
 user perspective, so that is just an implementation detail that
 would probably get through even without a DIP. Unless it's
 already done.

 The problem I'm targeting is that currently the compiler will not
 complain, if the user, when expecting to mutate range elements,
 iterates by `ref` over foreach. But since I know there's need for
 the notion I'm deprecating, I also suggest `auto ref` for the
 behaviour.

 I know this is a kind of a jerk change, as it forces to change
 code. But I thought that the migration path is so simple (just
 changing `ref` to `auto ref` where you get deprecation messages)
 that avoiding it (see alternative 1 of the DIP) is not worth
 having a language inconsistency.


I'm thought about it some more, and I think you're right about the deprecation.
I think I can agree that the probability of confusion + mistake where
a ref to temporary exists is higher weighted than the uniformity with
function calling.
I tend to hate special-case rules (in this case, loop counters
behaving differently than function arguments), but I think I'm
persuaded here.

I support your text unamended ;) .. we've needed `auto ref` in foreach
for a long time.

I'm curious about this line: "It should be allowed in static foreach,
but with no effect, as elements of compile-time aggregates can never
be lvalues."

Is that true? I don't think there's any reason they can't be lvalues.
But what I'm trying to get my head around is if the loop counter for
`static foreach` is a variable at all, or if it's an alias?

I think there's inherent complexity here, because there's a lot of
cases... and some should work with auto ref, but not all.

Okay, here's some experiment cases:

  int x = 1;
  static foreach (i; AliasSeq!(10, x, 20))
      pragma(msg, __traits(isRef, i));


 false false false


In this case, `i` is an alias. Ref-ness doesn't mean anything here.

This can't work:

  int x = 1;
  static foreach (int i; AliasSeq!(10, x, 20))
      pragma(msg, __traits(isRef, i));


 Error: can't initialise `i` because `x` is a runtime value


Because it tries to initialise an int from elements of tuple, and `x`
is a runtime variable.

This could work if ref was treated the same way as parameter arguments:

  int x = 1;
  static foreach (ref int i; AliasSeq!(10, x, 20))
      pragma(msg, __traits(isRef, i));


 true true true


In that case, rvalues (which can be determined at compile time) would
populate temporaries, the same way as function parameters.
But according to the rules in your dip, we don't allow that, and I
conceded above.

This should work though:

  int x = 1;
  static foreach (ref int i; AliasSeq!(x, x, x))
      pragma(msg, __traits(isRef, i));


 true true true


All elements are lvalues, so we accept `ref`.

Finally, this should work as expected:

  int x = 1;
  static foreach (auto ref int i; AliasSeq!(10, x, 20))
      pragma(msg, __traits(isRef, i));


 false true false


Functionally identical to the first case, except the interesting
distinction that `i` contains information which can be used in
introspection logic, which is almost always present in static foreach
bodies.
I believe this should be allowed AND behave as expected; that is; `i`
is a ref when encountering an lvalue.
If it does not behave in this way, it should not be allowed.


Now, there's a different set of cases, where you call static foreach
over a not-a-tuple:

  static foreach (i; [10, 20, 30])
      pragma(msg, __traits(isRef, i));


 false false false


In this case, `i` is not an alias like the previous experiment, it is
inferred to be `int`.
Should be the same as `static foreach (int i; ...)`, right?

  static foreach (ref i; [10, 20, 30])
      pragma(msg, __traits(isRef, i));


 true true true


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


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


And:

  int[3] x = [10, 20, 30];
  static foreach (auto ref i; x)
      pragma(msg, __traits(isRef, i));


 true true true


These should work as expected.


TL;DR, your sentence "It should be allowed in static foreach, but with
no effect" should be removed, and if you want to detail the expected
semantics with `static foreach`, that might be a good idea.
Aug 10 2019
next sibling parent reply Nick Treleaven <nick geany.org> writes:
On Saturday, 10 August 2019 at 21:42:00 UTC, Manu wrote:

   static foreach (ref i; [10, 20, 30])
       pragma(msg, __traits(isRef, i));


 true true true


 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.


It doesn't need temporaries, [2][0] is an lvalue with existing 
dmd:

void f(ref int i);
void main(){
     f([2][0]); //compiles
}


 TL;DR, your sentence "It should be allowed in static foreach, 
 but with no effect" should be removed, and if you want to 
 detail the expected semantics with `static foreach`, that might 
 be a good idea.


+1
Aug 11 2019
next sibling parent Manu <turkeyman gmail.com> writes:
On Sun, Aug 11, 2019 at 7:55 AM Nick Treleaven via Digitalmars-d
<digitalmars-d puremagic.com> wrote:

 On Saturday, 10 August 2019 at 21:42:00 UTC, Manu wrote:

   static foreach (ref i; [10, 20, 30])
       pragma(msg, __traits(isRef, i));


 true true true


 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.


 It doesn't need temporaries, [2][0] is an lvalue with existing
 dmd:

 void f(ref int i);
 void main(){
      f([2][0]); //compiles
 }


Oh yeah... I forgot about that. Caused me so much trouble over the years.
So bad!


 TL;DR, your sentence "It should be allowed in static foreach,
 but with no effect" should be removed, and if you want to
 detail the expected semantics with `static foreach`, that might
 be a good idea.


 +1
Aug 11 2019
prev sibling parent Dukc <ajieskola gmail.com> writes:
On Sunday, 11 August 2019 at 14:52:08 UTC, Nick Treleaven wrote:

 void f(ref int i);
 void main(){
     f([2][0]); //compiles
 }


Huh? What happens here?
Aug 13 2019
prev sibling parent reply Dukc <ajieskola gmail.com> writes:
On Saturday, 10 August 2019 at 21:42:00 UTC, Manu wrote:


 I tend to hate special-case rules (in this case, loop counters
 behaving differently than function arguments), but I think I'm
 persuaded here.


Not sure what you mean here... example?


 Okay, here's some experiment cases:

   int x = 1;
   static foreach (i; AliasSeq!(10, x, 20))
       pragma(msg, __traits(isRef, i));


 false false false


 In this case, `i` is an alias. Ref-ness doesn't mean anything 
 here.


I thought that would compile only without the `static` keyword. 
Gotta investigate. If it compiles, I'm going to say it should 
behave exactly as written without the `static` keyword.

Same answer for the rest of examples with tuples.


 Now, there's a different set of cases, where you call static 
 foreach over a not-a-tuple:

   static foreach (i; [10, 20, 30])
       pragma(msg, __traits(isRef, i));


 false false false


 In this case, `i` is not an alias like the previous experiment, 
 it is
 inferred to be `int`.
 Should be the same as `static foreach (int i; ...)`, right?

   static foreach (ref i; [10, 20, 30])
       pragma(msg, __traits(isRef, i));


 true true true




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.


 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.


 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




I don't think that compiles. x is not an enum value. But I might 
be wrong, I'll have to recheck this also.


 And:

   int[3] x = [10, 20, 30];
   static foreach (auto ref i; x)
       pragma(msg, __traits(isRef, i));


 true true true




Same as above. If x was enum, then it should not compile with 
`ref` and should return false false false with `auto ref`.


 TL;DR, your sentence "It should be allowed in static foreach, 
 but with no effect" should be removed, and if you want to 
 detail the expected semantics with `static foreach`, that might 
 be a good idea.


I will check those examples that I thought won't compile do. If 
they do, I'll be more explicit.
Aug 13 2019
parent reply Manu <turkeyman gmail.com> writes:
On Tue, Aug 13, 2019 at 6:50 AM Dukc via Digitalmars-d
<digitalmars-d puremagic.com> wrote:

 On Saturday, 10 August 2019 at 21:42:00 UTC, Manu wrote:


 I tend to hate special-case rules (in this case, loop counters
 behaving differently than function arguments), but I think I'm
 persuaded here.


 Not sure what you mean here... example?


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.


 Okay, here's some experiment cases:

   int x = 1;
   static foreach (i; AliasSeq!(10, x, 20))
       pragma(msg, __traits(isRef, i));


 false false false


 In this case, `i` is an alias. Ref-ness doesn't mean anything
 here.


 I thought that would compile only without the `static` keyword.
 Gotta investigate. If it compiles, I'm going to say it should
 behave exactly as written without the `static` keyword.


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.


 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.


 Now, there's a different set of cases, where you call static
 foreach over a not-a-tuple:

   static foreach (i; [10, 20, 30])
       pragma(msg, __traits(isRef, i));


 false false false


 In this case, `i` is not an alias like the previous experiment,
 it is
 inferred to be `int`.
 Should be the same as `static foreach (int i; ...)`, right?

   static foreach (ref i; [10, 20, 30])
       pragma(msg, __traits(isRef, i));


 true true true




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


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


 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




 I don't think that compiles. x is not an enum value. But I might
 be wrong, I'll have to recheck this also.


Why not? It's an alias... it should compile just fine.


 And:

   int[3] x = [10, 20, 30];
   static foreach (auto ref i; x)
       pragma(msg, __traits(isRef, i));


 true true true




 Same as above. If x was enum, then it should not compile with
 `ref` and should return false false false with `auto ref`.


`x` is not an enum, it's a local.
The cases I propose to consider are exactly what I wrote, not some other thing.

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));
}

And there's another interesting case of 'weird shit' here too. That
second one should output `false`, not `false false false`; the
pragma(msg) should only evaluate once, because the non-static foreach
shouldn't be expanding... but strangely, runtime foreach when given a
tuple behaves almost like a static foreach, but the loop counter is
not an alias for the tuple element, it is rather an `auto i =
element[n]` copy.
I actually think this form of runtime foreach should be deprecated, if
you want to do this with runtime foreach, wrap the tuple in `[ ]`.

Here there be dargons!


 TL;DR, your sentence "It should be allowed in static foreach,
 but with no effect" should be removed, and if you want to
 detail the expected semantics with `static foreach`, that might
 be a good idea.


 I will check those examples that I thought won't compile do. If
 they do, I'll be more explicit.


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.
Aug 13 2019
next sibling parent reply Dukc <ajieskola gmail.com> writes:
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.
Aug 14 2019
parent reply Manu <turkeyman gmail.com> writes:
On Wed, Aug 14, 2019 at 8:40 AM Dukc via Digitalmars-d
<digitalmars-d 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.
Aug 14 2019
parent reply Dukc <ajieskola gmail.com> writes:
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 

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.
Aug 15 2019
parent reply Manu <turkeyman gmail.com> writes:
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

 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.
Aug 15 2019
parent reply Dukc <ajieskola gmail.com> writes:
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".
Aug 17 2019
parent reply Manu <turkeyman gmail.com> writes:
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 :)
Aug 17 2019
parent Dukc <ajieskola gmail.com> writes:
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.
Aug 18 2019
prev sibling parent Nick Treleaven <nick geany.org> writes:
On Tuesday, 13 August 2019 at 23:34:32 UTC, Manu wrote:

 On Tue, Aug 13, 2019 at 6:50 AM Dukc via Digitalmars-d 
 <digitalmars-d puremagic.com> wrote:

   static foreach (ref i; [10, 20, 30])
       pragma(msg, __traits(isRef, i));


 true true true




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


An array literal is an rvalue:

void f(ref int[] a);
void main(){
     f([10, 20, 30]); // error
}

error: cannot pass rvalue argument `[10, 20, 30]` of type `int[]` 
to parameter `ref int[] a`


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


An array literal is an rvalue, but indexing an array literal is 
an lvalue. So the above should print 'true' for each pragma.
Aug 14 2019
prev sibling next sibling parent reply rikki cattermole <rikki cattermole.co.nz> writes:
- In the first alternative a disadvantage is listed but no advantages. 
An advantage of it is that there would be no breaking changes unlike the 
proposed solution which has an unknown amount of breakage.

If we could get an idea of the severity (1-2 modules vs half vs almost 
all of Phobos) of the breakage being proposed, that would great.

- What is the interaction with indexes? No mention of them, nor example.

- The examples you have may look good for an implementer, but for 
everybody else perhaps some examples using concrete types that ignore 
the foreach body would be a good edition. Just to make it clear what 
we're getting out of it. I.e.

RangeWithoutRefFront expr; // T front()

foreach(v; expr) // ok
foreach(ref v; expr) // Success but will be error after deprecation
foreach(auto ref v; expr) // ok
Aug 08 2019
parent reply Dukc <ajieskola gmail.com> writes:
On Thursday, 8 August 2019 at 12:38:51 UTC, rikki cattermole 
wrote:

 - In the first alternative a disadvantage is listed but no 
 advantages. An advantage of it is that there would be no 
 breaking changes unlike the proposed solution which has an 
 unknown amount of breakage.


Read the paragraph slower :D.


 If we could get an idea of the severity (1-2 modules vs half vs 
 almost all of Phobos) of the breakage being proposed, that 
 would great.


Remember that nothing will break before deprecation period ends. 
If it proves to be too bad we can back up and go with alternative 



 - What is the interaction with indexes? No mention of them, nor 
 example.


Yes, this is likely worth an explicit mention. Same as with the 
equivalent regular or ref foreach, depending on which is chosen 
according to the rules menitoned on the DIP.


 - The examples you have may look good for an implementer, but 
 for everybody else perhaps some examples using concrete types 
 that ignore the foreach body would be a good edition. Just to 
 make it clear what we're getting out of it. I.e.

 RangeWithoutRefFront expr; // T front()

 foreach(v; expr) // ok
 foreach(ref v; expr) // Success but will be error after 
 deprecation
 foreach(auto ref v; expr) // ok


Isn't the EMSI-containers example already like yours? Or did you 
mean that your example as how-not-to example?
Aug 08 2019
next sibling parent reply rikki cattermole <rikki cattermole.co.nz> writes:
On 09/08/2019 12:52 AM, Dukc wrote:

 On Thursday, 8 August 2019 at 12:38:51 UTC, rikki cattermole wrote:

 - In the first alternative a disadvantage is listed but no advantages. 
 An advantage of it is that there would be no breaking changes unlike 
 the proposed solution which has an unknown amount of breakage.


 
 Read the paragraph slower :D.


Darn it, you're right!


 If we could get an idea of the severity (1-2 modules vs half vs almost 
 all of Phobos) of the breakage being proposed, that would great.


 
 Remember that nothing will break before deprecation period ends. If it 



If we can do a little bit of research now to prevent changing an 
implementation after a DIP is accepted, we probably should.

When no code is written, changing tactic is cheap :)


 - What is the interaction with indexes? No mention of them, nor example.


 
 Yes, this is likely worth an explicit mention. Same as with the 
 equivalent regular or ref foreach, depending on which is chosen 
 according to the rules menitoned on the DIP.
 

 - The examples you have may look good for an implementer, but for 
 everybody else perhaps some examples using concrete types that ignore 
 the foreach body would be a good edition. Just to make it clear what 
 we're getting out of it. I.e.

 RangeWithoutRefFront expr; // T front()

 foreach(v; expr) // ok
 foreach(ref v; expr) // Success but will be error after deprecation
 foreach(auto ref v; expr) // ok


 
 Isn't the EMSI-containers example already like yours? Or did you mean 
 that your example as how-not-to example?


The EMSI-containers example while useful, has a lot of extra code that 
you have to think about. Or at least it does for me.

Basically what I would like to have is an expression +
foreach statements that are current (and will remain) +
foreach statements that will no longer be valid +
foreach statements that will become valid

Essentially the bare bones of what is changing and something to compare 
against. So if the only thing you look at within the DIP, this will tell 
you pretty much what you need to know.
Aug 08 2019
parent Dukc <ajieskola gmail.com> writes:
On Thursday, 8 August 2019 at 13:06:28 UTC, rikki cattermole 
wrote:

 On 09/08/2019 12:52 AM, Dukc wrote:

 On Thursday, 8 August 2019 at 12:38:51 UTC, rikki cattermole


 If we can do a little bit of research now to prevent changing 
 an implementation after a DIP is accepted, we probably should.

 When no code is written, changing tactic is cheap :)


Yes, but I can't, quickly at least, come up with any way to 
reveal those loops without touching the compiler.



 The EMSI-containers example while useful, has a lot of extra 
 code that you have to think about. Or at least it does for me.

 Basically what I would like to have is an expression +
 foreach statements that are current (and will remain) +
 foreach statements that will no longer be valid +
 foreach statements that will become valid


So it's mainly about minimizing the example more? Fair argument. 
I'll have to try.
Aug 08 2019
prev sibling parent Dukc <ajieskola gmail.com> writes:
On Thursday, 8 August 2019 at 12:52:43 UTC, Dukc wrote:

 On Thursday, 8 August 2019 at 12:38:51 UTC, rikki cattermole 
 wrote:

 If we could get an idea of the severity (1-2 modules vs half 
 vs almost all of Phobos) of the breakage being proposed, that 
 would great.


 Remember that nothing will break before deprecation period 
 ends. If it proves to be too bad we can back up and go with 



Just realized how stupid this would be in case of Phobos, if just 
about standard library usage will spit deprecation messages all 
over the place.

Phobos has to be migrated with a compiler fork that has this 
feature implemented (running it against the standard test suite 
should reveal a good enough portion of offending loops), and if 
it proves to be too bad to migrate them behind the scenes, then a 
preview switch.

Sounds unlikely to me that a switch would be needed, since it's 
just about changing `ref`s to `auto ref`s. So probably not worth 
it to add anything extra mentions of that to the DIP. But the 
main contributors are better at estimating that than me -any 
opinions?
Aug 08 2019
prev sibling parent Dukc <ajieskola gmail.com> writes:
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.
Aug 23 2019