Bug 13390 – [REG2.066] std.range.cycle ctor fails when empty input passed

Status
RESOLVED
Resolution
FIXED
Severity
regression
Priority
P1
Component
phobos
Product
D
Version
D2
Platform
All
OS
All
Creation time
2014-08-28T08:39:00Z
Last change time
2017-07-19T17:42:09Z
Assigned to
nobody
Creator
drug2004

Comments

Comment #0 by drug2004 — 2014-08-28T08:39:57Z
import std.range; void main() { auto d = [0, 1, 2]; auto b = cycle(d[0..0]); // generates SIGFPE } because std.range.cycle ctor doesn't check length equality to zero (https://github.com/D-Programming-Language/phobos/blob/master/std/range.d#L4471) But I believe zero length should be possible.
Comment #1 by drug2004 — 2014-08-28T08:50:33Z
Sorry, I used master instead of 2.066 branch, this reference should be https://github.com/D-Programming-Language/phobos/blob/2.066/std/range.d#L4289 P.S. it's strange, but I cannot fork phobos to make even simple PR
Comment #2 by k.hara.pg — 2014-08-28T14:18:16Z
In Windows platform, the code will raise a runtime error "Integer Divide by Zero" with 2.066 and git-head (2.067alpha). But the issue does not occur with 2.065.
Comment #3 by drug2004 — 2014-08-28T14:53:15Z
Yes, this is 2.066 regression. Is it suitable decision instead of: this(R input, size_t index = 0) { _original = input; _index = index % _original.length; } use this this(R input, size_t index = 0) { _original = input; _index = (_original.length) ? index % _original.length : 0; } If yes I can make PR.
Comment #4 by hsteoh — 2014-08-28T22:44:10Z
Sounds reasonable, though I would write "(_original.length > 0) ? ..." for clarity's sake.
Comment #5 by drug2004 — 2014-08-29T06:54:42Z
I agree with clarity. But I'm not sure if cycle may be empty at all, that's the question, I guess...
Comment #6 by monarchdodra — 2014-08-29T10:24:46Z
(In reply to drug007 from comment #5) > I agree with clarity. But I'm not sure if cycle may be empty at all, that's > the question, I guess... I believe cycle should not be passed an empty range. Or to be more precise, the underlying range may not at any point in time become empty. The reasoning for this is that cycle is an infinite range, meaning it can be *statically* verified as being not empty. Now, it is always incorrect for phobos to produce a crash. At the very least, an assert should be placed. Furthermore, I don't really believe that the constructor should fail. Rather, the operations pop should error out, and the operations front should index out of bounds. And we should document the error. I'm on it.
Comment #7 by code — 2014-10-07T00:08:12Z
When was this bug introduced? Any fix in sight?
Comment #8 by hsteoh — 2014-10-07T00:25:40Z
I'm surprised this worked in older releases. Or *appears* to work -- it's probably actually broken because cycle() returns an infinite range, but when the input is empty, it can't be an infinite range. So this is one of those places where the compile-time requirement of isInfinite breaks down.
Comment #9 by monarchdodra — 2014-10-07T07:40:37Z
(In reply to Martin Nowak from comment #7) > When was this bug introduced? I don't know if it's a bug, but I think I introduced it. I don't remember the details though. > Any fix in sight? I remember having started a fix, and I still have the branch. I thought it would be as simple as placing an assert, but it turned out asserting in the constructor is over-eager, yet asserting in front is wasteful. I need to review what I had written...
Comment #10 by kuettler — 2015-02-18T10:01:13Z
Comment #11 by github-bugzilla — 2015-03-01T00:01:11Z
Commits pushed to master at https://github.com/D-Programming-Language/phobos https://github.com/D-Programming-Language/phobos/commit/568cd18b43fb2fe98a5eb62f06a00648450d7068 Fix Issue 13390 - Be explicit about non-empty input and fail early Add unittest https://github.com/D-Programming-Language/phobos/commit/4f3c90aaa3f47404647993eab2e92b53a9bfa4b9 Merge pull request #3001 from kuettler/fix13390 Fix Issue 13390 - Be explicit about non-empty input and fail early
Comment #12 by github-bugzilla — 2015-03-10T11:46:22Z
Commit pushed to 2.067 at https://github.com/D-Programming-Language/phobos https://github.com/D-Programming-Language/phobos/commit/e2d774fc85804ce1eabdaf5f526fb287a2d48417 Merge pull request #3001 from kuettler/fix13390 Fix Issue 13390 - Be explicit about non-empty input and fail early
Comment #13 by github-bugzilla — 2015-04-11T12:25:00Z
Comment #14 by github-bugzilla — 2015-06-17T21:02:58Z
Comment #15 by github-bugzilla — 2017-07-19T17:42:09Z
Commits pushed to dmd-cxx at https://github.com/dlang/phobos https://github.com/dlang/phobos/commit/568cd18b43fb2fe98a5eb62f06a00648450d7068 Fix Issue 13390 - Be explicit about non-empty input and fail early https://github.com/dlang/phobos/commit/4f3c90aaa3f47404647993eab2e92b53a9bfa4b9 Merge pull request #3001 from kuettler/fix13390