View mode: basic / threaded / horizontal-split · Log in · Help
June 16, 2012
[Issue 6652] foreach parameter with number range is always ref
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: -------
June 16, 2012
[Issue 6652] foreach parameter with number range is always ref
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: -------
June 16, 2012
[Issue 6652] foreach parameter with number range is always ref
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: -------
June 16, 2012
[Issue 6652] foreach parameter with number range is always ref
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: -------
June 16, 2012
[Issue 6652] foreach parameter with number range is always ref
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: -------
June 16, 2012
[Issue 6652] foreach parameter with number range is always ref
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: -------
June 16, 2012
[Issue 6652] foreach parameter with number range is always ref
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: -------
June 17, 2012
[Issue 6652] foreach parameter with number range is always ref
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: -------
June 19, 2012
[Issue 6652] foreach parameter with number range is always ref
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: -------
June 21, 2012
[Issue 6652] foreach parameter with number range is always ref
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: -------
1 2 3 4
Top | Discussion index | About this forum | D home