http://d.puremagic.com/issues/show_bug.cgi?id=6652


Kenji Hara <k.hara.pg@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code


--- Comment #10 from Kenji Hara <k.hara.pg@gmail.com> 2012-06-15 19:40:31 PDT ---
(In reply to comment #9)
> (In reply to comment #8)
> 
> > To reduce breaking of existing codes,
> > 1. Warn to modifying loop variable in foreach body.
> >    It is shown only when -w switch is specified.
> > 2. Deprecate modifying loop variable in foreach body.
> >    If user specifies -d switch, it is allowed.
> > 3. Allow modifying loop variable in foreach body, and it does not affect to
> >    the number of iterations of the loop.
> 
> This is great, you are the best Kenji Hara.
> 
> I prefer the number 2. I think it breaks none of my programs.

They are the phases to change behavior. I think we should allow modifying loop variable in foreach body, but it should not affect to iteration.

> The number 3 is a trap, because it silently changes the semantics of old D code. And it's bug-prone for new D programmers too because they can change the variable by mistake. Generally immutable variables are safer.

#3 is a goal.

> Are you able and willing to compile the whole Phobos with the option number 2? So we can see how often Phobos code change the foreach-on-range iteration variable.

http://d.puremagic.com/test-results/pulls.ghtml
See auto tester. With all pull request, Phobos compile succeeds. So there is no
code that changes the foreach-on-range iteration variable.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=6652


Ryuichi OHORI <r.97all@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |r.97all@gmail.com


--- Comment #11 from Ryuichi OHORI <r.97all@gmail.com> 2012-06-15 19:57:43 PDT ---
(In reply to comment #9)
> I prefer the number 2. I think it breaks none of my programs.
> 
> The number 3 is a trap, because it silently changes the semantics of old D code. And it's bug-prone for new D programmers too because they can change the variable by mistake. Generally immutable variables are safer.

In my point of view, as a newcomer to D, more bug-prone is the current behavior.

Foreach statement provides iteration over arrays, ranges, etc, and notation of range "0..n" also *looks like* a collection. So, foreach range statements should work like foreach over collection.

I have wrote to stuck in my program:
foreach (i; 0..M^^n)
{
    foreach (j; 0..n)
    {
        a[j] = i % M;
        i /= M;
    }
    // operations which use a but i
}
which I wrote in Python before:
for i in range(M**n):
    for j in range(n):
        a[j] = i % M
        i /= M
    # operations which use a but i
and was sad to see an infinite loop.

Even a new programmer *intends* to change the value of i when changing, if not
just a typo.
If someone want to affect loop, s/he can write
i = 0;
while (i < 10)
{
    // operations which change the value of i
    i += 1;
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=6652



--- Comment #12 from bearophile_hugs@eml.cc 2012-06-16 05:07:21 PDT ---
(In reply to comment #10)

> They are the phases to change behavior.

I see.


> I think we should allow modifying loop
> variable in foreach body, but it should not affect to iteration.

Generally changing the iteration variable isnt't a very good idea. It looks bug-prone, like modifying function arguments inside functions :-)

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


> So there is no
> code that changes the foreach-on-range iteration variable.

Good.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=6652



--- Comment #13 from bearophile_hugs@eml.cc 2012-06-16 05:14:39 PDT ---
If phase 3 will be accepted, I hope this syntax too will be accepted:

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

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=6652



--- Comment #14 from bearophile_hugs@eml.cc 2012-06-16 05:15:07 PDT ---
(In reply to comment #11)

> In my point of view, as a newcomer to D, more bug-prone is the current behavior.

Of course. But here the comparison wasn't between the current behavour and the phase 3. It was mostly a comparison between the phase 2 and phase 3.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=6652



--- Comment #15 from Kenji Hara <k.hara.pg@gmail.com> 2012-06-16 05:27:32 PDT ---
(In reply to comment #13)
> If phase 3 will be accepted, I hope this syntax too will be accepted:
> 
> foreach (const i; 0 .. 10) {}

I think it should be allowed.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=6652



--- Comment #16 from Ryuichi OHORI <r.97all@gmail.com> 2012-06-16 09:00:32 PDT ---
(In reply to comment #14)
> > In my point of view, as a newcomer to D, more bug-prone is the current behavior.

> Of course. But here the comparison wasn't between the current behavour and the phase 3. It was mostly a comparison between the phase 2 and phase 3.

What followed it was my opinion to it:
> > Even a new programmer *intends* to change the value of i when changing, if not just a typo.
The problem is that changing the value of a variable affects loop, not changing itself.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=6652



--- Comment #17 from bearophile_hugs@eml.cc 2012-06-17 15:31:45 PDT ---
Just a note.

void main() {
    import std.stdio;
    auto array = [10, 20, 30, 40, 50];
    foreach (i, item; array) {
        writeln(item);
        i++;
    }
}


Currently it prints:

10
30
50

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=6652



--- Comment #18 from dawg@dawgfoto.de 2012-06-19 06:10:15 PDT ---
>How about?
Sounds great. It doesn't break code and allows us to fix this finally.

>foreach (i, item; array)
Yeah, it should apply to the index variable as well.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=6652



--- Comment #19 from Kenji Hara <k.hara.pg@gmail.com> 2012-06-21 08:32:51 PDT ---
(In reply to comment #18)
> >How about?
> Sounds great. It doesn't break code and allows us to fix this finally.
> 
> >foreach (i, item; array)
> Yeah, it should apply to the index variable as well.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------