From 31f5ee8b1f2ded71ddd6fce55974dd5021b88bf1 Mon Sep 17 00:00:00 2001 From: Naja Melan Date: Wed, 4 Mar 2020 20:12:02 +0100 Subject: [PATCH] Don't require Unpin when pin projecting. There are two scenarios here. Either you want users to be able to pass in streams that are `!Unpin`, in which case pin_project and the compiler have your back and make sure this works, or... You are going to require Unpin on it in which case you can remove the pin projection. Both together don't make sense. Either one works here, but by removing the Unpin the API is less restrictive, which I would favor, even if I don't immediately see what concrete `!Unpin` type someone might want to call this with, but sooner or later someone probably will, so why disable if it can work. This leaves another problem though, there are many other places in the lib where `Unpin` is required, notably on the wrappers around `Stream`, so it's not said that with this change users can actually use any `!Unpin` streams. There is one other use of pin-project on `MidHandshake`. As far as I can tell the effect of this one is that `Role::InternalStream` can now be `!Unpin` in this specific file, but is it even possible to use the lib with an `!Unpin` type for this? It looks like it might be good to rethink the choices about pin for the entire lib and make something consistent. Maybe aim for the largest API compatibility by trying to never require `Unpin` and seeing if pin-project makes that possible. In any case, the Unpin in `Stream` has no good reason for being there unless you would like to [get rid of pin-project](https://github.com/najamelan/tokio-tungstenite/commit/18cc347aaa7f106b28f54ed38b0a030130ffbc1f). As an added bonus, if you pin project, there is no need for re-pinning in the match. --- src/stream.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/stream.rs b/src/stream.rs index ad48769..87e4915 100644 --- a/src/stream.rs +++ b/src/stream.rs @@ -18,7 +18,7 @@ pub enum Stream { Tls(#[pin] T), } -impl AsyncRead for Stream { +impl AsyncRead for Stream { #[project] fn poll_read( self: Pin<&mut Self>, @@ -27,13 +27,13 @@ impl AsyncRead for Stream { ) -> Poll> { #[project] match self.project() { - Stream::Plain(ref mut s) => Pin::new(s).poll_read(cx, buf), - Stream::Tls(ref mut s) => Pin::new(s).poll_read(cx, buf), + Stream::Plain(s) => s.poll_read(cx, buf), + Stream::Tls(s) => s.poll_read(cx, buf), } } } -impl AsyncWrite for Stream { +impl AsyncWrite for Stream { #[project] fn poll_write( self: Pin<&mut Self>, @@ -42,8 +42,8 @@ impl AsyncWrite for Stream { ) -> Poll> { #[project] match self.project() { - Stream::Plain(ref mut s) => Pin::new(s).poll_write(cx, buf), - Stream::Tls(ref mut s) => Pin::new(s).poll_write(cx, buf), + Stream::Plain(s) => s.poll_write(cx, buf), + Stream::Tls(s) => s.poll_write(cx, buf), } } @@ -51,8 +51,8 @@ impl AsyncWrite for Stream { fn poll_flush(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { #[project] match self.project() { - Stream::Plain(ref mut s) => Pin::new(s).poll_flush(cx), - Stream::Tls(ref mut s) => Pin::new(s).poll_flush(cx), + Stream::Plain(s) => s.poll_flush(cx), + Stream::Tls(s) => s.poll_flush(cx), } } @@ -63,8 +63,8 @@ impl AsyncWrite for Stream { ) -> Poll> { #[project] match self.project() { - Stream::Plain(ref mut s) => Pin::new(s).poll_shutdown(cx), - Stream::Tls(ref mut s) => Pin::new(s).poll_shutdown(cx), + Stream::Plain(s) => s.poll_shutdown(cx), + Stream::Tls(s) => s.poll_shutdown(cx), } } }