On 03/24/2018 11:36 PM, Simen Kjærås wrote:
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". */
}
None of this is surprising. When you call r2.popFront(), it's being
forwarded to s.popFront. That's what refRange does, it's what it's
supposed to do, and it's the only thing it could sensibly do.
I think it's surprising. It works as intended and as documented, but
it's surprising. Note that the behavior is different when you change
auto r2 = r;
r2 = r2.save;
to
auto r2 = r.save;
`.save` actually makes a new slice that can be popped independently from
`s`. But the assignment throws that independent slice away.
This is conceptually what refRange does:
struct RefRange(R) {
R* innerRange;
this(R* innerRange) {
this.innerRange = innerRange;
}
void popFront() {
innerRange.popFront();
}
@property auto front() {
return innerRange.front;
}
@property bool empty() {
return innerRange.empty;
}
}
That's all fine. You're missing the interesting bit: opAssign. `save`
might also be interesting, but the actual `save` is perfectly fine.
[...]
So when you do this:
string s = "foo";
auto r = refRange(&s).group;
writeln(r.save);
r.save is going to create a new Group range, which contains a RefRange,
which ultimately points to s. writeln() then repeatedly calls
popFront(). This mutates s, as specified above.
That's not how it should be. A saved range should be iterable
independently from the original. That's the point of `.save`. If that's
not possible, `.save` should not be there.
You're implying that a saved RefRange points to the same range as the
original. That's not true. RefRange.save saves the underlying range and
lets the new RefRange point to that. This is correct. The two ranges can
be iterated independently.
It's inside `Group` that things get messed up. There you have this
innocent looking implementation of `save` [1]:
@property typeof(this) save() {
typeof(this) ret = this;
ret._input = this._input.save;
ret._current = this._current;
return ret;
}
The problem is in the line `ret._input = this._input.save;`. That calls
RefRange.opAssign. If you refactor the code to avoid opAssign, saving
works properly. For example:
private this(typeof(_input) inp, typeof(_current) cur)
{
this._input = inp; /* Initialization, not assigment. */
this._current = cur; /* Ditto, but doesn't matter. */
}
@property typeof(this) save() {
return typeof(this)(this._input.save, this._current);
}
Note that it calls `_input.save` just the same as before. The code only
avoids RefRange.opAssign.
[...]
The 'chain', 'choose', and 'cycle' examples are an effect of strings not
being random access ranges. I'm uncertain if we should call this a bug,
but I agree the behavior is unexpected, so we probably should.
I don't think random access has anything to do with this.
The splitter example is of course a bug. The crash, that is. The
expected behavior is that the first writeln prints [foo, ar], which it
does, and that the second print [].
Again, I don't agree. You're effectively saying that saving a RefRange
isn't possible. If that were so, it simply shouldn't have a `save`
method. But the `save` method works perfectly fine. It's Phobos
(accidentally) calling RefRange.opAssign that breaks things.
[1]
https://github.com/dlang/phobos/blob/3225b19c9bae4b7e8d7a93d2d8c22c94b2a1a6c5/std/algorithm/iteration.d#L1505-L1510
