www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - foreach bug, or shoddy docs, or something, or both.

reply Dave Jones <dave jones.com> writes:
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.
Dec 09 2017
parent reply Jonathan M Davis <newsgroup.d jmdavisprog.com> writes:
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
Dec 09 2017
parent Dave Jones <dave jones.com> writes:
On Sunday, 10 December 2017 at 02:31:47 UTC, Jonathan M Davis 
wrote:
 On Sunday, December 10, 2017 02:02:31 Dave Jones via 
 Digitalmars-d wrote:
 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.
Its not something I normally do but I was porting some C++ code and changed the fors to foreaches. Just it took a while to figure out what was going wrong.
 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.
Well it's just one of those "APIs should be hard to use in the wrong way" kinda things to me. Either the loop var should be actually the loop var rather than a copy, or it should be an error to modify it. Anyway it's not a bit deal like you say, but should be better explained in the docs at least. Thanks,
Dec 10 2017