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

impl AsyncWrite for Cursor<Vec<u8>> doesn't extend vector as needed #1510

Closed
Nemo157 opened this issue Apr 3, 2019 · 5 comments · Fixed by #1936
Closed

impl AsyncWrite for Cursor<Vec<u8>> doesn't extend vector as needed #1510

Nemo157 opened this issue Apr 3, 2019 · 5 comments · Fixed by #1936
Milestone

Comments

@Nemo157
Copy link
Member

Nemo157 commented Apr 3, 2019

This implementation is currently provided by impl<T: AsMut<[u8]>> AsyncWrite for Cursor<T> which doesn't allow it to know that the vector is extendable. std provides a separate impl io::Write for Cursor<Vec<u8>> which extends the vector as needed, but this means it can't provide a generic version.

We're also missing the direct impl AsyncWrite for Vec<u8> which could be used instead (EDIT: Added in #1511). It almost seems like we should drop the Cursor wrapped implementations until we get an AsyncSeek trait available?

@Nemo157
Copy link
Member Author

Nemo157 commented Apr 3, 2019

Actually the Cursor implementations are still useful as they allow reading off how many bytes were read/written after operating on it.


One thing that may be worth looking into is whether std plans to use specialization to get both generic implementations and an override that extends Vec as needed, that seems to me to be the best endpoint, but maybe there are reasons not to do it.

@yoshuawuyts
Copy link
Member

until we get an AsyncSeek trait available?

Chiming in here: I've def been missing an AsyncSeek impl. We've been looking to implement a filesystem API for Romio, and that's one of the traits we need. Tokio has a counterpart directly implemented on the File struct, but it seems being able to generalize it would be ideal.

@Nemo157
Copy link
Member Author

Nemo157 commented Jun 7, 2019

Test commit providing this behaviour via specialization: 7997066.

@cramertj
Copy link
Member

@Nemo157

It almost seems like we should drop the Cursor wrapped implementations until we get an AsyncSeek trait available?

#1553 added AsyncSeek-- what would you like to see happen on this issue?

@Nemo157
Copy link
Member Author

Nemo157 commented Nov 3, 2019

The main issue is still that impl std::io::Write for Cursor<Vec<u8>> and impl futures::io::AsyncWrite for Cursor<Vec<u8>> behave differently, the former will extend the vector as data is written to fit it, the latter will return EOF when attempting to write past the current length.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants