Bug 9570 – Wrong foreach index implicit conversion error

Status
RESOLVED
Resolution
FIXED
Severity
enhancement
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2013-02-22T09:38:53Z
Last change time
2020-09-02T09:46:32Z
Keywords
diagnostic, pull, rejects-valid
Assigned to
No Owner
Creator
bearophile_hugs
See also
http://d.puremagic.com/issues/show_bug.cgi?id=9572

Comments

Comment #0 by bearophile_hugs — 2013-02-22T09:38:53Z
void main() { ubyte[256] data; foreach (const i, x; data) data[i] = i; } DMD 2.063alpha gives: test.d(4): Error: cannot implicitly convert expression (i) of type const(uint) to ubyte Here "data" is a fixed-size array, with length known statically to be 256, so the index "i" of the foreach is statically known to be in [0,256[, that is the range of a ubyte. So the conversion error given by the compiler is wrong.
Comment #1 by yebblies — 2013-11-16T21:43:57Z
*** Issue 10685 has been marked as a duplicate of this issue. ***
Comment #2 by yebblies — 2013-11-16T21:44:41Z
Same thing for foreach(immutable i; 0..7) etc
Comment #3 by lio+bugzilla — 2014-05-21T19:01:47Z
Comment #4 by bearophile_hugs — 2014-05-21T20:17:18Z
(In reply to Lionello Lunesu from comment #3) > https://github.com/D-Programming-Language/dmd/pull/3567 Is this supported? void main() { ubyte[256] data; foreach (ubyte i, ref x; data) x = i; }
Comment #5 by lio+bugzilla — 2014-05-21T20:19:27Z
(In reply to bearophile_hugs from comment #4) > (In reply to Lionello Lunesu from comment #3) > > https://github.com/D-Programming-Language/dmd/pull/3567 > > Is this supported? > > void main() { > ubyte[256] data; > foreach (ubyte i, ref x; data) > x = i; > } No, the key must be declared const or immutable. Otherwise we need flow analysis to ensure the key doesn't get mutated in the scope.
Comment #6 by bearophile_hugs — 2014-05-21T20:34:10Z
(In reply to Lionello Lunesu from comment #5) > No, the key must be declared const or immutable. Otherwise we need flow > analysis to ensure the key doesn't get mutated in the scope. I don't understand what's the problem. What kind of bad things do happen if the key gets mutated in the scope? void main() { ubyte[256] data; foreach (ubyte i, ref x; data) { x = i; i++; // overflow here } }
Comment #7 by yebblies — 2014-05-21T20:40:02Z
(In reply to bearophile_hugs from comment #6) > (In reply to Lionello Lunesu from comment #5) > > > No, the key must be declared const or immutable. Otherwise we need flow > > analysis to ensure the key doesn't get mutated in the scope. > > I don't understand what's the problem. What kind of bad things do happen if > the key gets mutated in the scope? > > void main() { > ubyte[256] data; > foreach (ubyte i, ref x; data) { > x = i; > i++; // overflow here > } > } If i is mutated before it's used, the range information from the foreach aggregate might not be accurate. Your example is fine, but to tell it apart from the bad ones requires flow analysis for non-trivial cases. void main() { ubyte[256] data; foreach (ubyte i, ref x; data) { i = 99999; x = i; // information lost } }
Comment #8 by bearophile_hugs — 2014-05-21T20:47:43Z
(In reply to yebblies from comment #7) > void main() { > ubyte[256] data; > foreach (ubyte i, ref x; data) { > i = 99999; > x = i; // information lost > } > } This code should give (as it currently gives): temp.d(4,13): Error: cannot implicitly convert expression (99999) of type int to ubyte Here i scan a ubyte range 0 .. 255, so tagging it as ubyte is OK. I think it's irrelevant that later I try to overflow the contents of i. Even this is OK for D, despite 'i' gets overflowed: void main() { ubyte[256] data; foreach (ubyte i, ref x; data) { i += 200; i = 200; x = i; } } So I still don't see the problem.
Comment #9 by yebblies — 2014-05-21T20:54:34Z
Oh right, I misread your example. That won't be fixed by this pull.
Comment #10 by lio+bugzilla — 2014-05-21T20:59:30Z
I also misread. The problem here is that the internal iterator (the one named __key##, declared by the compiler) needs to be able to count to 256 inclusive (for the comparison the be false and terminate the for.) There's no reason why the internal iterator and the declared one are the same type (in fact, they need not be), but at the moment the compiler uses the declared type for both the declared iterator and the internal one. It's an unrelated fix.
Comment #11 by lio+bugzilla — 2014-05-21T21:01:53Z
In fact, we should probably open a new bug for that case, since it's not the same.
Comment #12 by bearophile_hugs — 2014-05-21T21:07:47Z
(In reply to Lionello Lunesu from comment #11) > In fact, we should probably open a new bug for that case, since it's not the > same. OK, it's Issue 12782
Comment #13 by github-bugzilla — 2014-06-15T21:02:09Z
Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/603f15cd1c30086de597e7ed0d649310ac8d03c3 Fix Issue 9570 - Wrong foreach index implicit conversion error https://github.com/D-Programming-Language/dmd/commit/34d970836b202f71d0a07c4b2a195a405b69bc6d Merge pull request #3567 from lionello/bug9570 Issue 9570 - Wrong foreach index implicit conversion error
Comment #14 by pro.mathias.lang — 2020-09-02T09:46:32Z
*** Issue 12611 has been marked as a duplicate of this issue. ***