Bug 16149 – foreach_reverse can't handle index variable of type int

Status
RESOLVED
Resolution
FIXED
Severity
enhancement
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
x86_64
OS
All
Creation time
2016-06-09T19:24:16Z
Last change time
2020-02-05T14:21:32Z
Assigned to
No Owner
Creator
Acer.Yang

Comments

Comment #0 by yangacer — 2016-06-09T19:24:16Z
import std.stdio; void main() { string s = "12345"; foreach(int i, c; s) {} // ok foreach_reverse(int i, c; s) {} // error } --- Compiler msg: Error: cannot implicitly convert expression (__r1493.length) of type ulong to int
Comment #1 by ketmar — 2016-06-10T11:59:25Z
...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.
Comment #2 by yangacer — 2016-06-10T15:24:37Z
(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).
Comment #3 by schveiguy — 2016-06-10T15:47:11Z
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 :)
Comment #4 by ketmar — 2016-06-11T12:11:01Z
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. ;-)
Comment #5 by schveiguy — 2016-06-12T11:50:21Z
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.
Comment #6 by ketmar — 2016-06-12T12:03:53Z
(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.
Comment #7 by schveiguy — 2016-06-13T12:24:25Z
(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.
Comment #8 by moonlightsentinel — 2020-02-05T14:21:32Z
The code works and yields an appropriate warning concerning int -> size_t for both loops