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

Read::read_exact works with StdinLock, but not with Cursor<StdinLock>? #52470

Closed
tko opened this issue Jul 17, 2018 · 5 comments
Closed

Read::read_exact works with StdinLock, but not with Cursor<StdinLock>? #52470

tko opened this issue Jul 17, 2018 · 5 comments
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools

Comments

@tko
Copy link
Contributor

tko commented Jul 17, 2018

I've some code reading a stream from stdin. Wanted to wrap it in a Cursor to have it keep track of the number of bytes read already instead of having to do it manually, but started getting odd compiler errors.

Since read_exact works just fine with StdinLock (and Stdin) shouldn't it work just the same with Cursor<StdinLock>?

use std::io::{self, Cursor, Read};
fn main() {
    let mut stdin = io::stdin(); // works with this
    let mut stdin = stdin.lock(); // works with this too
    let mut stdin = Cursor::new(stdin); // but both/either become invalid if wrapped in Cursor?
    let _b = read_u8(&mut stdin).expect("ok");
}
fn read_u8<R: Read>(r: &mut R) -> io::Result<u8> {
    let mut buf = [0u8; 1];
    r.read_exact(&mut buf)?;
    Ok(buf[0])
}

(Playground)

Errors:

   Compiling playground v0.0.1 (file:///playground)
error[E0277]: the trait bound `std::io::StdinLock<'_>: std::convert::AsRef<[u8]>` is not satisfied
 --> src/main.rs:6:14
  |
6 |     let _b = read_u8(&mut stdin).expect("ok");
  |              ^^^^^^^ the trait `std::convert::AsRef<[u8]>` is not implemented for `std::io::StdinLock<'_>`
  |
  = note: required because of the requirements on the impl of `std::io::Read` for `std::io::Cursor<std::io::StdinLock<'_>>`
note: required by `read_u8`
 --> src/main.rs:8:1
  |
8 | fn read_u8<R: Read>(r: &mut R) -> io::Result<u8> {
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
error: Could not compile `playground`.

To learn more, run the command again with --verbose.

@bjorn3
Copy link
Member

bjorn3 commented Jul 18, 2018

A Cursor is meant as a wrapper to Vec or &mut [u8] to be able to write to them. The compiler error says that StdinLock cant be converted to a slice.

@tko
Copy link
Contributor Author

tko commented Jul 18, 2018

That's not how I understood it from the documentation.

A Cursor wraps another type and provides it with a Seek implementation.

to me that suggests the main purpose is to provide Seek implementation.

Cursors are typically used with in-memory buffers ...

If it's only meant to wrap Vec or [u8] I think it would avoid misunderstandings to say so out right.

Creates a new cursor wrapping the provided underlying I/O object.

Again suggesting Cursor is fairly generic about underlying I/O object. (FWIW if asked to name some I/O objects, I'd be pressed to consider Vec and [u8] as such.)

...

The compiler error is confusing as at no point is there any (obvious) attempt to convert StdinLock into a slice. It's only ever used through the Read trait. Though looking more closely I now see the fine print later on (where T: AsRef<[u8]>) so I guess one could argue this is just a minor documentation issue (+ user error.)

I'd still argue the behavior is surprising. StdinLock implements Read, so does Cursor, there's no obvious reason why Cursor<StdinLock> wouldn't. I can sort of understand why it's currently that way, and can imagine some problems there may be generalizing it, but can't quite see any fundamental reasons for Cursor to be restricted to only Vec and [u8] and the documentation is vague enough to make that seem like a bug / unintended omission rather than intentional design / limitation.

@kennytm kennytm added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Jul 18, 2018
@sfackler
Copy link
Member

It is a docs bug. The entire purpose of Cursor is to take a buffer of bytes and turn that into a reader by adding a cursor indicating the current position in the buffer.

tko added a commit to tko/rust that referenced this issue Jul 19, 2018
Reduce misconceptions about Cursor being more general than it really is.

Fixes: rust-lang#52470
@tko
Copy link
Contributor Author

tko commented Jul 19, 2018

Made a PR to suggest some documentation changes to better describe the intention.

I do wonder though if the AsRef<[u8]> guard should also be included for new, it could capture the problem more accurately where it's happening?

@sfackler
Copy link
Member

There probably should have been an AsRef<[u8]> bound on new, but we can't add it now.

kennytm added a commit to kennytm/rust that referenced this issue Jul 24, 2018
Cursor: update docs to clarify Cursor only works with in-memory buffers

Reduce misconceptions about Cursor being more general than it really is.

Fixes: rust-lang#52470
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools
Projects
None yet
Development

No branches or pull requests

4 participants