Skip to content

Commit

Permalink
Make resuming generators unsafe instead of the creation of immovable …
Browse files Browse the repository at this point in the history
…generators. Fixes rust-lang#47787
  • Loading branch information
Zoxc committed Mar 20, 2018
1 parent a04b88d commit 57896ab
Show file tree
Hide file tree
Showing 30 changed files with 96 additions and 124 deletions.
20 changes: 10 additions & 10 deletions src/doc/unstable-book/src/language-features/generators.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
}
Expand Down Expand Up @@ -69,9 +69,9 @@ fn main() {
};

println!("1");
generator.resume();
unsafe { generator.resume() };
println!("3");
generator.resume();
unsafe { generator.resume() };
println!("5");
}
```
Expand All @@ -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>;
}
```

Expand Down Expand Up @@ -175,8 +175,8 @@ fn main() {
return ret
};

generator.resume();
generator.resume();
unsafe { generator.resume() };
unsafe { generator.resume() };
}
```

Expand All @@ -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) => {
Expand All @@ -223,8 +223,8 @@ fn main() {
__Generator::Start(ret)
};

generator.resume();
generator.resume();
unsafe { generator.resume() };
unsafe { generator.resume() };
}
```

Expand Down
2 changes: 1 addition & 1 deletion src/liballoc/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
12 changes: 8 additions & 4 deletions src/libcore/ops/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
/// }
Expand Down Expand Up @@ -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
Expand All @@ -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")]
Expand All @@ -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()
}
}
10 changes: 5 additions & 5 deletions src/librustc_mir/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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:
Expand All @@ -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>.
Expand Down
12 changes: 2 additions & 10 deletions src/librustc_mir/transform/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,21 +125,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);
Expand Down
2 changes: 1 addition & 1 deletion src/test/run-pass/dynamic-drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ fn generator(a: &Allocator, run_count: usize) {
);
};
for _ in 0..run_count {
gen.resume();
unsafe { gen.resume(); }
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/test/run-pass/generator/conditional-drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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);
}
2 changes: 1 addition & 1 deletion src/test/run-pass/generator/control-flow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/test/run-pass/generator/drop-env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions src/test/run-pass/generator/issue-44197.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(()));
}
}
4 changes: 3 additions & 1 deletion src/test/run-pass/generator/iterator-count.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/run-pass/generator/live-upvar-across-yield.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ fn main() {
let mut a = || {
b(yield);
};
a.resume();
unsafe { a.resume() };
}
2 changes: 1 addition & 1 deletion src/test/run-pass/generator/nested_generators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ fn main() {
yield 2;
};

match sub_generator.resume() {
match unsafe { sub_generator.resume() } {
GeneratorState::Yielded(x) => {
yield x;
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/run-pass/generator/panic-drops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/test/run-pass/generator/panic-safe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/run-pass/generator/resume-after-return.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(_) => {}
}
Expand Down
Loading

0 comments on commit 57896ab

Please sign in to comment.