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

Consider relaxing Unpin bound on Sink impl for FramedWrite #46

Open
ebkalderon opened this issue Aug 20, 2020 · 4 comments
Open

Consider relaxing Unpin bound on Sink impl for FramedWrite #46

ebkalderon opened this issue Aug 20, 2020 · 4 comments

Comments

@ebkalderon
Copy link

ebkalderon commented Aug 20, 2020

It appears that the Sink trait implementation for tokio_util::codec::FramedWrite requires only AsyncWrite and not AsyncWrite + Unpin. Is there a chance that futures_codec::FramedWrite could do the same?

As of today, I cannot invoke StreamExt::forward() on a futures_codec::FramedWrite which wraps a non-Unpin type.

@ebkalderon
Copy link
Author

ebkalderon commented Aug 23, 2020

In tokio-util, FramedWrite contains a #[pin] field of type FramedImpl (source), which contains the inner AsyncWrite field behind a #[pin] and the codec which is not (source). This means that one can do self.project() and extract the pinned AsyncWrite and unpinned codec, as needed for implementing Sink.

In this crate, however, there seems to be an extra layer: FramedWrite contains a FramedWrite2 which contains a Fuse which contains the inner AsyncWrite field and codec together. The fact that the codec is behind a Pin when it doesn't need to be unfortunately requires it to also be Unpin. Could this be restructured? Why does the double layering of FramedWrite2 and Fuse exist? Couldn't FramedWrite2 contain an inner: T and codec: E directly?

@fogti
Copy link
Contributor

fogti commented Oct 25, 2020

I tried to restructure and refactor this crate, WIP version @ https://github.com/YZITE/futures-codec (which e.g. gets rid of the Unpin requirement)

@ebkalderon
Copy link
Author

For future readers: since the above repo no longer exists, there is now a similar crate at silvanshade/async-codec-lite.

@fogti
Copy link
Contributor

fogti commented May 20, 2021

ah sorry, moved it here: https://github.com/YZITE/futures/tree/main/crates/codec

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

No branches or pull requests

2 participants