BREAKING: eof errors if stream isn't readable (#230)

Co-authored-by: Mark Kittisopikul <[email protected]>

Author: nhz2

docs/make.jl

@@ -4,7 +4,7 @@ using TranscodingStreams
 makedocs(
     sitename="TranscodingStreams.jl",
     modules=[TranscodingStreams],
-    pages=["index.md", "examples.md", "reference.md", "devnotes.md"],
+    pages=["index.md", "examples.md", "reference.md", "migrating.md", "devnotes.md"],
     format=Documenter.HTML(; assets=["assets/custom.css"]),
 )
 

docs/src/migrating.md

@@ -0,0 +1,28 @@
+Migration
+=========
+
+How to migrate from v0.10 to v0.11
+----------------------------------
+
+v0.11 has a few subtle breaking changes to `eof` and `seekend`.
+
+### `Memory(data::ByteData)`
+
+The `Memory(data::ByteData)` constructor was removed.
+Use `Memory(pointer(data), sizeof(data))` instead.
+
+### `seekend(stream::TranscodingStream)`
+
+Generic `seekend` for `TranscodingStream` was removed.
+If the objective is to discard all remaining data in the stream, use `skip(stream, typemax(Int64))` instead where `typemax(Int64)` is meant to be a large number to exhaust the stream.
+Ideally, specific implementations of `TranscodingStream` will implement `seekend` only if efficient means exist to avoid fully processing the stream.
+`NoopStream` still supports `seekend`.
+
+The previous behavior of the generic `seekend` was something like 
+`(seekstart(stream); seekend(stream.stream); stream)` but this led to
+inconsistencies with the position of the stream.
+
+### `eof(stream::TranscodingStream)`
+
+`eof` now throws an error if called on a stream that is closed or in writing mode.
+Use `!isreadable(stream) || eof(stream)` if you need to more closely match previous behavior.

src/stream.jl

@@ -206,31 +206,24 @@ function Base.eof(stream::TranscodingStream)
     eof = buffersize(stream.buffer1) == 0
     state = stream.state
     mode = state.mode
-    if !(mode == :read || mode == :stop) || eof
+    if !(mode === :read || mode === :stop)
+        changemode!(stream, :read)
+    end
+    if eof
         eof = sloweof(stream)
     end
     return eof
 end
 @noinline function sloweof(stream::TranscodingStream)
-    while true
-        state = stream.state
-        mode = state.mode
-        if mode == :read
-            return (buffersize(stream.buffer1) == 0 && fillbuffer(stream) == 0)
-        elseif mode == :idle
-            changemode!(stream, :read)
-            continue
-        elseif mode == :write
-            return eof(stream.stream)
-        elseif mode == :close
-            return true
-        elseif mode == :stop
-            return buffersize(stream.buffer1) == 0
-        elseif mode == :panic
-            throw_panic_error()
-        end
-        @assert false
+    state = stream.state
+    mode = state.mode
+    @assert mode == :read || mode == :stop
+    if mode == :read
+        return (buffersize(stream.buffer1) == 0 && fillbuffer(stream) == 0)
+    elseif mode == :stop
+        return buffersize(stream.buffer1) == 0
     end
+    @assert false
 end
 
 function Base.ismarked(stream::TranscodingStream)::Bool
@@ -316,14 +309,7 @@ end
 
 # needed for `peek(stream, Char)` to work
 function Base.peek(stream::TranscodingStream, ::Type{UInt8})::UInt8
-    # eof and ready_to_read! are inlined here because ready_to_read! is very slow and eof is broken
-    eof = buffersize(stream.buffer1) == 0
-    state = stream.state
-    mode = state.mode
-    if !(mode == :read || mode == :stop)
-        changemode!(stream, :read)
-    end
-    if eof && sloweof(stream)
+    if eof(stream)
         throw(EOFError())
     end
     buf = stream.buffer1

test/codecnoop.jl

@@ -442,9 +442,9 @@ using FillArrays: Zeros
 
     stream = NoopStream(IOBuffer(""))
     unsafe_write(stream, C_NULL, 0)
-    @test eof(stream)  # write
+    @test_throws ArgumentError eof(stream)  # write
     close(stream)
-    @test eof(stream)  # close
+    @test_throws ArgumentError eof(stream)  # close
 
     @testset "readuntil" begin
         stream = NoopStream(IOBuffer(""))

test/codecquadruple.jl

@@ -205,21 +205,21 @@ end
         end
     end
 
-    @testset "eof is true after write" begin
+    @testset "eof throws ArgumentError after write" begin
         sink = IOBuffer()
         stream = TranscodingStream(QuadrupleCodec(), sink, bufsize=16)
         write(stream, "x")
-        @test eof(stream)
+        @test_throws ArgumentError eof(stream)
         @test_throws ArgumentError read(stream, UInt8)
-        @test eof(stream)
+        @test_throws ArgumentError eof(stream)
         write(stream, "y")
-        @test eof(stream)
+        @test_throws ArgumentError eof(stream)
         write(stream, TranscodingStreams.TOKEN_END)
-        @test eof(stream)
+        @test_throws ArgumentError eof(stream)
         flush(stream)
-        @test eof(stream)
+        @test_throws ArgumentError eof(stream)
         @test take!(sink) == b"xxxxyyyy"
         close(stream)
-        @test eof(stream)
+        @test_throws ArgumentError eof(stream)
     end
 end