Skip to content

Unpin bound in impl<P> Body for Pin<P> is unnecessarily restrictive #167

Description

@Morgane55440

the current code is :

impl<P> Body for Pin<P>
where
    P: Unpin + ops::DerefMut,
    P::Target: Body,
{
// ....
    fn poll_frame(
        self: Pin<&mut Self>,
        cx: &mut Context<'_>,
    ) -> Poll<Option<Result<Frame<Self::Data>, Self::Error>>> {
        Pin::get_mut(self).as_mut().poll_frame(cx)
    }
// ....
}

th correct code should be

impl<P> Body for Pin<P>
where
    P: ops::DerefMut,
    P::Target: Body,
{
// ....
    fn poll_frame(
        self: Pin<&mut Self>,
        cx: &mut Context<'_>,
    ) -> Poll<Option<Result<Frame<Self::Data>, Self::Error>>> {
        self.as_deref_mut().poll_frame(cx)
    }
// ....
}

without the Unpin bound. this likely has gone unnoticed as a !Unpin pinning pointer is a very strange idea, and all pinning pointers in std are Unpin.

but it is possible for such a type to exist, and thus this bound is making the api unnecessarily restrictive.

edit : i just saw as_deref_mut requires rust 1.84, which would be a significant bumb(prob unaceptably so) .
while there is no doubt this is sound in in earlier versions, i'm unsure wether the addition of unsafe code would be warranted.

maybe something to keep in mind for later , when/if http-body's msrv increases

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions