Repository: rust
Issue: Pin docs should mention pitfalls of generic code
Labels: C-enhancement,P-medium,T-lang,T-libs-api,A-docs,A-async-await,AsyncAwait-Triaged

The [pin module docs](https://doc.rust-lang.org/beta/std/pin/index.html) currently have a note about implementing `Future` combinators:

> When implementing a Future combinator, you will usually need structural pinning for the nested futures, as you need to get pinned references to them to call poll.

However, this can be tricky in the presence of generics. Consider this code.

```rust
use std::future::Future;
use std::task::Context;
use std::pin::Pin;
use std::task::Poll;
use std::marker::{Unpin, PhantomPinned};

mod other_mod {
    use super::*;

    pub enum DubiousDrop {
        First(PhantomPinned),
        Second(PhantomPinned)
    }

    impl Drop for DubiousDrop {
        fn drop(&mut self) {
            std::mem::forget(std::mem::replace(self, match self {
                &mut DubiousDrop::First(_) => DubiousDrop::Second(PhantomPinned),
                &mut DubiousDrop::Second(_) => DubiousDrop::First(PhantomPinned)
            }))
        }
    }

    impl Future for DubiousDrop {
        type Output = ();
        fn poll(self: Pin<&mut Self>, _cx: &mut Context) -> Poll<()> {
            Poll::Ready(())
        }
    }
}


struct MyWrapper<F: Future<Output = ()>> {
    pub fut: F
}

impl<F: Future<Output = ()>> Future for MyWrapper<F> {
    type Output = ();
    
    fn poll(self: Pin<&mut Self>, cx: &mut Context) -> Poll<()> {
        // Unsound - we don't know that 'fut' upholds the
        // guarantees of 'Pin'
        unsafe { self.map_unchecked_mut(|s| &mut s.fut) }.poll(cx)
    }
}

fn assert_unpin<T: Unpin>() {}

fn boom(cx: &mut Context) {
    // Uncomment to trigger a compilation error
    //assert_unpin::<MyWrapper<other_mod::DubiousDrop>>();
    let mut wrapper = Box::pin(MyWrapper { fut: other_mod::DubiousDrop::First(PhantomPinned) });
    Pin::new(&mut wrapper).poll(cx);
}

fn main() {}
```

Here, we have an enum `DubiousDrop`, whose destructor changes the enum variant. This type is slightly contrived but is perfectly safe (in fact, the entire containing module is safe).

We then implement a simple pass-through future combinator called `MyWrapper`, which simply delegates to a wrapped future using a pin projection. Unfortunately, `MyWrapper` is unsound - it constructs a `Pin<&mut F>` without knowing if `F` upholds the `Pin` guarantees. Our `DubiousDrop` enum explicitly does *not* uphold these guarantees - its destructor invalidates its memory by changing the enum variant, and it is not `Unpin`. Therefore, calling `boom` triggers UB in safe code.

AFAICT, there are two ways to prevent this kind of issue:

1. Require that the wrapped `Future` be `Unpin` (in this case, adding an `F: Unpin` bound to `MyWrapper`. This is the approach taken by types like [futures::future::Select](https://rust-lang-nursery.github.io/futures-api-docs/0.3.0-alpha.17/futures/future/struct.Select.html).
2. Make it unsafe to construct the wrapper/combinator type, and document that the caller must uphold the `Pin` requirements on their type.

However, none of this appears to explicitly documented, and it's fairly subtle. I think it's important for the `std::pin` and `std::pin::Pin` docs to make these requirements very clear, as it seems to be somewhat of a footgun.