Rollup merge of #49194 - Zoxc:unsafe-generator, r=cramertj

Make resuming generators unsafe instead of the creation of immovable generators

Fixes #47787

Author: kennytm

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

@@ -36,11 +36,11 @@ fn main() {
         return "foo"
     };
 
-    match generator.resume() {
+    match unsafe { generator.resume() } {
         GeneratorState::Yielded(1) => {}
         _ => panic!("unexpected value from resume"),
     }
-    match generator.resume() {
+    match unsafe { generator.resume() } {
         GeneratorState::Complete("foo") => {}
         _ => panic!("unexpected value from resume"),
     }
@@ -69,9 +69,9 @@ fn main() {
     };
 
     println!("1");
-    generator.resume();
+    unsafe { generator.resume() };
     println!("3");
-    generator.resume();
+    unsafe { generator.resume() };
     println!("5");
 }
 ```
@@ -92,7 +92,7 @@ The `Generator` trait in `std::ops` currently looks like:
 pub trait Generator {
     type Yield;
     type Return;
-    fn resume(&mut self) -> GeneratorState<Self::Yield, Self::Return>;
+    unsafe fn resume(&mut self) -> GeneratorState<Self::Yield, Self::Return>;
 }
 ```
 
@@ -175,8 +175,8 @@ fn main() {
         return ret
     };
 
-    generator.resume();
-    generator.resume();
+    unsafe { generator.resume() };
+    unsafe { generator.resume() };
 }
 ```
 
@@ -200,7 +200,7 @@ fn main() {
             type Yield = i32;
             type Return = &'static str;
 
-            fn resume(&mut self) -> GeneratorState<i32, &'static str> {
+            unsafe fn resume(&mut self) -> GeneratorState<i32, &'static str> {
                 use std::mem;
                 match mem::replace(self, __Generator::Done) {
                     __Generator::Start(s) => {
@@ -223,8 +223,8 @@ fn main() {
         __Generator::Start(ret)
     };
 
-    generator.resume();
-    generator.resume();
+    unsafe { generator.resume() };
+    unsafe { generator.resume() };
 }
 ```
 

src/liballoc/boxed.rs

@@ -892,7 +892,7 @@ impl<T> Generator for Box<T>
 {
     type Yield = T::Yield;
     type Return = T::Return;
-    fn resume(&mut self) -> GeneratorState<Self::Yield, Self::Return> {
+    unsafe fn resume(&mut self) -> GeneratorState<Self::Yield, Self::Return> {
         (**self).resume()
     }
 }

src/libcore/ops/generator.rs

@@ -56,11 +56,11 @@ pub enum GeneratorState<Y, R> {
 ///         return "foo"
 ///     };
 ///
-///     match generator.resume() {
+///     match unsafe { generator.resume() } {
 ///         GeneratorState::Yielded(1) => {}
 ///         _ => panic!("unexpected return from resume"),
 ///     }
-///     match generator.resume() {
+///     match unsafe { generator.resume() } {
 ///         GeneratorState::Complete("foo") => {}
 ///         _ => panic!("unexpected return from resume"),
 ///     }
@@ -98,6 +98,10 @@ 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
@@ -116,7 +120,7 @@ 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.
-    fn resume(&mut self) -> GeneratorState<Self::Yield, Self::Return>;
+    unsafe fn resume(&mut self) -> GeneratorState<Self::Yield, Self::Return>;
 }
 
 #[unstable(feature = "generator_trait", issue = "43122")]
@@ -125,7 +129,7 @@ impl<'a, T> Generator for &'a mut T
 {
     type Yield = T::Yield;
     type Return = T::Return;
-    fn resume(&mut self) -> GeneratorState<Self::Yield, Self::Return> {
+    unsafe fn resume(&mut self) -> GeneratorState<Self::Yield, Self::Return> {
         (**self).resume()
     }
 }

src/librustc_mir/diagnostics.rs

@@ -2247,7 +2247,7 @@ let mut b = || {
     yield (); // ...is still in scope here, when the yield occurs.
     println!("{}", a);
 };
-b.resume();
+unsafe { b.resume() };
 ```
 
 At present, it is not permitted to have a yield that occurs while a
@@ -2265,7 +2265,7 @@ let mut b = || {
     yield ();
     println!("{}", a);
 };
-b.resume();
+unsafe { b.resume() };
 ```
 
 This is a very simple case, of course. In more complex cases, we may
@@ -2283,7 +2283,7 @@ let mut b = || {
     yield x; // ...when this yield occurs.
   }
 };
-b.resume();
+unsafe { b.resume() };
 ```
 
 Such cases can sometimes be resolved by iterating "by value" (or using
@@ -2298,7 +2298,7 @@ let mut b = || {
     yield x; // <-- Now yield is OK.
   }
 };
-b.resume();
+unsafe { b.resume() };
 ```
 
 If taking ownership is not an option, using indices can work too:
@@ -2314,7 +2314,7 @@ let mut b = || {
     yield x; // <-- Now yield is OK.
   }
 };
-b.resume();
+unsafe { b.resume() };
 
 // (*) -- Unfortunately, these temporaries are currently required.
 // See <https://github.com/rust-lang/rust/issues/43122>.

src/librustc_mir/transform/check_unsafety.rs

@@ -126,21 +126,13 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
                 &AggregateKind::Array(..) |
                 &AggregateKind::Tuple |
                 &AggregateKind::Adt(..) => {}
-                &AggregateKind::Closure(def_id, _) => {
+                &AggregateKind::Closure(def_id, _) |
+                &AggregateKind::Generator(def_id, _, _) => {
                     let UnsafetyCheckResult {
                         violations, unsafe_blocks
                     } = self.tcx.unsafety_check_result(def_id);
                     self.register_violations(&violations, &unsafe_blocks);
                 }
-                &AggregateKind::Generator(def_id, _, interior) => {
-                    let UnsafetyCheckResult {
-                        violations, unsafe_blocks
-                    } = self.tcx.unsafety_check_result(def_id);
-                    self.register_violations(&violations, &unsafe_blocks);
-                    if !interior.movable {
-                        self.require_unsafe("construction of immovable generator")
-                    }
-                }
             }
         }
         self.super_rvalue(rvalue, location);

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

@@ -179,7 +179,7 @@ fn generator(a: &Allocator, run_count: usize) {
          );
     };
     for _ in 0..run_count {
-        gen.resume();
+        unsafe { gen.resume(); }
     }
 }
 

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

@@ -42,9 +42,9 @@ fn t1() {
     };
 
     let n = A.load(Ordering::SeqCst);
-    a.resume();
+    unsafe { a.resume() };
     assert_eq!(A.load(Ordering::SeqCst), n + 1);
-    a.resume();
+    unsafe { a.resume() };
     assert_eq!(A.load(Ordering::SeqCst), n + 1);
 }
 
@@ -58,8 +58,8 @@ fn t2() {
     };
 
     let n = A.load(Ordering::SeqCst);
-    a.resume();
+    unsafe { a.resume() };
     assert_eq!(A.load(Ordering::SeqCst), n);
-    a.resume();
+    unsafe { a.resume() };
     assert_eq!(A.load(Ordering::SeqCst), n + 1);
 }

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

