Bug 18657 – std.range and std.algorithm can't handle refRange

Status
RESOLVED
Resolution
FIXED
Severity
normal
Priority
P1
Component
phobos
Product
D
Version
D2
Platform
All
OS
All
Creation time
2018-03-24T13:49:22Z
Last change time
2019-12-18T08:27:08Z
Assigned to
No Owner
Creator
ag0aep6g

Comments

Comment #0 by ag0aep6g — 2018-03-24T13:49:22Z
Five examples: ---- void main() { import std.range: refRange; import std.stdio; { import std.algorithm.iteration: group; string s = "foo"; auto r = refRange(&s).group; writeln(r.save); /* Prints "[Tuple!(dchar, uint)('f', 1), Tuple!(dchar, uint)('o', 2)]". Ok. */ writeln(r.save); /* Should print the same as the line above. Actually prints "[Tuple!(dchar, uint)('f', 1)]". */ } { import std.range: chain; string s = "foo"; auto r = refRange(&s).chain("bar"); writeln(r.save); /* Should print "foobar". Actually prints "bar". */ } { import std.range: choose; string s = "foo"; auto r = choose(true, refRange(&s), "bar"); writeln(r); /* Should print "foo". Actually prints nothing. */ } { import std.range: cycle, take; string s = "foo"; auto r = refRange(&s).cycle.take(4); writeln(r.save); /* Prints "foof". Ok. */ writeln(r.save); /* Should print "foof", too. Actually prints "oofo". */ } { import std.algorithm.iteration: splitter; string s = "foobar"; auto r = refRange(&s).splitter!(c => c == 'b'); writeln(r.save); /* Prints "[foo, ar]". Ok. */ writeln(r.save); /* Should print the same. Actually crashes with an AssertError. */ } } ---- Most probably, there are more Phobos functions that can't handle refRange. I haven't checked them all. The root of the problem is RefRange's opAssign. Instead of just changing the reference, it actually overwrites the referenced range. That leads to surprising behavior: ---- void main() { import std.range; import std.stdio; string s = "foo"; auto r = refRange(&s); auto r2 = r; r2 = r2.save; /* Surprising: Effectively just does `s = s;` (i.e., nothing). */ r2.popFront(); writeln(r); /* Surprising: Prints "oo". */ } ---- Note that `r2 = r; r2 = r2.save;` is what you typically do in a postblit function. If RefRange's custom opAssign is removed, all the examples just work. Unfortunately, the surprising behavior is deliberate, and not just a bug. The docs on RefRange.opAssign say [1]: > This does not assign the pointer of rhs to this RefRange. > Rather it assigns the range pointed to by rhs to the range pointed > to by this RefRange. This is because any operation on a RefRange is > the same is if it occurred to the original range. The issue comes down to whether RefRange should be allowed to have its funky opAssign, or if range-handling code should be allowed to assume that assignment does the obvious thing. [1] https://dlang.org/phobos/std_range.html#.RefRange.opAssign
Comment #1 by ag0aep6g — 2018-03-25T22:59:09Z
PR to fix the first three examples without touching RefRange: https://github.com/dlang/phobos/pull/6346
Comment #2 by dlang-bot — 2019-03-24T14:21:50Z
@aG0aep6G updated dlang/phobos pull request #6346 "make `group`, `chain`, and `choose` compatible with `RefRange`" mentioning this issue: - make `chain` compatible with `RefRange` Part of a series on issue 18657. - make `group` compatible with `RefRange` Part of a series on issue 18657. - make `choose` compatible with `RefRange` Part of a series on issue 18657. https://github.com/dlang/phobos/pull/6346
Comment #3 by dlang-bot — 2019-03-26T11:44:14Z
dlang/phobos pull request #6346 "make `group`, `chain`, and `choose` compatible with `RefRange`" was merged into stable: - b1b8b7968ffda3d64f9ee4666ebe453311eeb66e by aG0aep6G: make `chain` compatible with `RefRange` Part of a series on issue 18657. - e14ef1c912dcd84c197c03e7a8683e344559144b by aG0aep6G: make `group` compatible with `RefRange` Part of a series on issue 18657. - be9020a369091f58c3e5b1f3f3d31441d402d3f1 by aG0aep6G: make `choose` compatible with `RefRange` Part of a series on issue 18657. https://github.com/dlang/phobos/pull/6346
Comment #4 by dlang-bot — 2019-03-26T20:48:43Z
@aG0aep6G created dlang/phobos pull request #6935 "make `cycle`, `splitter`, `roundRobin`, and `until` compatible with `RefRange`" mentioning this issue: - make `cycle` compatible with `RefRange` Part of a series on issue 18657. - make `splitter` compatible with `RefRange` Part of a series on issue 18657. - make `roundRobin` compatible with `RefRange` Part of a series on issue 18657. - make `until` compatible with `RefRange` Part of a series on issue 18657. https://github.com/dlang/phobos/pull/6935
Comment #5 by dlang-bot — 2019-03-28T02:34:43Z
dlang/phobos pull request #6935 "make `cycle`, `splitter`, `roundRobin`, and `until` compatible with `RefRange`" was merged into stable: - 3ef957baf536b55fa9ebd93050e187f52e2c47f5 by aG0aep6G: make `cycle` compatible with `RefRange` Part of a series on issue 18657. - 729f732fbc8716baa712ec9b5e645fc4919a9e78 by aG0aep6G: make `splitter` compatible with `RefRange` Part of a series on issue 18657. - aeea9598bde91a9e42e5a5bad791ee3378d84e89 by aG0aep6G: make `roundRobin` compatible with `RefRange` Part of a series on issue 18657. - bd47453b49cf93e77cf11972f4167b9fd420f58d by aG0aep6G: make `until` compatible with `RefRange` Part of a series on issue 18657. https://github.com/dlang/phobos/pull/6935
Comment #6 by dlang-bot — 2019-04-06T01:49:09Z
@MartinNowak created dlang/phobos pull request #6943 "Merge remote-tracking branch 'upstream/stable' into merge_stable" mentioning this issue: - make `chain` compatible with `RefRange` Part of a series on issue 18657. - make `group` compatible with `RefRange` Part of a series on issue 18657. - make `choose` compatible with `RefRange` Part of a series on issue 18657. - make `cycle` compatible with `RefRange` Part of a series on issue 18657. - make `splitter` compatible with `RefRange` Part of a series on issue 18657. - make `roundRobin` compatible with `RefRange` Part of a series on issue 18657. - make `until` compatible with `RefRange` Part of a series on issue 18657. https://github.com/dlang/phobos/pull/6943
Comment #7 by dlang-bot — 2019-04-09T16:46:53Z
@wilzbach created dlang/phobos pull request #6951 "Merge remote-tracking branch 'upstream/stable' into merge_stable" mentioning this issue: - make `chain` compatible with `RefRange` Part of a series on issue 18657. - make `group` compatible with `RefRange` Part of a series on issue 18657. - make `choose` compatible with `RefRange` Part of a series on issue 18657. - make `cycle` compatible with `RefRange` Part of a series on issue 18657. - make `splitter` compatible with `RefRange` Part of a series on issue 18657. - make `roundRobin` compatible with `RefRange` Part of a series on issue 18657. - make `until` compatible with `RefRange` Part of a series on issue 18657. https://github.com/dlang/phobos/pull/6951
Comment #8 by bugzilla — 2019-12-18T08:27:08Z
The original problem has meanwhile been fixed as well as many others related. If there are still some functions, that are not compatible with RefRange, IMHO they should be addressed in a separate bug report; else we will never know, if we can close this or not...