foreach bug, or shoddy docs, or something, or both.

[Jonathan M Davis]

On Sunday, December 10, 2017 02:02:31 Dave Jones via Digitalmars-d wrote:
> Foreach ignores modification to the loop variable...
>
> import std.stdio;
>
> void main() {
>      int[10] foo = 10;
>
>      foreach (i; 0..10) // writes '10' ten times
>      {
>          writeln(foo[i]);
>          if (i == 3) i+=2;
>      }
> }
>
>  From the docs...
>
> "ForeachType declares a variable with either an explicit type, or
> a type inferred from LwrExpression and UprExpression. <**snip**>
> If Foreach is foreach, then the variable is set to LwrExpression,
> then incremented at the end of each iteration."
>
> That's clearly not what is happening, yes we get a variable, but
> either its a copy of the actual loop variable, or something else
> is going on. Because if we got the actual variable then
> modifications to it would not be lost at the end of the current
> iteration.
>
> My opinion is the it should pick up the modification. I think
> people expect the foreach form to be a shorthand for a regular
> for loop.
>
> Failing that it should be an error to write to the loop variable.
>
> An at the least it should be explained in the documentation that
> actually you get a copy of the loop variable so modifying it is a
> waste of time.

https://issues.dlang.org/show_bug.cgi?id=14984

Honestly, it would have never occurred to me to try and modify the variables
declared in the foreach like that, and my first inclination is to think that
it shouldn't be allowed, but thinking it through, and looking at how things
actually work, I don't think that the current behavior is really a problem.

Given what the variant of foreach you used would presumably be lowered to
something like

for(int i = 0; i < 10; ++i)
{
    ...
}

it wouldn't have surprised me if modifying i would have worked, but clearly,
that's not quite what's happening, and in the general case, it doesn't make
much sense to expect that mucking with the loop variable would affect the
loop itself.

foreach(e; myRange)
{
    ...
}

gets lowered to something like

for(auto __c = range; !__c.empty; __c.popFront())
{
    auto e = __c.front;
    ...
}

and altering e in tha case should probably be fine, but it wouldn't affect
the iteration at all either.

However, when iterating over arrays or using foreach like you tried, it
turns out that you can use ref to control things. e.g.

foreach(i, ref e; arr)
{
}

will allow you to modify the elements in the array, and

foreach(ref i, e; arr)
{
}

will actually allow you to increment i to skip elements. And if you do

foreach(ref i; 0 .. 10)
{
}

then you can increment i the way you want to. So, maybe modifying the loop
variable in something like

foreach(i; 0 .. 10)
{
    ++i;
}

should be disallowed, but I don't really think that the current behavior is
really a problem either so long as it's properly documented. Modifying i
doesn't really cause any problems and ref or the lack thereof allows you to
control whether it affects the loop. Ultimaetly, it looks to me like what we
currently have is reasonably well designed. It doesn't surprise me in the
least if it's not well-documented though.

- Jonathan M Davis