@@ -16,7 +16,7 @@ fn finish<T>(mut amt: usize, mut t: T) -> T::Return
     where T: Generator<Yield = ()>
 {
     loop {
-        match t.resume() {
+        match unsafe { t.resume() } {
             GeneratorState::Yielded(()) => amt = amt.checked_sub(1).unwrap(),
             GeneratorState::Complete(ret) => {
                 assert_eq!(amt, 0);

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

@@ -37,7 +37,7 @@ fn t1() {
     };
 
     let n = A.load(Ordering::SeqCst);
-    drop(foo.resume());
+    drop(unsafe { foo.resume() });
     assert_eq!(A.load(Ordering::SeqCst), n);
     drop(foo);
     assert_eq!(A.load(Ordering::SeqCst), n + 1);
@@ -50,7 +50,7 @@ fn t2() {
     };
 
     let n = A.load(Ordering::SeqCst);
-    drop(foo.resume());
+    drop(unsafe { foo.resume() });
     assert_eq!(A.load(Ordering::SeqCst), n + 1);
     drop(foo);
     assert_eq!(A.load(Ordering::SeqCst), n + 1);

src/test/run-pass/generator/issue-44197.rs

@@ -35,6 +35,8 @@ fn bar2(baz: String) -> impl Generator<Yield = String, Return = ()> {
 }
 
 fn main() {
-    assert_eq!(bar(String::new()).resume(), GeneratorState::Yielded(String::new()));
-    assert_eq!(bar2(String::new()).resume(), GeneratorState::Complete(()));
+    unsafe {
+        assert_eq!(bar(String::new()).resume(), GeneratorState::Yielded(String::new()));
+        assert_eq!(bar2(String::new()).resume(), GeneratorState::Complete(()));
+    }
 }

src/test/run-pass/generator/iterator-count.rs

@@ -14,11 +14,13 @@ use std::ops::{GeneratorState, Generator};
 
 struct W<T>(T);
 
+// This impl isn't safe in general, but the generator used in this test is movable
+// so it won't cause problems.
 impl<T: Generator<Return = ()>> Iterator for W<T> {
     type Item = T::Yield;
 
     fn next(&mut self) -> Option<Self::Item> {
-        match self.0.resume() {
+        match unsafe { self.0.resume() } {
             GeneratorState::Complete(..) => None,
             GeneratorState::Yielded(v) => Some(v),
         }

src/test/run-pass/generator/live-upvar-across-yield.rs

@@ -17,5 +17,5 @@ fn main() {
     let mut a = || {
         b(yield);
     };
-    a.resume();
+    unsafe { a.resume() };
 }

src/test/run-pass/generator/nested_generators.rs

@@ -20,7 +20,7 @@ fn main() {
             yield 2;
         };
 
-        match sub_generator.resume() {
+        match unsafe { sub_generator.resume() } {
             GeneratorState::Yielded(x) => {
                 yield x;
             }

src/test/run-pass/generator/panic-drops.rs

@@ -42,7 +42,7 @@ fn main() {
 
     assert_eq!(A.load(Ordering::SeqCst), 0);
     let res = panic::catch_unwind(panic::AssertUnwindSafe(|| {
-        foo.resume()
+        unsafe { foo.resume() }
     }));
     assert!(res.is_err());
     assert_eq!(A.load(Ordering::SeqCst), 1);
@@ -57,7 +57,7 @@ fn main() {
 
     assert_eq!(A.load(Ordering::SeqCst), 1);
     let res = panic::catch_unwind(panic::AssertUnwindSafe(|| {
-        foo.resume()
+        unsafe { foo.resume() }
     }));
     assert!(res.is_err());
     assert_eq!(A.load(Ordering::SeqCst), 1);

src/test/run-pass/generator/panic-safe.rs

@@ -24,13 +24,13 @@ fn main() {
     };
 
     let res = panic::catch_unwind(panic::AssertUnwindSafe(|| {
-        foo.resume()
+        unsafe { foo.resume() }
     }));
     assert!(res.is_err());
 
     for _ in 0..10 {
         let res = panic::catch_unwind(panic::AssertUnwindSafe(|| {
-            foo.resume()
+            unsafe { foo.resume() }
         }));
         assert!(res.is_err());
     }

src/test/run-pass/generator/resume-after-return.rs

@@ -23,12 +23,12 @@ fn main() {
         yield;
     };
 
-    match foo.resume() {
+    match unsafe { foo.resume() } {
         GeneratorState::Complete(()) => {}
         s => panic!("bad state: {:?}", s),
     }
 
-    match panic::catch_unwind(move || foo.resume()) {
+    match panic::catch_unwind(move || unsafe { foo.resume() }) {
         Ok(_) => panic!("generator successfully resumed"),
         Err(_) => {}
     }