Thread overview
[Issue 16149] foreach_reverse can't handle index variable of type int
Jun 10, 2016
Ketmar Dark
Jun 10, 2016
Acer.Yang
Jun 11, 2016
Ketmar Dark
Jun 12, 2016
Ketmar Dark
June 10, 2016
https://issues.dlang.org/show_bug.cgi?id=16149

Ketmar Dark <ketmar@ketmar.no-ip.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ketmar@ketmar.no-ip.org

--- Comment #1 from Ketmar Dark <ketmar@ketmar.no-ip.org> ---
...and, techincally, it shouldn't.

first case (roughly) does: `int i; while (i < s.length) ...` which is perfectly
legal. but second does: `int i = s.length;`, which requires conversion from
`size_t` (of type `ulong` on 64-bit arch) to `int`, which is illegal.

i'd say that first case should be forbidden too, but the seconds case -- in my opinion -- doesn't require fixing.

--
June 10, 2016
https://issues.dlang.org/show_bug.cgi?id=16149

--- Comment #2 from Acer.Yang <yangacer@gmail.com> ---
(In reply to Ketmar Dark from comment #1)
> ...and, techincally, it shouldn't.
> 
> first case (roughly) does: `int i; while (i < s.length) ...` which is
> perfectly legal. but second does: `int i = s.length;`, which requires
> conversion from `size_t` (of type `ulong` on 64-bit arch) to `int`, which is
> illegal.
> 
> i'd say that first case should be forbidden too, but the seconds case -- in my opinion -- doesn't require fixing.

I agree that the first case should be forbidden(since signed/unsigned comparison is not safe per my eyes).

--
June 10, 2016
https://issues.dlang.org/show_bug.cgi?id=16149

Steven Schveighoffer <schveiguy@yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy@yahoo.com
                 OS|Linux                       |All

--- Comment #3 from Steven Schveighoffer <schveiguy@yahoo.com> ---
I think it's a decent enhancement request. Most arrays are not going to exceed int.max, and if they do, well, you wrote the stupid code!

For example, foreach(ubyte i, v; iota(500).array) is going to compile, and get
into an infinite loop. I think you should be able to make the same dumb mistake
with foreach_reverse :)

--
June 11, 2016
https://issues.dlang.org/show_bug.cgi?id=16149

--- Comment #4 from Ketmar Dark <ketmar@ketmar.no-ip.org> ---
while deep inside of me i think the same thing, it still sounds wrong, i believe. if we'll enable such use of `foreach_reverse`, this will be the only exception in D where it allows assignment of `ulong` to `int` without explicit cast. of course, this will allow me to write portable code without `cast(uint)` on each `.length` usage, but... but i'd rather keep writing casts. ;-)

--
June 12, 2016
https://issues.dlang.org/show_bug.cgi?id=16149

--- Comment #5 from Steven Schveighoffer <schveiguy@yahoo.com> ---
int x = a.length would continue to be invalid (on 64-bit CPU). It's just for
foreach_reverse.

What feels wrong is the OP's code that I can foreach some array in a certain way, but not foreach reverse the identical thing.

My point was that foreach_reverse with a smaller index is valid for all cases that foreach with a smaller index is valid. But foreach we allow and foreach_reverse we disallow. Seems arbitrary.

--
June 12, 2016
https://issues.dlang.org/show_bug.cgi?id=16149

--- Comment #6 from Ketmar Dark <ketmar@ketmar.no-ip.org> ---
(In reply to Steven Schveighoffer from comment #5)
> int x = a.length would continue to be invalid (on 64-bit CPU). It's just for
> foreach_reverse.

then i'll inevitably ask why `int x = a.length;` is invalid, but `foreach_reverse (int x, v; a)` is valid. that `foreach` obviously does the assign under the hood (at least this is the most practical way to imagine it).

the only way to skip that "hidden assign" is to redefive `foreach_reverse` completely — by still using increasing index in it, so x will go from 0 upto limit. otherwise you just introducing a random pseudo-solution by randomly breaking the rules.

--
June 13, 2016
https://issues.dlang.org/show_bug.cgi?id=16149

--- Comment #7 from Steven Schveighoffer <schveiguy@yahoo.com> ---
(In reply to Ketmar Dark from comment #6)
> (In reply to Steven Schveighoffer from comment #5)
> > int x = a.length would continue to be invalid (on 64-bit CPU). It's just for
> > foreach_reverse.
> 
> then i'll inevitably ask why `int x = a.length;` is invalid, but `foreach_reverse (int x, v; a)` is valid. that `foreach` obviously does the assign under the hood (at least this is the most practical way to imagine it).

foreach_reverse could be implemented like this:

for(size_t __idx = a.length; __idx; --__idx)
{
   int x = cast(int)(__idx - 1);
   auto v = a[__idx-1];
   ... // your code
}

In fact, if foreach was implemented similarly (always using hidden size_t to iterate and assigning to your chosen variable as the index), it wouldn't get into an infinite loop.

> the only way to skip that "hidden assign" is to redefive `foreach_reverse` completely — by still using increasing index in it, so x will go from 0 upto limit. otherwise you just introducing a random pseudo-solution by randomly breaking the rules.

The definition of foreach_reverse on an array is that it works exactly like foreach, but visits the elements in reverse. There is no definition of how it does this, so my implementation is valid. (in fact, the spec says the index must be int, uint, or size_t, but I think this is a relic from 32-bit, non VRP days).

The fact that the implementation rejects it is an implementation detail leaking. It's not a *bad* thing, but clearly this is the case of the compiler being too cautious, because it's OK with you being stupid in foreach.

--
February 05, 2020
https://issues.dlang.org/show_bug.cgi?id=16149

moonlightsentinel@disroot.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |moonlightsentinel@disroot.o
                   |                            |rg
         Resolution|---                         |FIXED

--- Comment #8 from moonlightsentinel@disroot.org ---
The code works and yields an appropriate warning concerning int -> size_t for both loops

--