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

Make resuming generators unsafe instead of the creation of immovable generators

cc @withoutboats

Fixes #47787

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(_) => {}
     }

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

@@ -24,7 +24,7 @@ fn simple() {
         }
     };
 
-    match foo.resume() {
+    match unsafe { foo.resume() } {
         GeneratorState::Complete(()) => {}
         s => panic!("bad state: {:?}", s),
     }
@@ -40,7 +40,7 @@ fn return_capture() {
         a
     };
 
-    match foo.resume() {
+    match unsafe { foo.resume() } {
         GeneratorState::Complete(ref s) if *s == "foo" => {}
         s => panic!("bad state: {:?}", s),
     }
@@ -52,11 +52,11 @@ fn simple_yield() {
         yield;
     };
 
-    match foo.resume() {
+    match unsafe { foo.resume() } {
         GeneratorState::Yielded(()) => {}
         s => panic!("bad state: {:?}", s),
     }
-    match foo.resume() {
+    match unsafe { foo.resume() } {
         GeneratorState::Complete(()) => {}
         s => panic!("bad state: {:?}", s),
     }
@@ -69,11 +69,11 @@ fn yield_capture() {
         yield b;
     };
 
-    match foo.resume() {
+    match unsafe { foo.resume() } {
         GeneratorState::Yielded(ref s) if *s == "foo" => {}
         s => panic!("bad state: {:?}", s),
     }
-    match foo.resume() {
+    match unsafe { foo.resume() } {
         GeneratorState::Complete(()) => {}
         s => panic!("bad state: {:?}", s),
     }
@@ -86,11 +86,11 @@ fn simple_yield_value() {
         return String::from("foo")
     };
 
-    match foo.resume() {
+    match unsafe { foo.resume() } {
         GeneratorState::Yielded(ref s) if *s == "bar" => {}
         s => panic!("bad state: {:?}", s),
     }
-    match foo.resume() {
+    match unsafe { foo.resume() } {
         GeneratorState::Complete(ref s) if *s == "foo" => {}
         s => panic!("bad state: {:?}", s),
     }
@@ -104,11 +104,11 @@ fn return_after_yield() {
         return a
     };
 
-    match foo.resume() {
+    match unsafe { foo.resume() } {
         GeneratorState::Yielded(()) => {}
         s => panic!("bad state: {:?}", s),
     }
-    match foo.resume() {
+    match unsafe { foo.resume() } {
         GeneratorState::Complete(ref s) if *s == "foo" => {}
         s => panic!("bad state: {:?}", s),
     }
@@ -156,11 +156,11 @@ fn send_and_sync() {
 fn send_over_threads() {
     let mut foo = || { yield };
     thread::spawn(move || {
-        match foo.resume() {
+        match unsafe { foo.resume() } {
             GeneratorState::Yielded(()) => {}
             s => panic!("bad state: {:?}", s),
         }
-        match foo.resume() {
+        match unsafe { foo.resume() } {
             GeneratorState::Complete(()) => {}
             s => panic!("bad state: {:?}", s),
         }
@@ -169,11 +169,11 @@ fn send_over_threads() {
     let a = String::from("a");
     let mut foo = || { yield a };
     thread::spawn(move || {
-        match foo.resume() {
+        match unsafe { foo.resume() } {
             GeneratorState::Yielded(ref s) if *s == "a" => {}
             s => panic!("bad state: {:?}", s),
         }
-        match foo.resume() {
+        match unsafe { foo.resume() } {
             GeneratorState::Complete(()) => {}
             s => panic!("bad state: {:?}", s),
         }

src/test/run-pass/generator/static-generators.rs

@@ -13,14 +13,14 @@
 use std::ops::{Generator, GeneratorState};
 
 fn main() {
-    let mut generator = unsafe {
-        static || {
-            let a = true;
-            let b = &a;
-            yield;
-            assert_eq!(b as *const _, &a as *const _);
-        }
+    let mut generator = static || {
+        let a = true;
+        let b = &a;
+        yield;
+        assert_eq!(b as *const _, &a as *const _);
     };
-    assert_eq!(generator.resume(), GeneratorState::Yielded(()));
-    assert_eq!(generator.resume(), GeneratorState::Complete(()));
+    unsafe {
+        assert_eq!(generator.resume(), GeneratorState::Yielded(()));
+        assert_eq!(generator.resume(), GeneratorState::Complete(()));
+    }
 }

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

@@ -17,5 +17,5 @@ extern crate xcrate_reachable as foo;
 use std::ops::Generator;
 
 fn main() {
-    foo::foo().resume();
+    unsafe { foo::foo().resume(); }
 }

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

@@ -19,18 +19,18 @@ use std::ops::{GeneratorState, Generator};
 fn main() {
     let mut foo = xcrate::foo();
 
-    match foo.resume() {
+    match unsafe { foo.resume() } {
         GeneratorState::Complete(()) => {}
         s => panic!("bad state: {:?}", s),
     }
 
     let mut foo = xcrate::bar(3);
 
-    match foo.resume() {
+    match unsafe { foo.resume() } {
         GeneratorState::Yielded(3) => {}
         s => panic!("bad state: {:?}", s),
     }
-    match foo.resume() {
+    match unsafe { foo.resume() } {
         GeneratorState::Complete(()) => {}
         s => panic!("bad state: {:?}", s),
     }

src/test/ui/generator/borrowing.rs

@@ -15,7 +15,7 @@ use std::ops::Generator;
 fn main() {
     let _b = {
         let a = 3;
-        (|| yield &a).resume()
+        unsafe { (|| yield &a).resume() }
         //~^ ERROR: `a` does not live long enough
     };
 

src/test/ui/generator/borrowing.stderr

@@ -1,10 +1,10 @@
 error[E0597]: `a` does not live long enough
-  --> $DIR/borrowing.rs:18:20
+  --> $DIR/borrowing.rs:18:29
    |
-LL |         (|| yield &a).resume()
-   |          --        ^ borrowed value does not live long enough
-   |          |
-   |          capture occurs here
+LL |         unsafe { (|| yield &a).resume() }
+   |                   --        ^ borrowed value does not live long enough
+   |                   |
+   |                   capture occurs here
 LL |         //~^ ERROR: `a` does not live long enough
 LL |     };
    |     - borrowed value only lives until here

src/test/ui/generator/dropck.rs

@@ -23,6 +23,6 @@ fn main() {
         let _d = ref_.take(); //~ ERROR `ref_` does not live long enough
         yield;
     };
-    gen.resume();
+    unsafe { gen.resume(); }
     // drops the RefCell and then the Ref, leading to use-after-free
 }

src/test/ui/generator/sized-yield.rs

@@ -17,5 +17,5 @@ fn main() {
    let mut gen = move || { //~ ERROR the trait bound `str: std::marker::Sized` is not satisfied
        yield s[..];
    };
-   gen.resume(); //~ ERROR the trait bound `str: std::marker::Sized` is not satisfied
+   unsafe { gen.resume(); } //~ ERROR the trait bound `str: std::marker::Sized` is not satisfied
 }

src/test/ui/generator/sized-yield.stderr

@@ -11,10 +11,10 @@ LL | |    };
    = note: the yield type of a generator must have a statically known size
 
 error[E0277]: the trait bound `str: std::marker::Sized` is not satisfied
-  --> $DIR/sized-yield.rs:20:8
+  --> $DIR/sized-yield.rs:20:17
    |
-LL |    gen.resume(); //~ ERROR the trait bound `str: std::marker::Sized` is not satisfied
-   |        ^^^^^^ `str` does not have a constant size known at compile-time
+LL |    unsafe { gen.resume(); } //~ ERROR the trait bound `str: std::marker::Sized` is not satisfied
+   |                 ^^^^^^ `str` does not have a constant size known at compile-time
    |
    = help: the trait `std::marker::Sized` is not implemented for `str`
 

src/test/ui/generator/unsafe-immovable.rs

@@ -1,17 +0,0 @@
-// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
-// file at the top-level directory of this distribution and at
-// http://rust-lang.org/COPYRIGHT.
-//
-// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
-// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
-// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
-// option. This file may not be copied, modified, or distributed
-// except according to those terms.
-
-#![feature(generators)]
-
-fn main() {
-    static || { //~ ERROR: construction of immovable generator requires unsafe
-        yield;
-    };
-}

src/test/ui/generator/unsafe-immovable.stderr

@@ -1,11 +0,0 @@
-error[E0133]: construction of immovable generator requires unsafe function or block
-  --> $DIR/unsafe-immovable.rs:14:5
-   |
-LL | /     static || { //~ ERROR: construction of immovable generator requires unsafe
-LL | |         yield;
-LL | |     };
-   | |_____^ construction of immovable generator
-
-error: aborting due to previous error
-
-For more information about this error, try `rustc --explain E0133`.

src/test/ui/generator/yield-while-iterating.rs

@@ -43,7 +43,7 @@ fn yield_during_iter_borrowed_slice_2() {
     println!("{:?}", x);
 }
 
-fn yield_during_iter_borrowed_slice_3() {
+unsafe fn yield_during_iter_borrowed_slice_3() {
     // OK to take a mutable ref to `x` and yield
     // up pointers from it:
     let mut x = vec![22_i32];
@@ -55,7 +55,7 @@ fn yield_during_iter_borrowed_slice_3() {
     b.resume();
 }
 
-fn yield_during_iter_borrowed_slice_4() {
+unsafe fn yield_during_iter_borrowed_slice_4() {
     // ...but not OK to do that while reading
     // from `x` too
     let mut x = vec![22_i32];
@@ -68,7 +68,7 @@ fn yield_during_iter_borrowed_slice_4() {
     b.resume();
 }
 
-fn yield_during_range_iter() {
+unsafe fn yield_during_range_iter() {
     // Should be OK.
     let mut b = || {
         let v = vec![1,2,3];

src/test/ui/generator/yield-while-local-borrowed.rs

@@ -15,7 +15,7 @@
 use std::ops::{GeneratorState, Generator};
 use std::cell::Cell;
 
-fn borrow_local_inline() {
+unsafe fn borrow_local_inline() {
     // Not OK to yield with a borrow of a temporary.
     //
     // (This error occurs because the region shows up in the type of
@@ -30,7 +30,7 @@ fn borrow_local_inline() {
     b.resume();
 }
 
-fn borrow_local_inline_done() {
+unsafe fn borrow_local_inline_done() {
     // No error here -- `a` is not in scope at the point of `yield`.
     let mut b = move || {
         {
@@ -41,7 +41,7 @@ fn borrow_local_inline_done() {
     b.resume();
 }
 
-fn borrow_local() {
+unsafe fn borrow_local() {
     // Not OK to yield with a borrow of a temporary.
     //
     // (This error occurs because the region shows up in the type of

src/test/ui/generator/yield-while-ref-reborrowed.rs

@@ -13,7 +13,7 @@
 use std::ops::{GeneratorState, Generator};
 use std::cell::Cell;
 
-fn reborrow_shared_ref(x: &i32) {
+unsafe fn reborrow_shared_ref(x: &i32) {
     // This is OK -- we have a borrow live over the yield, but it's of
     // data that outlives the generator.
     let mut b = move || {
@@ -24,7 +24,7 @@ fn reborrow_shared_ref(x: &i32) {
     b.resume();
 }
 
-fn reborrow_mutable_ref(x: &mut i32) {
+unsafe fn reborrow_mutable_ref(x: &mut i32) {
     // This is OK -- we have a borrow live over the yield, but it's of
     // data that outlives the generator.
     let mut b = move || {
@@ -35,7 +35,7 @@ fn reborrow_mutable_ref(x: &mut i32) {
     b.resume();
 }
 
-fn reborrow_mutable_ref_2(x: &mut i32) {
+unsafe fn reborrow_mutable_ref_2(x: &mut i32) {
     // ...but not OK to go on using `x`.
     let mut b = || {
         let a = &mut *x;