Auto merge of #55704 - Nemo157:pinned-generators, r=Zoxc

Use pinning for generators to make trait safe

I'm unsure whether there needs to be any changes to the actual generator transform. Tests are passing so the fact that `Pin<&mut T>` is fundamentally the same as `&mut T` seems to allow it to still work, but maybe there's something subtle here that could go wrong.

This is specified in [RFC 2349 § Immovable generators](https://github.com/rust-lang/rfcs/blob/master/text/2349-pin.md#immovable-generators) (although, since that RFC it has become safe to create an immovable generator, and instead it's unsafe to resume any generator; with these changes both are now safe and instead the unsafety is moved to creating a `Pin<&mut [static generator]>` which there are safe APIs for).

CC #43122

Author: bors

Diff for: 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();
 }
 ```
 

Diff for: src/liballoc/boxed.rs

@@ -873,13 +873,22 @@ 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))
+    }
+}
+
+#[unstable(feature = "generator_trait", issue = "43122")]
+impl<G: ?Sized + Generator> Generator for Pin<Box<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())
     }
 }
 

Diff for: src/libcore/marker.rs

@@ -627,6 +627,7 @@ unsafe impl<T: ?Sized> Freeze for &mut T {}
 /// [`Pin`]: ../pin/struct.Pin.html
 /// [`pin module`]: ../../std/pin/index.html
 #[stable(feature = "pin", since = "1.33.0")]
+#[cfg_attr(not(stage0), lang = "unpin")]
 pub auto trait Unpin {}
 
 /// A marker type which does not implement `Unpin`.

Diff for: 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))
     }
 }

Diff for: src/libcore/pin.rs

@@ -117,6 +117,7 @@ use ops::{Deref, DerefMut, Receiver, CoerceUnsized, DispatchFromDyn};
 // implementations, are allowed because they all only use `&P`, so they cannot move
 // the value behind `pointer`.
 #[stable(feature = "pin", since = "1.33.0")]
+#[cfg_attr(not(stage0), lang = "pin")]
 #[fundamental]
 #[repr(transparent)]
 #[derive(Copy, Clone, Hash, Eq, Ord)]

Diff for: src/librustc/middle/lang_items.rs

@@ -299,6 +299,8 @@ language_item_table! {
 
     GeneratorStateLangItem,      "generator_state",    gen_state,               Target::Enum;
     GeneratorTraitLangItem,      "generator",          gen_trait,               Target::Trait;
+    UnpinTraitLangItem,          "unpin",              unpin_trait,             Target::Trait;
+    PinTypeLangItem,             "pin",                pin_type,                Target::Struct;
 
     EqTraitLangItem,             "eq",                 eq_trait,                Target::Trait;
     PartialOrdTraitLangItem,     "partial_ord",        partial_ord_trait,       Target::Trait;

Diff for: src/librustc/traits/select.rs

@@ -2017,6 +2017,24 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                     // the auto impl might apply, we don't know
                     candidates.ambiguous = true;
                 }
+                ty::Generator(_, _, movability)
+                    if self.tcx().lang_items().unpin_trait() == Some(def_id) =>
+                {
+                    match movability {
+                        hir::GeneratorMovability::Static => {
+                            // Immovable generators are never `Unpin`, so
+                            // suppress the normal auto-impl candidate for it.
+                        }
+                        hir::GeneratorMovability::Movable => {
+                            // Movable generators are always `Unpin`, so add an
+                            // unconditional builtin candidate.
+                            candidates.vec.push(BuiltinCandidate {
+                                has_nested: false,
+                            });
+                        }
+                    }
+                }
+
                 _ => candidates.vec.push(AutoImplCandidate(def_id.clone())),
             }
         }

Diff for: src/librustc/ty/instance.rs

@@ -76,6 +76,11 @@ impl<'a, 'tcx> Instance<'tcx> {
                 let env_region = ty::ReLateBound(ty::INNERMOST, ty::BrEnv);
                 let env_ty = tcx.mk_mut_ref(tcx.mk_region(env_region), ty);
 
+                let pin_did = tcx.lang_items().pin_type().unwrap();
+                let pin_adt_ref = tcx.adt_def(pin_did);
+                let pin_substs = tcx.intern_substs(&[env_ty.into()]);
+                let env_ty = tcx.mk_adt(pin_adt_ref, pin_substs);
+
                 sig.map_bound(|sig| {
                     let state_did = tcx.lang_items().gen_state().unwrap();
                     let state_adt_ref = tcx.adt_def(state_did);

Diff for: src/librustc_codegen_ssa/mir/mod.rs

@@ -586,10 +586,17 @@ fn arg_local_refs<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>>(
                 return;
             }
 
+            let pin_did = tcx.lang_items().pin_type();
             // Or is it the closure environment?
             let (closure_layout, env_ref) = match arg.layout.ty.sty {
                 ty::RawPtr(ty::TypeAndMut { ty, .. }) |
                 ty::Ref(_, ty, _)  => (bx.layout_of(ty), true),
+                ty::Adt(def, substs) if Some(def.did) == pin_did => {
+                    match substs.type_at(0).sty {
+                        ty::Ref(_, ty, _)  => (bx.layout_of(ty), true),
+                        _ => (arg.layout, false),
+                    }
+                }
                 _ => (arg.layout, false)
             };
 

Diff for: 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>.