Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make write(io, Any[...]) an error #6466

Closed
mbauman opened this issue Apr 8, 2014 · 5 comments
Closed

make write(io, Any[...]) an error #6466

mbauman opened this issue Apr 8, 2014 · 5 comments
Assignees
Labels
domain:io Involving the I/O subsystem: libuv, read, write, etc. kind:bug Indicates an unexpected problem or unintended behavior
Milestone

Comments

@mbauman
Copy link
Sponsor Member

mbauman commented Apr 8, 2014

I noticed this with IOBuffer, but it looks like IOStream, AsyncStream, and File all have the same issue. In each of their read!{T,N}(::IO, a::Array{T,N}) methods, they assume that the number of bytes to read is length(a)*sizeof(T). This is not true for Char:

julia> buf = IOBuffer("abcdefghijklmnop");

julia> read(buf, Char)
'a'

julia> read(buf, Char, 2) # dispatches to read!(IOBuffer, Array{Char,1})
2-element Array{Char,1}:
 '\U65646362'
 '\U69686766'

For IOBuffer, I think the specialization could simply be removed (albeit perhaps at a small performance loss). The others, however, are trickier — File performance is much more critical and the Async/IOStream ones need to know how many bytes to wait for.

@JeffBezanson
Copy link
Sponsor Member

I think the best solution here is to better separate text and binary I/O. If you want to serialize an Array{Char}, then arguably text encoding is beside the point and it makes sense to just dump the bits.

@kshyatt
Copy link
Contributor

kshyatt commented Jan 25, 2017

This is reproducible on today's master.

@StefanKarpinski StefanKarpinski added this to the 1.0 milestone Jan 30, 2017
@JeffBezanson JeffBezanson self-assigned this Jul 20, 2017
@JeffBezanson
Copy link
Sponsor Member

This has gotten slightly better since we changed read(io, Char, 2) to read!(io, Vector{Char}(2)), which makes it clear you're reading an Array, and that Array determines the representation.

One question is what read(io, Char) should do. Currently it's UTF-8; is that the right thing to do?

Another issue is writing non-isbits arrays, or Any arrays. Currently we handle these by calling write in a loop, but this is a strange thing to do since you'll end up with a basically unusable binary representation. Arguably you should have to write the loop yourself if you really want this. Triage thinks we should make this case an error.

@JeffBezanson JeffBezanson changed the title Some IO subtypes ignore type-specific read methods when reading into an Array make write(io, Any[...]) an error Jul 20, 2017
@StefanKarpinski
Copy link
Sponsor Member

Shouldn't it be an error to serialize any pointer array?

@JeffBezanson
Copy link
Sponsor Member

Yes, I was using Any as a shorthand for that.

JeffBezanson added a commit that referenced this issue Jul 24, 2017
Also remove an unnecessary method, and generalize a `write` method
for `SubArray` from `::IOStream` to `::IO`.
JeffBezanson added a commit that referenced this issue Jul 24, 2017
Also remove an unnecessary method, and generalize a `write` method
for `SubArray` from `::IOStream` to `::IO`.
JeffBezanson added a commit that referenced this issue Jul 28, 2017
Also remove an unnecessary method, and generalize a `write` method
for `SubArray` from `::IOStream` to `::IO`.
JeffBezanson added a commit that referenced this issue Jul 28, 2017
fix #6466, deprecate `write` on non-isbits arrays
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:io Involving the I/O subsystem: libuv, read, write, etc. kind:bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

4 participants