isempty(::EachLine) expends a line

[oxinabox]

Eachline is (almost?) always a stateful iterator.
This means the default isempty which called iterate will consume a line if there is one to consume.

Sometimes (always?) in the use of Eachline (but not more generally), a defensive deepcopy can protect against that.

This is in 0.7
I feel like I (or someone else) opened a similar issue for this in 0.6
but I could be mis-remembering.

Downside of this fix, is that I'm not sure it works for all uses of Eachline.
It works for IOBuffer (and I think IOStream) because there is a pointer integer that is copied.
But if it was consuming unbuffered network data, then not possible because what is gone is gone.
(idk that it is possible to access unbuffered network data in julia)

Minimum Breaking Example

julia> data = IOBuffer("A\nB\nC\nD")
IOBuffer(data=UInt8[...], readable=true, writable=false, seekable=true, append=false, size=7, maxsize=Inf, ptr=1, mark=-1)

julia> itr  = eachline(data)
Base.EachLine(IOBuffer(data=UInt8[...], readable=true, writable=false, seekable=true, append=false, size=7, maxsize=Inf, ptr=1, mark=-1), getfield(Base, Symbol("##289#292"))(), false)

julia> isempty(itr)
false

julia> collect(itr)
3-element Array{String,1}:
 "B"
 "C"
 "D"

Defensive-copy work-around

julia> data = IOBuffer("A\nB\nC\nD")
IOBuffer(data=UInt8[...], readable=true, writable=false, seekable=true, append=false, size=7, maxsize=Inf, ptr=1, mark=-1)

julia> itr = eachline(data)
Base.EachLine(IOBuffer(data=UInt8[...], readable=true, writable=false, seekable=true, append=false, size=7, maxsize=Inf, ptr=1, mark=-1), getfield(Base, Symbol("##289#292"))(), false)

julia> isempty(deepcopy(itr))
false

julia> collect(itr)
4-element Array{String,1}:
 "A"
 "B"
 "C"
 "D"

[oxinabox]

When discussing this on slack more broadly.
I suggested that perhaps we should have a trait for
IsStateful

Such a trait could be tri-valued

• NotStateful, so just running iterate would work
• InternallyStateful, where the state of the iterator is real but it is visible to julia, so defensive copy would work.
• ExternallyStateful where the state is not visible to julia so deepcopy will not work, so any call to things like isempty should error.

And that trait would determine which default implementation of isempty etc one received
Similar for tee (do we have tee? It turns one iterator into two)

[haampie]

Another defensive approach is to make isempty error by default; I don't think deepcopy is suitable for iterators that contain for instance huge vectors.

About isempty for eachline: maybe we should not have it implemented for this iterator at all, but rather only for a stateful wrapper of this iterator that caches the current line (i.e. the current Stateful iterator).

[haampie]

Another benefit is that this allows a trivial implementation of zip for iterators that are not stateful; the current implementation is quite convoluted as it must assume stateful iterators, and I have seen examples where the compiler produces suboptimal code. (The issue is that it has to call isdone on all iterators, followed by iterate on all iterators, but it still has to check whether the iterators return nothing or not.)

[JeffBezanson]

To copy the state of a stateful iterator, copy should suffice; deepcopy should not be necessary.

Just a bandaid, but we could add an isempty method for EachLine that just calls eof.

[cmcaine]

This bit me again today. What other stateful iterators are there in Julia? Maybe we can just add bandaids for most of them?

From the docs, it looks like we should just add Base.isdone(itr::Eachline) = eof(itr.stream) ? I've opened a PR (#39562) that does that.

Agree with traits or similar so code can identify stateful iterators.

[vtjnash]

Fixed by #39562 in ede6486