Filter(f, Task) is fundamentally broken

[StefanKarpinski]

An issue with filtering a Task was raised here:

https://groups.google.com/forum/#!topic/julia-users/gn0Rj9XexNk

The immediate patch I tried is to invert the predicate like this:

diff --git a/base/iterator.jl b/base/iterator.jl
index 977defd..0e661c8 100644
--- a/base/iterator.jl
+++ b/base/iterator.jl
@@ -71,7 +71,7 @@ function start_filter(pred, itr)
     s = start(itr)
     while !done(itr,s)
         v, t = next(itr,s)
-        if pred(v)
+        if !pred(v)
             break
         end
         s = t
@@ -84,7 +84,7 @@ function advance_filter(pred, itr, s)
     v, s = next(itr,s)
     while !done(itr,s)
         w, t = next(itr,s)
-        if pred(w)
+        if !pred(w)
             break
         end
         s = t

However, this is not correct – the predicate direction was actually right: keep values for which the predicate is true. Accordingly, this change causes our functional tests to fail:

julia> collect(filter(x->x[1], zip([true, false, true, false],"abcd")))
2-element Array{(Bool,Char),1}:
 (false,'b')
 (false,'d')

To really fix this issue, we need to keep the previously yielded state and value – specifically for iterables like tasks and IO streams where the state is really simply the state of the iterable object itself and the functional state is a dummy. We could make that change, but it's really awkward, annoying and inefficient – and makes it impossible to avoid some kind of unintuitiveness since advancing the iterator happens before people expect it too. Perhaps we should really consider changing the iteration protocol as brought up in #6125. All of this is highly exacerbated when composed with lazy iteration like Filter, Zip, etc

[JeffBezanson]

Here's an interesting idea. We could require stateful iterators to be able to repeatedly return the current value without side effects, e.g.:

1. The state (currently nothing) should be an integer counting up
2. You must save the last value returned by next
3. If next is called again with the previous state, return the value again without side effects
4. If next is called with a state earlier than the previous state, throw an error
5. If next is called with a state later than the previous state, do your side effects and get the next value

The integer is only one possible implementation, but used as an example for clarity.
I think this would be sufficient to get Filter to work, and most other iterator patterns. It also pushes more of the burden onto the stateful iterators where it belongs.

[porterjamesj]

bump; I'm working on an HTML parsing library and it would be really convenient to be able to define tree traversal functions on HTML elements by just returning tasks, but I'm hesitant to do so because of this :/

[porterjamesj]

Jeff's idea here works fine; if it's not a huge performance hit I'm all in favor of changing to that. Even if it's slightly slower, better to be correct.

[JeffBezanson]

fixed by 1f5ed7a

[porterjamesj]

👏 💯 I had thought about fixing it via Filter but this is almost certainly better than whatever I would have come up with. Thanks!