futures-codec icon indicating copy to clipboard operation
futures-codec copied to clipboard

Consider relaxing Unpin bound on Sink impl for FramedWrite

Open ebkalderon opened this issue 5 years ago • 4 comments

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 avatar Aug 20 '20 12:08 ebkalderon

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?

ebkalderon avatar Aug 23 '20 05:08 ebkalderon

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)

fogti avatar Oct 25 '20 00:10 fogti

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

ebkalderon avatar May 20 '21 01:05 ebkalderon

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

fogti avatar May 20 '21 22:05 fogti