Bug 19823 – std.algorithm.iteration.filter's popFront doesn't always pop the first element like it's supposed to

It seems that in some cases dropOne doesn't work when called after std.algorithm.iteration.filter. In the example I'm pasting below it seems that it only works when the 1st position in the array is not filtered out. In production we had more complex cases where it would do the correct thing when compiled with dmd, but it would not work when compiled with ldc2. Anyone knows why? I didn't look at the source code, but it looks like buggy code generation. import std.range: dropOne; import std.stdio: writefln; import std.algorithm.iteration: filter; import std.array: array; void main() { auto a = [1,2,3,4]; writefln("%s", a.filter!(a => a != 1).dropOne); // [2, 3, 4] WRONG writefln("%s", a.filter!(a => a != 2).dropOne); // [3, 4] writefln("%s", a.filter!(a => a != 3).dropOne); // [2, 4] writefln("%s", a.filter!(a => a != 4).dropOne); // [2, 3] writefln("%s", a.filter!(a => a == 1).dropOne); // [] writefln("%s", a.filter!(a => a == 2).dropOne); // [2] WRONG writefln("%s", a.filter!(a => a == 3).dropOne); // [3] WRONG writefln("%s", a.filter!(a => a == 4).dropOne); // [4] WRONG writefln("%s", a.filter!(a => a != 1).array.dropOne); // [3, 4] }

@shove70 created dlang/phobos pull request #7071 "Fix issue 19823 - std.range.dropOne doesn't drop the element when cal…" fixing this issue: - Fix issue 19823 - std.range.dropOne doesn't drop the element when called after std.algorithm.iteration.filter https://github.com/dlang/phobos/pull/7071

@shove70 created dlang/phobos pull request #7072 "Fix issue 19823 - std.range.dropOne doesn't drop the element when cal…" fixing this issue: - Fix issue 19823 - std.range.dropOne doesn't drop the element when called after std.algorithm.iteration.filter https://github.com/dlang/phobos/pull/7072

dropOne isn't actually the problem at all. Rather, it looks like the problem is with either `Filter`'s `popFront` or with the code that dmd is generating. This reduced test case has the same problem: void main() { import std.algorithm.comparison : equal; import std.algorithm.iteration : filter; auto arr = [1, 2, 3, 4]; auto result = arr.filter!(a => a != 1)(); assert(result.save.equal([2, 3, 4])); result.popFront(); assert(result.equal([3, 4])); } For some reason, `popFront` is not doing anything with this particular example, and the second assertion fails. Rather if the result is printed after `popFront`, it's equivalent to [2, 3, 4], and changing the second assertion to assert(result.equal([2, 3, 4])); makes it pass. So, clearly, `popFront` is doing nothing for some reason.

(In reply to Jonathan M Davis from comment #3) > dropOne isn't actually the problem at all. Rather, it looks like the problem > is with either `Filter`'s `popFront` or with the code that dmd is > generating. This reduced test case has the same problem: > > void main() > { > import std.algorithm.comparison : equal; > import std.algorithm.iteration : filter; > > auto arr = [1, 2, 3, 4]; > > auto result = arr.filter!(a => a != 1)(); > assert(result.save.equal([2, 3, 4])); > > result.popFront(); > assert(result.equal([3, 4])); > } > > > For some reason, `popFront` is not doing anything with this particular > example, and the second assertion fails. Rather if the result is printed > after `popFront`, it's equivalent to [2, 3, 4], and changing the second > assertion to > > assert(result.equal([2, 3, 4])); > > makes it pass. So, clearly, `popFront` is doing nothing for some reason. You're right, drop, dropOne... have no design or coding problems. The problem is struct FilterResult of std.algorithm.iteration.d: ... private struct FilterResult(alias pred, Range) { alias R = Unqual!Range; R _input; private bool _primed; private void prime() { if (_primed) return; while (!_input.empty && !pred(_input.front)) { _input.popFront(); } _primed = true; } this(R r) { _input = r; } private this(R r, bool primed) { _input = r; _primed = primed; } auto opSlice() { return this; } static if (isInfinite!Range) { enum bool empty = false; } else { @property bool empty() { prime; return _input.empty; } } //void popFront() //{ // do // { // _input.popFront(); // } while (!_input.empty && !pred(_input.front)); // _primed = true; //} // Modify it to read as follows: void popFront() { prime; do { _input.popFront(); } while (!_input.empty && !pred(_input.front)); } @property auto ref front() { prime; assert(!empty, "Attempting to fetch the front of an empty filter."); return _input.front; } static if (isForwardRange!R) { @property auto save() { return typeof(this)(_input.save, _primed); } } } ... In this way, dropOne can work properly. But the scope of influence here is too wide, there are too many APIs, classes and templates in Phobos that use filter. If it's not necessary, it's better not to change it. I'd like to hear your further advice. Thank you!

dlang/phobos pull request #7072 "Fix issue 19823 - std.range.dropOne doesn't drop the element when cal…" was merged into master: - 83a3ab4d6a7e8c2634070193316add121971ea4d by shove70: Fix issue 19823 - std.range.dropOne doesn't drop the element when called after std.algorithm.iteration.filter https://github.com/dlang/phobos/pull/7072