Change generator trait to use pinning

Author: Nemo157

src/doc/unstable-book/src/language-features/generators.md

@@ -29,18 +29,19 @@ A syntactical example of a generator is:
 #![feature(generators, generator_trait)]
 
 use std::ops::{Generator, GeneratorState};
+use std::pin::Pin;
 
 fn main() {
     let mut generator = || {
         yield 1;
         return "foo"
     };
 
-    match unsafe { generator.resume() } {
+    match Pin::new(&mut generator).resume() {
         GeneratorState::Yielded(1) => {}
         _ => panic!("unexpected value from resume"),
     }
-    match unsafe { generator.resume() } {
+    match Pin::new(&mut generator).resume() {
         GeneratorState::Complete("foo") => {}
         _ => panic!("unexpected value from resume"),
     }
@@ -60,6 +61,7 @@ prints all numbers in order:
 #![feature(generators, generator_trait)]
 
 use std::ops::Generator;
+use std::pin::Pin;
 
 fn main() {
     let mut generator = || {
@@ -69,9 +71,9 @@ fn main() {
     };
 
     println!("1");
-    unsafe { generator.resume() };
+    Pin::new(&mut generator).resume();
     println!("3");
-    unsafe { generator.resume() };
+    Pin::new(&mut generator).resume();
     println!("5");
 }
 ```
@@ -86,13 +88,14 @@ Feedback on the design and usage is always appreciated!
 The `Generator` trait in `std::ops` currently looks like:
 
 ```
-# #![feature(generator_trait)]
+# #![feature(arbitrary_self_types, generator_trait)]
 # use std::ops::GeneratorState;
+# use std::pin::Pin;
 
 pub trait Generator {
     type Yield;
     type Return;
-    unsafe fn resume(&mut self) -> GeneratorState<Self::Yield, Self::Return>;
+    fn resume(self: Pin<&mut Self>) -> GeneratorState<Self::Yield, Self::Return>;
 }
 ```
 
@@ -167,6 +170,7 @@ Let's take a look at an example to see what's going on here:
 #![feature(generators, generator_trait)]
 
 use std::ops::Generator;
+use std::pin::Pin;
 
 fn main() {
     let ret = "foo";
@@ -175,17 +179,18 @@ fn main() {
         return ret
     };
 
-    unsafe { generator.resume() };
-    unsafe { generator.resume() };
+    Pin::new(&mut generator).resume();
+    Pin::new(&mut generator).resume();
 }
 ```
 
 This generator literal will compile down to something similar to:
 
 ```rust
-#![feature(generators, generator_trait)]
+#![feature(arbitrary_self_types, generators, generator_trait)]
 
 use std::ops::{Generator, GeneratorState};
+use std::pin::Pin;
 
 fn main() {
     let ret = "foo";
@@ -200,9 +205,9 @@ fn main() {
             type Yield = i32;
             type Return = &'static str;
 
-            unsafe fn resume(&mut self) -> GeneratorState<i32, &'static str> {
+            fn resume(mut self: Pin<&mut Self>) -> GeneratorState<i32, &'static str> {
                 use std::mem;
-                match mem::replace(self, __Generator::Done) {
+                match mem::replace(&mut *self, __Generator::Done) {
                     __Generator::Start(s) => {
                         *self = __Generator::Yield1(s);
                         GeneratorState::Yielded(1)
@@ -223,8 +228,8 @@ fn main() {
         __Generator::Start(ret)
     };
 
-    unsafe { generator.resume() };
-    unsafe { generator.resume() };
+    Pin::new(&mut generator).resume();
+    Pin::new(&mut generator).resume();
 }
 ```
 

src/liballoc/boxed.rs

@@ -873,13 +873,12 @@ impl<T: ?Sized> AsMut<T> for Box<T> {
 impl<T: ?Sized> Unpin for Box<T> { }
 
 #[unstable(feature = "generator_trait", issue = "43122")]
-impl<T> Generator for Box<T>
-    where T: Generator + ?Sized
-{
-    type Yield = T::Yield;
-    type Return = T::Return;
-    unsafe fn resume(&mut self) -> GeneratorState<Self::Yield, Self::Return> {
-        (**self).resume()
+impl<G: ?Sized + Generator + Unpin> Generator for Box<G> {
+    type Yield = G::Yield;
+    type Return = G::Return;
+
+    fn resume(mut self: Pin<&mut Self>) -> GeneratorState<Self::Yield, Self::Return> {
+        G::resume(Pin::new(&mut *self))
     }
 }
 

src/libcore/ops/generator.rs

@@ -1,3 +1,6 @@
+use crate::marker::Unpin;
+use crate::pin::Pin;
+
 /// The result of a generator resumption.
 ///
 /// This enum is returned from the `Generator::resume` method and indicates the
@@ -39,18 +42,19 @@ pub enum GeneratorState<Y, R> {
 /// #![feature(generators, generator_trait)]
 ///
 /// use std::ops::{Generator, GeneratorState};
+/// use std::pin::Pin;
 ///
 /// fn main() {
 ///     let mut generator = || {
 ///         yield 1;
 ///         return "foo"
 ///     };
 ///
-///     match unsafe { generator.resume() } {
+///     match Pin::new(&mut generator).resume() {
 ///         GeneratorState::Yielded(1) => {}
 ///         _ => panic!("unexpected return from resume"),
 ///     }
-///     match unsafe { generator.resume() } {
+///     match Pin::new(&mut generator).resume() {
 ///         GeneratorState::Complete("foo") => {}
 ///         _ => panic!("unexpected return from resume"),
 ///     }
@@ -88,10 +92,6 @@ pub trait Generator {
     /// generator will continue executing until it either yields or returns, at
     /// which point this function will return.
     ///
-    /// The function is unsafe because it can be used on an immovable generator.
-    /// After such a call, the immovable generator must not move again, but
-    /// this is not enforced by the compiler.
-    ///
     /// # Return value
     ///
     /// The `GeneratorState` enum returned from this function indicates what
@@ -110,16 +110,25 @@ pub trait Generator {
     /// been returned previously. While generator literals in the language are
     /// guaranteed to panic on resuming after `Complete`, this is not guaranteed
     /// for all implementations of the `Generator` trait.
-    unsafe fn resume(&mut self) -> GeneratorState<Self::Yield, Self::Return>;
+    fn resume(self: Pin<&mut Self>) -> GeneratorState<Self::Yield, Self::Return>;
+}
+
+#[unstable(feature = "generator_trait", issue = "43122")]
+impl<G: ?Sized + Generator> Generator for Pin<&mut G> {
+    type Yield = G::Yield;
+    type Return = G::Return;
+
+    fn resume(mut self: Pin<&mut Self>) -> GeneratorState<Self::Yield, Self::Return> {
+        G::resume((*self).as_mut())
+    }
 }
 
 #[unstable(feature = "generator_trait", issue = "43122")]
-impl<T> Generator for &mut T
-    where T: Generator + ?Sized
-{
-    type Yield = T::Yield;
-    type Return = T::Return;
-    unsafe fn resume(&mut self) -> GeneratorState<Self::Yield, Self::Return> {
-        (**self).resume()
+impl<G: ?Sized + Generator + Unpin> Generator for &mut G {
+    type Yield = G::Yield;
+    type Return = G::Return;
+
+    fn resume(mut self: Pin<&mut Self>) -> GeneratorState<Self::Yield, Self::Return> {
+        G::resume(Pin::new(&mut *self))
     }
 }

src/librustc_mir/diagnostics.rs

@@ -2119,14 +2119,15 @@ This error occurs because a borrow in a generator persists across a
 yield point.
 
 ```compile_fail,E0626
-# #![feature(generators, generator_trait)]
+# #![feature(generators, generator_trait, pin)]
 # use std::ops::Generator;
+# use std::pin::Pin;
 let mut b = || {
     let a = &String::new(); // <-- This borrow...
     yield (); // ...is still in scope here, when the yield occurs.
     println!("{}", a);
 };
-unsafe { b.resume() };
+Pin::new(&mut b).resume();
 ```
 
 At present, it is not permitted to have a yield that occurs while a
@@ -2137,14 +2138,15 @@ resolve the previous example by removing the borrow and just storing
 the integer by value:
 
 ```
-# #![feature(generators, generator_trait)]
+# #![feature(generators, generator_trait, pin)]
 # use std::ops::Generator;
+# use std::pin::Pin;
 let mut b = || {
     let a = 3;
     yield ();
     println!("{}", a);
 };
-unsafe { b.resume() };
+Pin::new(&mut b).resume();
 ```
 
 This is a very simple case, of course. In more complex cases, we may
@@ -2154,37 +2156,40 @@ in those cases, something like the `Rc` or `Arc` types may be useful.
 This error also frequently arises with iteration:
 
 ```compile_fail,E0626
-# #![feature(generators, generator_trait)]
+# #![feature(generators, generator_trait, pin)]
 # use std::ops::Generator;
+# use std::pin::Pin;
 let mut b = || {
   let v = vec![1,2,3];
   for &x in &v { // <-- borrow of `v` is still in scope...
     yield x; // ...when this yield occurs.
   }
 };
-unsafe { b.resume() };
+Pin::new(&mut b).resume();
 ```
 
 Such cases can sometimes be resolved by iterating "by value" (or using
 `into_iter()`) to avoid borrowing:
 
 ```
-# #![feature(generators, generator_trait)]
+# #![feature(generators, generator_trait, pin)]
 # use std::ops::Generator;
+# use std::pin::Pin;
 let mut b = || {
   let v = vec![1,2,3];
   for x in v { // <-- Take ownership of the values instead!
     yield x; // <-- Now yield is OK.
   }
 };
-unsafe { b.resume() };
+Pin::new(&mut b).resume();
 ```
 
 If taking ownership is not an option, using indices can work too:
 
 ```
-# #![feature(generators, generator_trait)]
+# #![feature(generators, generator_trait, pin)]
 # use std::ops::Generator;
+# use std::pin::Pin;
 let mut b = || {
   let v = vec![1,2,3];
   let len = v.len(); // (*)
@@ -2193,7 +2198,7 @@ let mut b = || {
     yield x; // <-- Now yield is OK.
   }
 };
-unsafe { b.resume() };
+Pin::new(&mut b).resume();
 
 // (*) -- Unfortunately, these temporaries are currently required.
 // See <https://github.com/rust-lang/rust/issues/43122>.

src/libstd/future.rs

@@ -33,7 +33,9 @@ impl<T: Generator<Yield = ()>> !Unpin for GenFuture<T> {}
 impl<T: Generator<Yield = ()>> Future for GenFuture<T> {
     type Output = T::Return;
     fn poll(self: Pin<&mut Self>, lw: &LocalWaker) -> Poll<Self::Output> {
-        set_task_waker(lw, || match unsafe { Pin::get_unchecked_mut(self).0.resume() } {
+        // Safe because we're !Unpin + !Drop mapping to a ?Unpin value
+        let gen = unsafe { Pin::map_unchecked_mut(self, |s| &mut s.0) };
+        set_task_waker(lw, || match gen.resume() {
             GeneratorState::Yielded(()) => Poll::Pending,
             GeneratorState::Complete(x) => Poll::Ready(x),
         })

src/test/run-pass/drop/dynamic-drop.rs

@@ -12,6 +12,7 @@
 use std::cell::{Cell, RefCell};
 use std::ops::Generator;
 use std::panic;
+use std::pin::Pin;
 use std::usize;
 
 struct InjectedFailure;
@@ -172,7 +173,7 @@ fn generator(a: &Allocator, run_count: usize) {
          );
     };
     for _ in 0..run_count {
-        unsafe { gen.resume(); }
+        Pin::new(&mut gen).resume();
     }
 }
 

src/test/run-pass/generator/auxiliary/xcrate.rs

@@ -1,5 +1,6 @@
 #![feature(generators, generator_trait)]
 
+use std::marker::Unpin;
 use std::ops::Generator;
 
 pub fn foo() -> impl Generator<Yield = (), Return = ()> {
@@ -10,7 +11,7 @@ pub fn foo() -> impl Generator<Yield = (), Return = ()> {
     }
 }
 
-pub fn bar<T: 'static>(t: T) -> Box<Generator<Yield = T, Return = ()>> {
+pub fn bar<T: Unpin + 'static>(t: T) -> Box<Generator<Yield = T, Return = ()> + Unpin> {
     Box::new(|| {
         yield t;
     })

src/test/run-pass/generator/conditional-drop.rs

@@ -3,6 +3,7 @@
 #![feature(generators, generator_trait)]
 
 use std::ops::Generator;
+use std::pin::Pin;
 use std::sync::atomic::{AtomicUsize, Ordering};
 
 static A: AtomicUsize = AtomicUsize::new(0);
@@ -34,9 +35,9 @@ fn t1() {
     };
 
     let n = A.load(Ordering::SeqCst);
-    unsafe { a.resume() };
+    Pin::new(&mut a).resume();
     assert_eq!(A.load(Ordering::SeqCst), n + 1);
-    unsafe { a.resume() };
+    Pin::new(&mut a).resume();
     assert_eq!(A.load(Ordering::SeqCst), n + 1);
 }
 
@@ -50,8 +51,8 @@ fn t2() {
     };
 
     let n = A.load(Ordering::SeqCst);
-    unsafe { a.resume() };
+    Pin::new(&mut a).resume();
     assert_eq!(A.load(Ordering::SeqCst), n);
-    unsafe { a.resume() };
+    Pin::new(&mut a).resume();
     assert_eq!(A.load(Ordering::SeqCst), n + 1);
 }

src/test/run-pass/generator/control-flow.rs

@@ -2,13 +2,15 @@
 
 #![feature(generators, generator_trait)]
 
+use std::marker::Unpin;
 use std::ops::{GeneratorState, Generator};
+use std::pin::Pin;
 
 fn finish<T>(mut amt: usize, mut t: T) -> T::Return
-    where T: Generator<Yield = ()>
+    where T: Generator<Yield = ()> + Unpin,
 {
     loop {
-        match unsafe { t.resume() } {
+        match Pin::new(&mut t).resume() {
             GeneratorState::Yielded(()) => amt = amt.checked_sub(1).unwrap(),
             GeneratorState::Complete(ret) => {
                 assert_eq!(amt, 0);