From 044fe0f558aa62926e6de9a76b95e4a74c0b1f99 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sat, 25 Jan 2020 20:03:10 +0100 Subject: [PATCH 01/25] Add a resume type parameter to `Generator` --- .../src/language-features/generators.md | 26 ++++++-------- src/liballoc/boxed.rs | 24 +++++++++++++ src/libcore/ops/generator.rs | 35 ++++++++++++++++--- src/librustc/traits/util.rs | 6 ++-- src/librustc/ty/layout.rs | 4 +-- src/librustc_error_codes/error_codes/E0626.md | 10 +++--- .../dataflow/impls/storage_liveness.rs | 20 ++++++----- src/librustc_mir/transform/generator.rs | 3 +- src/librustc_mir_build/build/mod.rs | 9 +++-- src/libstd/future.rs | 5 ++- src/test/debuginfo/generator-locals.rs | 6 ++-- src/test/debuginfo/generator-objects.rs | 6 ++-- src/test/debuginfo/issue-57822.rs | 2 +- .../run-fail/generator-resume-after-panic.rs | 4 +-- ...65419-generator-resume-after-completion.rs | 6 ++-- src/test/ui/drop/dynamic-drop.rs | 2 +- .../generator/auxiliary/xcrate-reachable.rs | 2 +- src/test/ui/generator/auxiliary/xcrate.rs | 4 +-- src/test/ui/generator/borrowing.rs | 2 +- src/test/ui/generator/borrowing.stderr | 2 +- src/test/ui/generator/conditional-drop.rs | 8 ++--- src/test/ui/generator/control-flow.rs | 4 +-- src/test/ui/generator/drop-and-replace.rs | 2 +- src/test/ui/generator/drop-env.rs | 4 +-- src/test/ui/generator/dropck.rs | 2 +- .../generator-region-requirements.rs | 2 +- src/test/ui/generator/issue-44197.rs | 13 ++++--- .../issue-61442-stmt-expr-with-drop.rs | 4 +-- src/test/ui/generator/iterator-count.rs | 6 ++-- .../ui/generator/live-upvar-across-yield.rs | 2 +- src/test/ui/generator/nested_generators.rs | 2 +- src/test/ui/generator/panic-drops.rs | 4 +-- src/test/ui/generator/panic-safe.rs | 4 +-- src/test/ui/generator/resume-after-return.rs | 4 +-- src/test/ui/generator/sized-yield.rs | 2 +- src/test/ui/generator/sized-yield.stderr | 2 +- src/test/ui/generator/smoke.rs | 28 +++++++-------- src/test/ui/generator/static-generators.rs | 4 +-- src/test/ui/generator/xcrate-reachable.rs | 2 +- src/test/ui/generator/xcrate.rs | 6 ++-- .../ui/generator/yield-while-iterating.rs | 6 ++-- .../ui/generator/yield-while-iterating.stderr | 2 +- .../generator/yield-while-local-borrowed.rs | 6 ++-- .../generator/yield-while-ref-reborrowed.rs | 6 ++-- .../yield-while-ref-reborrowed.stderr | 2 +- src/test/ui/nll/issue-55850.rs | 2 +- 46 files changed, 185 insertions(+), 122 deletions(-) diff --git a/src/doc/unstable-book/src/language-features/generators.md b/src/doc/unstable-book/src/language-features/generators.md index 97cf58e57e605..8bc62418b3969 100644 --- a/src/doc/unstable-book/src/language-features/generators.md +++ b/src/doc/unstable-book/src/language-features/generators.md @@ -37,11 +37,11 @@ fn main() { return "foo" }; - match Pin::new(&mut generator).resume() { + match Pin::new(&mut generator).resume(()) { GeneratorState::Yielded(1) => {} _ => panic!("unexpected value from resume"), } - match Pin::new(&mut generator).resume() { + match Pin::new(&mut generator).resume(()) { GeneratorState::Complete("foo") => {} _ => panic!("unexpected value from resume"), } @@ -71,9 +71,9 @@ fn main() { }; println!("1"); - Pin::new(&mut generator).resume(); + Pin::new(&mut generator).resume(()); println!("3"); - Pin::new(&mut generator).resume(); + Pin::new(&mut generator).resume(()); println!("5"); } ``` @@ -92,10 +92,10 @@ The `Generator` trait in `std::ops` currently looks like: # use std::ops::GeneratorState; # use std::pin::Pin; -pub trait Generator { +pub trait Generator { type Yield; type Return; - fn resume(self: Pin<&mut Self>) -> GeneratorState; + fn resume(self: Pin<&mut Self>, resume: R) -> GeneratorState; } ``` @@ -152,10 +152,6 @@ closure-like semantics. Namely: * Whenever a generator is dropped it will drop all captured environment variables. -Note that unlike closures, generators at this time cannot take any arguments. -That is, generators must always look like `|| { ... }`. This restriction may be -lifted at a future date, the design is ongoing! - ### Generators as state machines In the compiler, generators are currently compiled as state machines. Each @@ -179,8 +175,8 @@ fn main() { return ret }; - Pin::new(&mut generator).resume(); - Pin::new(&mut generator).resume(); + Pin::new(&mut generator).resume(()); + Pin::new(&mut generator).resume(()); } ``` @@ -205,7 +201,7 @@ fn main() { type Yield = i32; type Return = &'static str; - fn resume(mut self: Pin<&mut Self>) -> GeneratorState { + fn resume(mut self: Pin<&mut Self>, resume: ()) -> GeneratorState { use std::mem; match mem::replace(&mut *self, __Generator::Done) { __Generator::Start(s) => { @@ -228,8 +224,8 @@ fn main() { __Generator::Start(ret) }; - Pin::new(&mut generator).resume(); - Pin::new(&mut generator).resume(); + Pin::new(&mut generator).resume(()); + Pin::new(&mut generator).resume(()); } ``` diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 8735c2c8f3625..04be86862ae9c 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -1103,6 +1103,7 @@ impl AsMut for Box { #[stable(feature = "pin", since = "1.33.0")] impl Unpin for Box {} +#[cfg(bootstrap)] #[unstable(feature = "generator_trait", issue = "43122")] impl Generator for Box { type Yield = G::Yield; @@ -1113,6 +1114,7 @@ impl Generator for Box { } } +#[cfg(bootstrap)] #[unstable(feature = "generator_trait", issue = "43122")] impl Generator for Pin> { type Yield = G::Yield; @@ -1123,6 +1125,28 @@ impl Generator for Pin> { } } +#[cfg(not(bootstrap))] +#[unstable(feature = "generator_trait", issue = "43122")] +impl + Unpin, R> Generator for Box { + type Yield = G::Yield; + type Return = G::Return; + + fn resume(mut self: Pin<&mut Self>, arg: R) -> GeneratorState { + G::resume(Pin::new(&mut *self), arg) + } +} + +#[cfg(not(bootstrap))] +#[unstable(feature = "generator_trait", issue = "43122")] +impl, R> Generator for Pin> { + type Yield = G::Yield; + type Return = G::Return; + + fn resume(mut self: Pin<&mut Self>, arg: R) -> GeneratorState { + G::resume((*self).as_mut(), arg) + } +} + #[stable(feature = "futures_api", since = "1.36.0")] impl Future for Box { type Output = F::Output; diff --git a/src/libcore/ops/generator.rs b/src/libcore/ops/generator.rs index 5401fff860e9b..4e43561996c37 100644 --- a/src/libcore/ops/generator.rs +++ b/src/libcore/ops/generator.rs @@ -50,11 +50,11 @@ pub enum GeneratorState { /// return "foo" /// }; /// -/// match Pin::new(&mut generator).resume() { +/// match Pin::new(&mut generator).resume(()) { /// GeneratorState::Yielded(1) => {} /// _ => panic!("unexpected return from resume"), /// } -/// match Pin::new(&mut generator).resume() { +/// match Pin::new(&mut generator).resume(()) { /// GeneratorState::Complete("foo") => {} /// _ => panic!("unexpected return from resume"), /// } @@ -67,7 +67,7 @@ pub enum GeneratorState { #[lang = "generator"] #[unstable(feature = "generator_trait", issue = "43122")] #[fundamental] -pub trait Generator { +pub trait Generator<#[cfg(not(bootstrap))] R = ()> { /// The type of value this generator yields. /// /// This associated type corresponds to the `yield` expression and the @@ -110,9 +110,13 @@ 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(self: Pin<&mut Self>) -> GeneratorState; + fn resume( + self: Pin<&mut Self>, + #[cfg(not(bootstrap))] arg: R, + ) -> GeneratorState; } +#[cfg(bootstrap)] #[unstable(feature = "generator_trait", issue = "43122")] impl Generator for Pin<&mut G> { type Yield = G::Yield; @@ -123,6 +127,7 @@ impl Generator for Pin<&mut G> { } } +#[cfg(bootstrap)] #[unstable(feature = "generator_trait", issue = "43122")] impl Generator for &mut G { type Yield = G::Yield; @@ -132,3 +137,25 @@ impl Generator for &mut G { G::resume(Pin::new(&mut *self)) } } + +#[cfg(not(bootstrap))] +#[unstable(feature = "generator_trait", issue = "43122")] +impl, R> Generator for Pin<&mut G> { + type Yield = G::Yield; + type Return = G::Return; + + fn resume(mut self: Pin<&mut Self>, arg: R) -> GeneratorState { + G::resume((*self).as_mut(), arg) + } +} + +#[cfg(not(bootstrap))] +#[unstable(feature = "generator_trait", issue = "43122")] +impl + Unpin, R> Generator for &mut G { + type Yield = G::Yield; + type Return = G::Return; + + fn resume(mut self: Pin<&mut Self>, arg: R) -> GeneratorState { + G::resume(Pin::new(&mut *self), arg) + } +} diff --git a/src/librustc/traits/util.rs b/src/librustc/traits/util.rs index ae1a5e3efa2a7..947d66e38b434 100644 --- a/src/librustc/traits/util.rs +++ b/src/librustc/traits/util.rs @@ -643,8 +643,10 @@ pub fn generator_trait_ref_and_outputs( self_ty: Ty<'tcx>, sig: ty::PolyGenSig<'tcx>, ) -> ty::Binder<(ty::TraitRef<'tcx>, Ty<'tcx>, Ty<'tcx>)> { - let trait_ref = - ty::TraitRef { def_id: fn_trait_def_id, substs: tcx.mk_substs_trait(self_ty, &[]) }; + let trait_ref = ty::TraitRef { + def_id: fn_trait_def_id, + substs: tcx.mk_substs_trait(self_ty, &[tcx.mk_unit().into()]), + }; ty::Binder::bind((trait_ref, sig.skip_binder().yield_ty, sig.skip_binder().return_ty)) } diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index bda42db40b0ae..966b60c6cfbc1 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -2350,8 +2350,8 @@ impl<'tcx> ty::Instance<'tcx> { ]); let ret_ty = tcx.mk_adt(state_adt_ref, state_substs); - tcx.mk_fn_sig(iter::once(env_ty), - ret_ty, + tcx.mk_fn_sig([env_ty, tcx.mk_unit()].iter(), + &ret_ty, false, hir::Unsafety::Normal, rustc_target::spec::abi::Abi::Rust diff --git a/src/librustc_error_codes/error_codes/E0626.md b/src/librustc_error_codes/error_codes/E0626.md index db50bd7ac4f6f..cc6e03d1ca70f 100644 --- a/src/librustc_error_codes/error_codes/E0626.md +++ b/src/librustc_error_codes/error_codes/E0626.md @@ -12,7 +12,7 @@ let mut b = || { yield (); // ...is still in scope here, when the yield occurs. println!("{}", a); }; -Pin::new(&mut b).resume(); +Pin::new(&mut b).resume(()); ``` At present, it is not permitted to have a yield that occurs while a @@ -31,7 +31,7 @@ let mut b = || { yield (); println!("{}", a); }; -Pin::new(&mut b).resume(); +Pin::new(&mut b).resume(()); ``` This is a very simple case, of course. In more complex cases, we may @@ -50,7 +50,7 @@ let mut b = || { yield x; // ...when this yield occurs. } }; -Pin::new(&mut b).resume(); +Pin::new(&mut b).resume(()); ``` Such cases can sometimes be resolved by iterating "by value" (or using @@ -66,7 +66,7 @@ let mut b = || { yield x; // <-- Now yield is OK. } }; -Pin::new(&mut b).resume(); +Pin::new(&mut b).resume(()); ``` If taking ownership is not an option, using indices can work too: @@ -83,7 +83,7 @@ let mut b = || { yield x; // <-- Now yield is OK. } }; -Pin::new(&mut b).resume(); +Pin::new(&mut b).resume(()); // (*) -- Unfortunately, these temporaries are currently required. // See . diff --git a/src/librustc_mir/dataflow/impls/storage_liveness.rs b/src/librustc_mir/dataflow/impls/storage_liveness.rs index 6a48d1e98032c..040c13e8210ea 100644 --- a/src/librustc_mir/dataflow/impls/storage_liveness.rs +++ b/src/librustc_mir/dataflow/impls/storage_liveness.rs @@ -31,10 +31,12 @@ impl<'a, 'tcx> BitDenotation<'tcx> for MaybeStorageLive<'a, 'tcx> { self.body.local_decls.len() } - fn start_block_effect(&self, _on_entry: &mut BitSet) { - // Nothing is live on function entry (generators only have a self - // argument, and we don't care about that) - assert_eq!(1, self.body.arg_count); + fn start_block_effect(&self, on_entry: &mut BitSet) { + // The resume argument is live on function entry (we don't care about + // the `self` argument) + for arg in self.body.args_iter().skip(1) { + on_entry.insert(arg); + } } fn statement_effect(&self, trans: &mut GenKillSet, loc: Location) { @@ -100,10 +102,12 @@ impl<'mir, 'tcx> BitDenotation<'tcx> for RequiresStorage<'mir, 'tcx> { self.body.local_decls.len() } - fn start_block_effect(&self, _sets: &mut BitSet) { - // Nothing is live on function entry (generators only have a self - // argument, and we don't care about that) - assert_eq!(1, self.body.arg_count); + fn start_block_effect(&self, on_entry: &mut BitSet) { + // The resume argument is live on function entry (we don't care about + // the `self` argument) + for arg in self.body.args_iter().skip(1) { + on_entry.insert(arg); + } } fn before_statement_effect(&self, sets: &mut GenKillSet, loc: Location) { diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 1c86d6f3f65f2..a8defd03f7177 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -885,6 +885,7 @@ fn create_generator_drop_shim<'tcx>( drop_clean: BasicBlock, ) -> BodyAndCache<'tcx> { let mut body = body.clone(); + body.arg_count = 1; // make sure the resume argument is not included here let source_info = source_info(&body); @@ -1164,7 +1165,7 @@ impl<'tcx> MirPass<'tcx> for StateTransform { // Update our MIR struct to reflect the changed we've made body.yield_ty = None; - body.arg_count = 1; + body.arg_count = 2; // self, resume arg body.spread_arg = None; body.generator_layout = Some(layout); diff --git a/src/librustc_mir_build/build/mod.rs b/src/librustc_mir_build/build/mod.rs index 1f536b63a394a..ab539e6179e3d 100644 --- a/src/librustc_mir_build/build/mod.rs +++ b/src/librustc_mir_build/build/mod.rs @@ -75,13 +75,16 @@ fn mir_build(tcx: TyCtxt<'_>, def_id: DefId) -> BodyAndCache<'_> { // HACK(eddyb) Avoid having RustCall on closures, // as it adds unnecessary (and wrong) auto-tupling. abi = Abi::Rust; - Some(ArgInfo(liberated_closure_env_ty(tcx, id, body_id), None, None, None)) + vec![ArgInfo(liberated_closure_env_ty(tcx, id, body_id), None, None, None)] } ty::Generator(..) => { let gen_ty = tcx.body_tables(body_id).node_type(id); - Some(ArgInfo(gen_ty, None, None, None)) + vec![ + ArgInfo(gen_ty, None, None, None), + ArgInfo(tcx.mk_unit(), None, None, None), + ] } - _ => None, + _ => vec![], }; let safety = match fn_sig.unsafety { diff --git a/src/libstd/future.rs b/src/libstd/future.rs index 9c7422c2b20a6..f74c84e6dfd48 100644 --- a/src/libstd/future.rs +++ b/src/libstd/future.rs @@ -40,7 +40,10 @@ impl> Future for GenFuture { // Safe because we're !Unpin + !Drop mapping to a ?Unpin value let gen = unsafe { Pin::map_unchecked_mut(self, |s| &mut s.0) }; let _guard = unsafe { set_task_context(cx) }; - match gen.resume() { + match gen.resume( + #[cfg(not(bootstrap))] + (), + ) { GeneratorState::Yielded(()) => Poll::Pending, GeneratorState::Complete(x) => Poll::Ready(x), } diff --git a/src/test/debuginfo/generator-locals.rs b/src/test/debuginfo/generator-locals.rs index 59dbfecc39ffc..fd46c1a8b4db7 100644 --- a/src/test/debuginfo/generator-locals.rs +++ b/src/test/debuginfo/generator-locals.rs @@ -78,9 +78,9 @@ fn main() { _zzz(); // #break a = c; }; - Pin::new(&mut b).resume(); - Pin::new(&mut b).resume(); - Pin::new(&mut b).resume(); + Pin::new(&mut b).resume(()); + Pin::new(&mut b).resume(()); + Pin::new(&mut b).resume(()); _zzz(); // #break } diff --git a/src/test/debuginfo/generator-objects.rs b/src/test/debuginfo/generator-objects.rs index bfa7a05cad057..f19a3c71dd8d2 100644 --- a/src/test/debuginfo/generator-objects.rs +++ b/src/test/debuginfo/generator-objects.rs @@ -57,11 +57,11 @@ fn main() { println!("{} {} {}", a, c, d); }; _zzz(); // #break - Pin::new(&mut b).resume(); + Pin::new(&mut b).resume(()); _zzz(); // #break - Pin::new(&mut b).resume(); + Pin::new(&mut b).resume(()); _zzz(); // #break - Pin::new(&mut b).resume(); + Pin::new(&mut b).resume(()); _zzz(); // #break } diff --git a/src/test/debuginfo/issue-57822.rs b/src/test/debuginfo/issue-57822.rs index f18e41db0e6ba..4de88e9dae62b 100644 --- a/src/test/debuginfo/issue-57822.rs +++ b/src/test/debuginfo/issue-57822.rs @@ -45,7 +45,7 @@ fn main() { yield; }; let mut b = move || { - Pin::new(&mut a).resume(); + Pin::new(&mut a).resume(()); yield; }; diff --git a/src/test/run-fail/generator-resume-after-panic.rs b/src/test/run-fail/generator-resume-after-panic.rs index 910b4903bf6a3..1a7c2e8062901 100644 --- a/src/test/run-fail/generator-resume-after-panic.rs +++ b/src/test/run-fail/generator-resume-after-panic.rs @@ -16,7 +16,7 @@ fn main() { yield; }; panic::catch_unwind(panic::AssertUnwindSafe(|| { - let x = Pin::new(&mut g).resume(); + let x = Pin::new(&mut g).resume(()); })); - Pin::new(&mut g).resume(); + Pin::new(&mut g).resume(()); } diff --git a/src/test/ui/async-await/issues/issue-65419/issue-65419-generator-resume-after-completion.rs b/src/test/ui/async-await/issues/issue-65419/issue-65419-generator-resume-after-completion.rs index 23e3483e01c7d..9fc5667d6847e 100644 --- a/src/test/ui/async-await/issues/issue-65419/issue-65419-generator-resume-after-completion.rs +++ b/src/test/ui/async-await/issues/issue-65419/issue-65419-generator-resume-after-completion.rs @@ -19,7 +19,7 @@ fn main() { let mut g = || { yield; }; - Pin::new(&mut g).resume(); // Yields once. - Pin::new(&mut g).resume(); // Completes here. - Pin::new(&mut g).resume(); // Panics here. + Pin::new(&mut g).resume(()); // Yields once. + Pin::new(&mut g).resume(()); // Completes here. + Pin::new(&mut g).resume(()); // Panics here. } diff --git a/src/test/ui/drop/dynamic-drop.rs b/src/test/ui/drop/dynamic-drop.rs index b4406204a5db9..659e520d4cd12 100644 --- a/src/test/ui/drop/dynamic-drop.rs +++ b/src/test/ui/drop/dynamic-drop.rs @@ -184,7 +184,7 @@ fn generator(a: &Allocator, run_count: usize) { ); }; for _ in 0..run_count { - Pin::new(&mut gen).resume(); + Pin::new(&mut gen).resume(()); } } diff --git a/src/test/ui/generator/auxiliary/xcrate-reachable.rs b/src/test/ui/generator/auxiliary/xcrate-reachable.rs index 33337f8038f60..2dd5ea675233c 100644 --- a/src/test/ui/generator/auxiliary/xcrate-reachable.rs +++ b/src/test/ui/generator/auxiliary/xcrate-reachable.rs @@ -6,7 +6,7 @@ fn msg() -> u32 { 0 } -pub fn foo() -> impl Generator { +pub fn foo() -> impl Generator<(), Yield=(), Return=u32> { || { yield; return msg(); diff --git a/src/test/ui/generator/auxiliary/xcrate.rs b/src/test/ui/generator/auxiliary/xcrate.rs index 613c495832f00..d07abd0918c78 100644 --- a/src/test/ui/generator/auxiliary/xcrate.rs +++ b/src/test/ui/generator/auxiliary/xcrate.rs @@ -3,7 +3,7 @@ use std::marker::Unpin; use std::ops::Generator; -pub fn foo() -> impl Generator { +pub fn foo() -> impl Generator<(), Yield = (), Return = ()> { || { if false { yield; @@ -11,7 +11,7 @@ pub fn foo() -> impl Generator { } } -pub fn bar(t: T) -> Box + Unpin> { +pub fn bar(t: T) -> Box + Unpin> { Box::new(|| { yield t; }) diff --git a/src/test/ui/generator/borrowing.rs b/src/test/ui/generator/borrowing.rs index 6234b73804878..d36592583cdc5 100644 --- a/src/test/ui/generator/borrowing.rs +++ b/src/test/ui/generator/borrowing.rs @@ -6,7 +6,7 @@ use std::pin::Pin; fn main() { let _b = { let a = 3; - Pin::new(&mut || yield &a).resume() + Pin::new(&mut || yield &a).resume(()) //~^ ERROR: `a` does not live long enough }; diff --git a/src/test/ui/generator/borrowing.stderr b/src/test/ui/generator/borrowing.stderr index 3d58873f826da..83987e19839ce 100644 --- a/src/test/ui/generator/borrowing.stderr +++ b/src/test/ui/generator/borrowing.stderr @@ -1,7 +1,7 @@ error[E0597]: `a` does not live long enough --> $DIR/borrowing.rs:9:33 | -LL | Pin::new(&mut || yield &a).resume() +LL | Pin::new(&mut || yield &a).resume(()) | ----------^ | | | | | borrowed value does not live long enough diff --git a/src/test/ui/generator/conditional-drop.rs b/src/test/ui/generator/conditional-drop.rs index 907f7a3c06de7..990d94e6efc1b 100644 --- a/src/test/ui/generator/conditional-drop.rs +++ b/src/test/ui/generator/conditional-drop.rs @@ -35,9 +35,9 @@ fn t1() { }; let n = A.load(Ordering::SeqCst); - Pin::new(&mut a).resume(); + Pin::new(&mut a).resume(()); assert_eq!(A.load(Ordering::SeqCst), n + 1); - Pin::new(&mut a).resume(); + Pin::new(&mut a).resume(()); assert_eq!(A.load(Ordering::SeqCst), n + 1); } @@ -51,8 +51,8 @@ fn t2() { }; let n = A.load(Ordering::SeqCst); - Pin::new(&mut a).resume(); + Pin::new(&mut a).resume(()); assert_eq!(A.load(Ordering::SeqCst), n); - Pin::new(&mut a).resume(); + Pin::new(&mut a).resume(()); assert_eq!(A.load(Ordering::SeqCst), n + 1); } diff --git a/src/test/ui/generator/control-flow.rs b/src/test/ui/generator/control-flow.rs index df70e013bd330..9d4c217b76ed7 100644 --- a/src/test/ui/generator/control-flow.rs +++ b/src/test/ui/generator/control-flow.rs @@ -7,10 +7,10 @@ use std::ops::{GeneratorState, Generator}; use std::pin::Pin; fn finish(mut amt: usize, mut t: T) -> T::Return - where T: Generator + Unpin, + where T: Generator<(), Yield = ()> + Unpin, { loop { - match Pin::new(&mut t).resume() { + match Pin::new(&mut t).resume(()) { GeneratorState::Yielded(()) => amt = amt.checked_sub(1).unwrap(), GeneratorState::Complete(ret) => { assert_eq!(amt, 0); diff --git a/src/test/ui/generator/drop-and-replace.rs b/src/test/ui/generator/drop-and-replace.rs index bb33502815bf8..a9a50a122a19c 100644 --- a/src/test/ui/generator/drop-and-replace.rs +++ b/src/test/ui/generator/drop-and-replace.rs @@ -37,7 +37,7 @@ fn main() { }; loop { - match Pin::new(&mut a).resume() { + match Pin::new(&mut a).resume(()) { GeneratorState::Complete(()) => break, _ => (), } diff --git a/src/test/ui/generator/drop-env.rs b/src/test/ui/generator/drop-env.rs index ac4e06656285e..7ba711881045d 100644 --- a/src/test/ui/generator/drop-env.rs +++ b/src/test/ui/generator/drop-env.rs @@ -30,7 +30,7 @@ fn t1() { }; let n = A.load(Ordering::SeqCst); - drop(Pin::new(&mut foo).resume()); + drop(Pin::new(&mut foo).resume(())); assert_eq!(A.load(Ordering::SeqCst), n); drop(foo); assert_eq!(A.load(Ordering::SeqCst), n + 1); @@ -43,7 +43,7 @@ fn t2() { }; let n = A.load(Ordering::SeqCst); - drop(Pin::new(&mut foo).resume()); + drop(Pin::new(&mut foo).resume(())); assert_eq!(A.load(Ordering::SeqCst), n + 1); drop(foo); assert_eq!(A.load(Ordering::SeqCst), n + 1); diff --git a/src/test/ui/generator/dropck.rs b/src/test/ui/generator/dropck.rs index 65c61fbaac4e2..da00b230d9fb7 100644 --- a/src/test/ui/generator/dropck.rs +++ b/src/test/ui/generator/dropck.rs @@ -15,6 +15,6 @@ fn main() { let _d = ref_.take(); //~ ERROR `ref_` does not live long enough yield; }; - Pin::new(&mut gen).resume(); + Pin::new(&mut gen).resume(()); // drops the RefCell and then the Ref, leading to use-after-free } diff --git a/src/test/ui/generator/generator-region-requirements.rs b/src/test/ui/generator/generator-region-requirements.rs index 41cb339f45911..5f0a6bb09b784 100644 --- a/src/test/ui/generator/generator-region-requirements.rs +++ b/src/test/ui/generator/generator-region-requirements.rs @@ -8,7 +8,7 @@ fn dangle(x: &mut i32) -> &'static mut i32 { x }; loop { - match Pin::new(&mut g).resume() { + match Pin::new(&mut g).resume(()) { GeneratorState::Complete(c) => return c, //~^ ERROR explicit lifetime required GeneratorState::Yielded(_) => (), diff --git a/src/test/ui/generator/issue-44197.rs b/src/test/ui/generator/issue-44197.rs index 6efaff50c1e62..389b9d1396941 100644 --- a/src/test/ui/generator/issue-44197.rs +++ b/src/test/ui/generator/issue-44197.rs @@ -2,14 +2,14 @@ #![feature(generators, generator_trait)] -use std::ops::{ Generator, GeneratorState }; +use std::ops::{Generator, GeneratorState}; use std::pin::Pin; fn foo(_: &str) -> String { String::new() } -fn bar(baz: String) -> impl Generator { +fn bar(baz: String) -> impl Generator<(), Yield = String, Return = ()> { move || { yield foo(&baz); } @@ -19,7 +19,7 @@ fn foo2(_: &str) -> Result { Err(()) } -fn bar2(baz: String) -> impl Generator { +fn bar2(baz: String) -> impl Generator<(), Yield = String, Return = ()> { move || { if let Ok(quux) = foo2(&baz) { yield quux; @@ -28,6 +28,9 @@ fn bar2(baz: String) -> impl Generator { } fn main() { - assert_eq!(Pin::new(&mut bar(String::new())).resume(), GeneratorState::Yielded(String::new())); - assert_eq!(Pin::new(&mut bar2(String::new())).resume(), GeneratorState::Complete(())); + assert_eq!( + Pin::new(&mut bar(String::new())).resume(()), + GeneratorState::Yielded(String::new()) + ); + assert_eq!(Pin::new(&mut bar2(String::new())).resume(()), GeneratorState::Complete(())); } diff --git a/src/test/ui/generator/issue-61442-stmt-expr-with-drop.rs b/src/test/ui/generator/issue-61442-stmt-expr-with-drop.rs index e3d19029348a5..187c374021dca 100644 --- a/src/test/ui/generator/issue-61442-stmt-expr-with-drop.rs +++ b/src/test/ui/generator/issue-61442-stmt-expr-with-drop.rs @@ -18,12 +18,12 @@ fn drop_and_yield() { String::new(); yield; }; - Box::pin(x).as_mut().resume(); + Box::pin(x).as_mut().resume(()); let y = static || { String::new(); yield; }; - Box::pin(y).as_mut().resume(); + Box::pin(y).as_mut().resume(()); } fn main() { diff --git a/src/test/ui/generator/iterator-count.rs b/src/test/ui/generator/iterator-count.rs index ac7e122dd5802..90eefe02f664e 100644 --- a/src/test/ui/generator/iterator-count.rs +++ b/src/test/ui/generator/iterator-count.rs @@ -10,18 +10,18 @@ struct W(T); // This impl isn't safe in general, but the generator used in this test is movable // so it won't cause problems. -impl + Unpin> Iterator for W { +impl + Unpin> Iterator for W { type Item = T::Yield; fn next(&mut self) -> Option { - match Pin::new(&mut self.0).resume() { + match Pin::new(&mut self.0).resume(()) { GeneratorState::Complete(..) => None, GeneratorState::Yielded(v) => Some(v), } } } -fn test() -> impl Generator + Unpin { +fn test() -> impl Generator<(), Return=(), Yield=u8> + Unpin { || { for i in 1..6 { yield i diff --git a/src/test/ui/generator/live-upvar-across-yield.rs b/src/test/ui/generator/live-upvar-across-yield.rs index a1064165b2f7a..6a2e42a5573a8 100644 --- a/src/test/ui/generator/live-upvar-across-yield.rs +++ b/src/test/ui/generator/live-upvar-across-yield.rs @@ -10,5 +10,5 @@ fn main() { let mut a = || { b(yield); }; - Pin::new(&mut a).resume(); + Pin::new(&mut a).resume(()); } diff --git a/src/test/ui/generator/nested_generators.rs b/src/test/ui/generator/nested_generators.rs index b56cce1dc44f1..45519150eec2b 100644 --- a/src/test/ui/generator/nested_generators.rs +++ b/src/test/ui/generator/nested_generators.rs @@ -11,7 +11,7 @@ fn main() { yield 2; }; - match Pin::new(&mut sub_generator).resume() { + match Pin::new(&mut sub_generator).resume(()) { GeneratorState::Yielded(x) => { yield x; } diff --git a/src/test/ui/generator/panic-drops.rs b/src/test/ui/generator/panic-drops.rs index f88687858fd11..c9a201725aea2 100644 --- a/src/test/ui/generator/panic-drops.rs +++ b/src/test/ui/generator/panic-drops.rs @@ -35,7 +35,7 @@ fn main() { assert_eq!(A.load(Ordering::SeqCst), 0); let res = panic::catch_unwind(panic::AssertUnwindSafe(|| { - Pin::new(&mut foo).resume() + Pin::new(&mut foo).resume(()) })); assert!(res.is_err()); assert_eq!(A.load(Ordering::SeqCst), 1); @@ -50,7 +50,7 @@ fn main() { assert_eq!(A.load(Ordering::SeqCst), 1); let res = panic::catch_unwind(panic::AssertUnwindSafe(|| { - Pin::new(&mut foo).resume() + Pin::new(&mut foo).resume(()) })); assert!(res.is_err()); assert_eq!(A.load(Ordering::SeqCst), 1); diff --git a/src/test/ui/generator/panic-safe.rs b/src/test/ui/generator/panic-safe.rs index 5f6778674dce1..500a3c9c2950e 100644 --- a/src/test/ui/generator/panic-safe.rs +++ b/src/test/ui/generator/panic-safe.rs @@ -17,13 +17,13 @@ fn main() { }; let res = panic::catch_unwind(panic::AssertUnwindSafe(|| { - Pin::new(&mut foo).resume() + Pin::new(&mut foo).resume(()) })); assert!(res.is_err()); for _ in 0..10 { let res = panic::catch_unwind(panic::AssertUnwindSafe(|| { - Pin::new(&mut foo).resume() + Pin::new(&mut foo).resume(()) })); assert!(res.is_err()); } diff --git a/src/test/ui/generator/resume-after-return.rs b/src/test/ui/generator/resume-after-return.rs index 71a68ff684af3..efed08bd4708f 100644 --- a/src/test/ui/generator/resume-after-return.rs +++ b/src/test/ui/generator/resume-after-return.rs @@ -16,12 +16,12 @@ fn main() { yield; }; - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Complete(()) => {} s => panic!("bad state: {:?}", s), } - match panic::catch_unwind(move || Pin::new(&mut foo).resume()) { + match panic::catch_unwind(move || Pin::new(&mut foo).resume(())) { Ok(_) => panic!("generator successfully resumed"), Err(_) => {} } diff --git a/src/test/ui/generator/sized-yield.rs b/src/test/ui/generator/sized-yield.rs index f64849b3149b8..c6dd738d6ac60 100644 --- a/src/test/ui/generator/sized-yield.rs +++ b/src/test/ui/generator/sized-yield.rs @@ -9,6 +9,6 @@ fn main() { //~^ ERROR the size for values of type yield s[..]; }; - Pin::new(&mut gen).resume(); + Pin::new(&mut gen).resume(()); //~^ ERROR the size for values of type } diff --git a/src/test/ui/generator/sized-yield.stderr b/src/test/ui/generator/sized-yield.stderr index c2caac7ebe2e7..79aeec2ec0280 100644 --- a/src/test/ui/generator/sized-yield.stderr +++ b/src/test/ui/generator/sized-yield.stderr @@ -15,7 +15,7 @@ LL | | }; error[E0277]: the size for values of type `str` cannot be known at compilation time --> $DIR/sized-yield.rs:12:23 | -LL | Pin::new(&mut gen).resume(); +LL | Pin::new(&mut gen).resume(()); | ^^^^^^ doesn't have a size known at compile-time | = help: the trait `std::marker::Sized` is not implemented for `str` diff --git a/src/test/ui/generator/smoke.rs b/src/test/ui/generator/smoke.rs index 533f2399084fa..9289710b34bf9 100644 --- a/src/test/ui/generator/smoke.rs +++ b/src/test/ui/generator/smoke.rs @@ -17,7 +17,7 @@ fn simple() { } }; - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Complete(()) => {} s => panic!("bad state: {:?}", s), } @@ -33,7 +33,7 @@ fn return_capture() { a }; - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Complete(ref s) if *s == "foo" => {} s => panic!("bad state: {:?}", s), } @@ -45,11 +45,11 @@ fn simple_yield() { yield; }; - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Yielded(()) => {} s => panic!("bad state: {:?}", s), } - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Complete(()) => {} s => panic!("bad state: {:?}", s), } @@ -62,11 +62,11 @@ fn yield_capture() { yield b; }; - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Yielded(ref s) if *s == "foo" => {} s => panic!("bad state: {:?}", s), } - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Complete(()) => {} s => panic!("bad state: {:?}", s), } @@ -79,11 +79,11 @@ fn simple_yield_value() { return String::from("foo") }; - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Yielded(ref s) if *s == "bar" => {} s => panic!("bad state: {:?}", s), } - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Complete(ref s) if *s == "foo" => {} s => panic!("bad state: {:?}", s), } @@ -97,11 +97,11 @@ fn return_after_yield() { return a }; - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Yielded(()) => {} s => panic!("bad state: {:?}", s), } - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Complete(ref s) if *s == "foo" => {} s => panic!("bad state: {:?}", s), } @@ -149,11 +149,11 @@ fn send_and_sync() { fn send_over_threads() { let mut foo = || { yield }; thread::spawn(move || { - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Yielded(()) => {} s => panic!("bad state: {:?}", s), } - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Complete(()) => {} s => panic!("bad state: {:?}", s), } @@ -162,11 +162,11 @@ fn send_over_threads() { let a = String::from("a"); let mut foo = || { yield a }; thread::spawn(move || { - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Yielded(ref s) if *s == "a" => {} s => panic!("bad state: {:?}", s), } - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Complete(()) => {} s => panic!("bad state: {:?}", s), } diff --git a/src/test/ui/generator/static-generators.rs b/src/test/ui/generator/static-generators.rs index 965d3c61c22d5..3980766c4287e 100644 --- a/src/test/ui/generator/static-generators.rs +++ b/src/test/ui/generator/static-generators.rs @@ -15,6 +15,6 @@ fn main() { // Safety: We shadow the original generator variable so have no safe API to // move it after this point. let mut generator = unsafe { Pin::new_unchecked(&mut generator) }; - assert_eq!(generator.as_mut().resume(), GeneratorState::Yielded(())); - assert_eq!(generator.as_mut().resume(), GeneratorState::Complete(())); + assert_eq!(generator.as_mut().resume(()), GeneratorState::Yielded(())); + assert_eq!(generator.as_mut().resume(()), GeneratorState::Complete(())); } diff --git a/src/test/ui/generator/xcrate-reachable.rs b/src/test/ui/generator/xcrate-reachable.rs index 9483ad7395ea1..1b1cff3387d9f 100644 --- a/src/test/ui/generator/xcrate-reachable.rs +++ b/src/test/ui/generator/xcrate-reachable.rs @@ -10,5 +10,5 @@ use std::ops::Generator; use std::pin::Pin; fn main() { - Pin::new(&mut foo::foo()).resume(); + Pin::new(&mut foo::foo()).resume(()); } diff --git a/src/test/ui/generator/xcrate.rs b/src/test/ui/generator/xcrate.rs index febf5c3583f30..40986bbeb6517 100644 --- a/src/test/ui/generator/xcrate.rs +++ b/src/test/ui/generator/xcrate.rs @@ -12,18 +12,18 @@ use std::pin::Pin; fn main() { let mut foo = xcrate::foo(); - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Complete(()) => {} s => panic!("bad state: {:?}", s), } let mut foo = xcrate::bar(3); - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Yielded(3) => {} s => panic!("bad state: {:?}", s), } - match Pin::new(&mut foo).resume() { + match Pin::new(&mut foo).resume(()) { GeneratorState::Complete(()) => {} s => panic!("bad state: {:?}", s), } diff --git a/src/test/ui/generator/yield-while-iterating.rs b/src/test/ui/generator/yield-while-iterating.rs index e42781d1279e7..985e5d8bdc838 100644 --- a/src/test/ui/generator/yield-while-iterating.rs +++ b/src/test/ui/generator/yield-while-iterating.rs @@ -43,7 +43,7 @@ fn yield_during_iter_borrowed_slice_3() { yield p; } }; - Pin::new(&mut b).resume(); + Pin::new(&mut b).resume(()); } fn yield_during_iter_borrowed_slice_4() { @@ -56,7 +56,7 @@ fn yield_during_iter_borrowed_slice_4() { } }; println!("{}", x[0]); //~ ERROR - Pin::new(&mut b).resume(); + Pin::new(&mut b).resume(()); } fn yield_during_range_iter() { @@ -69,7 +69,7 @@ fn yield_during_range_iter() { yield x; } }; - Pin::new(&mut b).resume(); + Pin::new(&mut b).resume(()); } fn main() { } diff --git a/src/test/ui/generator/yield-while-iterating.stderr b/src/test/ui/generator/yield-while-iterating.stderr index 6a96b25b19fb4..b6563475235c2 100644 --- a/src/test/ui/generator/yield-while-iterating.stderr +++ b/src/test/ui/generator/yield-while-iterating.stderr @@ -16,7 +16,7 @@ LL | for p in &mut x { ... LL | println!("{}", x[0]); | ^ immutable borrow occurs here -LL | Pin::new(&mut b).resume(); +LL | Pin::new(&mut b).resume(()); | ------ mutable borrow later used here error: aborting due to 2 previous errors diff --git a/src/test/ui/generator/yield-while-local-borrowed.rs b/src/test/ui/generator/yield-while-local-borrowed.rs index b643bbf3376fb..061a64dbc364d 100644 --- a/src/test/ui/generator/yield-while-local-borrowed.rs +++ b/src/test/ui/generator/yield-while-local-borrowed.rs @@ -15,7 +15,7 @@ fn borrow_local_inline() { yield(); println!("{}", a); }; - Pin::new(&mut b).resume(); + Pin::new(&mut b).resume(()); } fn borrow_local_inline_done() { @@ -26,7 +26,7 @@ fn borrow_local_inline_done() { } yield(); }; - Pin::new(&mut b).resume(); + Pin::new(&mut b).resume(()); } fn borrow_local() { @@ -43,7 +43,7 @@ fn borrow_local() { println!("{}", b); } }; - Pin::new(&mut b).resume(); + Pin::new(&mut b).resume(()); } fn main() { } diff --git a/src/test/ui/generator/yield-while-ref-reborrowed.rs b/src/test/ui/generator/yield-while-ref-reborrowed.rs index f54a4f468f6a0..a03ef945dd231 100644 --- a/src/test/ui/generator/yield-while-ref-reborrowed.rs +++ b/src/test/ui/generator/yield-while-ref-reborrowed.rs @@ -12,7 +12,7 @@ fn reborrow_shared_ref(x: &i32) { yield(); println!("{}", a); }; - Pin::new(&mut b).resume(); + Pin::new(&mut b).resume(()); } fn reborrow_mutable_ref(x: &mut i32) { @@ -23,7 +23,7 @@ fn reborrow_mutable_ref(x: &mut i32) { yield(); println!("{}", a); }; - Pin::new(&mut b).resume(); + Pin::new(&mut b).resume(()); } fn reborrow_mutable_ref_2(x: &mut i32) { @@ -34,7 +34,7 @@ fn reborrow_mutable_ref_2(x: &mut i32) { println!("{}", a); }; println!("{}", x); //~ ERROR - Pin::new(&mut b).resume(); + Pin::new(&mut b).resume(()); } fn main() { } diff --git a/src/test/ui/generator/yield-while-ref-reborrowed.stderr b/src/test/ui/generator/yield-while-ref-reborrowed.stderr index 4c37cd351732b..fd885660d0927 100644 --- a/src/test/ui/generator/yield-while-ref-reborrowed.stderr +++ b/src/test/ui/generator/yield-while-ref-reborrowed.stderr @@ -8,7 +8,7 @@ LL | let a = &mut *x; ... LL | println!("{}", x); | ^ second borrow occurs here -LL | Pin::new(&mut b).resume(); +LL | Pin::new(&mut b).resume(()); | ------ first borrow later used here error: aborting due to previous error diff --git a/src/test/ui/nll/issue-55850.rs b/src/test/ui/nll/issue-55850.rs index a8f7299f89937..e6279bd028e01 100644 --- a/src/test/ui/nll/issue-55850.rs +++ b/src/test/ui/nll/issue-55850.rs @@ -15,7 +15,7 @@ where type Item = G::Yield; fn next(&mut self) -> Option { - match Pin::new(&mut self.0).resume() { + match Pin::new(&mut self.0).resume(()) { Yielded(y) => Some(y), _ => None } From 0117033c721d35ade8d815e1fbf83f10d73f15e4 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Thu, 23 Jan 2020 19:13:36 +0100 Subject: [PATCH 02/25] Add a resume type param to the generator substs ...and unify it with `()` for now --- src/librustc/infer/opaque_types/mod.rs | 1 + src/librustc/traits/util.rs | 2 +- src/librustc/ty/structural_impls.rs | 4 +-- src/librustc/ty/sty.rs | 41 ++++++++++++++++++++------ src/librustc_typeck/check/closure.rs | 5 ++++ src/librustc_typeck/collect.rs | 2 +- 6 files changed, 42 insertions(+), 13 deletions(-) diff --git a/src/librustc/infer/opaque_types/mod.rs b/src/librustc/infer/opaque_types/mod.rs index fe3a5d149f676..d28507f6eb2e3 100644 --- a/src/librustc/infer/opaque_types/mod.rs +++ b/src/librustc/infer/opaque_types/mod.rs @@ -744,6 +744,7 @@ where substs.as_generator().return_ty(def_id, self.tcx).visit_with(self); substs.as_generator().yield_ty(def_id, self.tcx).visit_with(self); + substs.as_generator().resume_ty(def_id, self.tcx).visit_with(self); } _ => { ty.super_visit_with(self); diff --git a/src/librustc/traits/util.rs b/src/librustc/traits/util.rs index 947d66e38b434..d4c3518260c60 100644 --- a/src/librustc/traits/util.rs +++ b/src/librustc/traits/util.rs @@ -645,7 +645,7 @@ pub fn generator_trait_ref_and_outputs( ) -> ty::Binder<(ty::TraitRef<'tcx>, Ty<'tcx>, Ty<'tcx>)> { let trait_ref = ty::TraitRef { def_id: fn_trait_def_id, - substs: tcx.mk_substs_trait(self_ty, &[tcx.mk_unit().into()]), + substs: tcx.mk_substs_trait(self_ty, &[sig.skip_binder().resume_ty.into()]), }; ty::Binder::bind((trait_ref, sig.skip_binder().yield_ty, sig.skip_binder().return_ty)) } diff --git a/src/librustc/ty/structural_impls.rs b/src/librustc/ty/structural_impls.rs index c1ae4d9fe1724..9d00d27226320 100644 --- a/src/librustc/ty/structural_impls.rs +++ b/src/librustc/ty/structural_impls.rs @@ -598,8 +598,8 @@ impl<'a, 'tcx> Lift<'tcx> for ty::adjustment::AutoBorrow<'a> { impl<'a, 'tcx> Lift<'tcx> for ty::GenSig<'a> { type Lifted = ty::GenSig<'tcx>; fn lift_to_tcx(&self, tcx: TyCtxt<'tcx>) -> Option { - tcx.lift(&(self.yield_ty, self.return_ty)) - .map(|(yield_ty, return_ty)| ty::GenSig { yield_ty, return_ty }) + tcx.lift(&(self.resume_ty, self.yield_ty, self.return_ty)) + .map(|(resume_ty, yield_ty, return_ty)| ty::GenSig { resume_ty, yield_ty, return_ty }) } } diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index dffe86d946212..0d30395d2501b 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -346,9 +346,17 @@ static_assert_size!(TyKind<'_>, 24); /// ## Generators /// /// Generators are handled similarly in `GeneratorSubsts`. The set of -/// type parameters is similar, but the role of CK and CS are -/// different. CK represents the "yield type" and CS represents the -/// "return type" of the generator. +/// type parameters is similar, but `CK` and `CS` are replaced by the +/// following type parameters: +/// +/// * `GS`: The generator's "resume type", which is the type of the +/// argument passed to `resume`, and the type of `yield` expressions +/// inside the generator. +/// * `GY`: The "yield type", which is the type of values passed to +/// `yield` inside the generator. +/// * `GR`: The "return type", which is the type of value returned upon +/// completion of the generator. +/// * `GW`: The "generator witness". #[derive(Copy, Clone, Debug, TypeFoldable)] pub struct ClosureSubsts<'tcx> { /// Lifetime and type parameters from the enclosing function, @@ -442,6 +450,7 @@ pub struct GeneratorSubsts<'tcx> { } struct SplitGeneratorSubsts<'tcx> { + resume_ty: Ty<'tcx>, yield_ty: Ty<'tcx>, return_ty: Ty<'tcx>, witness: Ty<'tcx>, @@ -453,10 +462,11 @@ impl<'tcx> GeneratorSubsts<'tcx> { let generics = tcx.generics_of(def_id); let parent_len = generics.parent_count; SplitGeneratorSubsts { - yield_ty: self.substs.type_at(parent_len), - return_ty: self.substs.type_at(parent_len + 1), - witness: self.substs.type_at(parent_len + 2), - upvar_kinds: &self.substs[parent_len + 3..], + resume_ty: self.substs.type_at(parent_len), + yield_ty: self.substs.type_at(parent_len + 1), + return_ty: self.substs.type_at(parent_len + 2), + witness: self.substs.type_at(parent_len + 3), + upvar_kinds: &self.substs[parent_len + 4..], } } @@ -485,6 +495,11 @@ impl<'tcx> GeneratorSubsts<'tcx> { }) } + /// Returns the type representing the resume type of the generator. + pub fn resume_ty(self, def_id: DefId, tcx: TyCtxt<'_>) -> Ty<'tcx> { + self.split(def_id, tcx).resume_ty + } + /// Returns the type representing the yield type of the generator. pub fn yield_ty(self, def_id: DefId, tcx: TyCtxt<'_>) -> Ty<'tcx> { self.split(def_id, tcx).yield_ty @@ -505,10 +520,14 @@ impl<'tcx> GeneratorSubsts<'tcx> { ty::Binder::dummy(self.sig(def_id, tcx)) } - /// Returns the "generator signature", which consists of its yield + /// Returns the "generator signature", which consists of its resume, yield /// and return types. pub fn sig(self, def_id: DefId, tcx: TyCtxt<'_>) -> GenSig<'tcx> { - ty::GenSig { yield_ty: self.yield_ty(def_id, tcx), return_ty: self.return_ty(def_id, tcx) } + ty::GenSig { + resume_ty: self.resume_ty(def_id, tcx), + yield_ty: self.yield_ty(def_id, tcx), + return_ty: self.return_ty(def_id, tcx), + } } } @@ -1072,6 +1091,7 @@ impl<'tcx> ProjectionTy<'tcx> { #[derive(Clone, Debug, TypeFoldable)] pub struct GenSig<'tcx> { + pub resume_ty: Ty<'tcx>, pub yield_ty: Ty<'tcx>, pub return_ty: Ty<'tcx>, } @@ -1079,6 +1099,9 @@ pub struct GenSig<'tcx> { pub type PolyGenSig<'tcx> = Binder>; impl<'tcx> PolyGenSig<'tcx> { + pub fn resume_ty(&self) -> ty::Binder> { + self.map_bound_ref(|sig| sig.resume_ty) + } pub fn yield_ty(&self) -> ty::Binder> { self.map_bound_ref(|sig| sig.yield_ty) } diff --git a/src/librustc_typeck/check/closure.rs b/src/librustc_typeck/check/closure.rs index 084e6c8d083c5..97067e0b0551d 100644 --- a/src/librustc_typeck/check/closure.rs +++ b/src/librustc_typeck/check/closure.rs @@ -94,6 +94,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }); if let Some(GeneratorTypes { yield_ty, interior, movability }) = generator_types { let generator_substs = substs.as_generator(); + self.demand_eqtype( + expr.span, + self.tcx.mk_unit(), // WIP + generator_substs.resume_ty(expr_def_id, self.tcx), + ); self.demand_eqtype( expr.span, yield_ty, diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 4d812d2621c61..dc089c9045693 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -1189,7 +1189,7 @@ fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::Generics { // and we don't do that for closures. if let Node::Expr(&hir::Expr { kind: hir::ExprKind::Closure(.., gen), .. }) = node { let dummy_args = if gen.is_some() { - &["", "", ""][..] + &["", "", "", ""][..] } else { &["", ""][..] }; From 25af2f66cec1366f845e1de1bfec8b64d4f5cfff Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sat, 25 Jan 2020 02:27:05 +0100 Subject: [PATCH 03/25] Use real resume type as second argument --- src/librustc/ty/layout.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs index 966b60c6cfbc1..0a5ab790adbab 100644 --- a/src/librustc/ty/layout.rs +++ b/src/librustc/ty/layout.rs @@ -2350,7 +2350,8 @@ impl<'tcx> ty::Instance<'tcx> { ]); let ret_ty = tcx.mk_adt(state_adt_ref, state_substs); - tcx.mk_fn_sig([env_ty, tcx.mk_unit()].iter(), + tcx.mk_fn_sig( + [env_ty, sig.resume_ty].iter(), &ret_ty, false, hir::Unsafety::Normal, From 8a1227a67bd5df8a8f27c02b7032bd8092d44a92 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sat, 25 Jan 2020 02:27:51 +0100 Subject: [PATCH 04/25] Infer type of `yield` to be resume type --- src/librustc_typeck/check/closure.rs | 5 +++-- src/librustc_typeck/check/expr.rs | 11 +++++++---- src/librustc_typeck/check/mod.rs | 18 ++++++++++++++---- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/src/librustc_typeck/check/closure.rs b/src/librustc_typeck/check/closure.rs index 97067e0b0551d..fd6be8520510b 100644 --- a/src/librustc_typeck/check/closure.rs +++ b/src/librustc_typeck/check/closure.rs @@ -92,11 +92,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .into(), GenericParamDefKind::Const => span_bug!(expr.span, "closure has const param"), }); - if let Some(GeneratorTypes { yield_ty, interior, movability }) = generator_types { + if let Some(GeneratorTypes { resume_ty, yield_ty, interior, movability }) = generator_types + { let generator_substs = substs.as_generator(); self.demand_eqtype( expr.span, - self.tcx.mk_unit(), // WIP + resume_ty, generator_substs.resume_ty(expr_def_id, self.tcx), ); self.demand_eqtype( diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index b4c2b85241f96..9ce89bd636304 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -1796,9 +1796,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expr: &'tcx hir::Expr<'tcx>, src: &'tcx hir::YieldSource, ) -> Ty<'tcx> { - match self.yield_ty { - Some(ty) => { - self.check_expr_coercable_to_type(&value, ty); + match self.resume_yield_tys { + Some((resume_ty, yield_ty)) => { + self.check_expr_coercable_to_type(&value, yield_ty); + + resume_ty } // Given that this `yield` expression was generated as a result of lowering a `.await`, // we know that the yield type must be `()`; however, the context won't contain this @@ -1806,6 +1808,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // value's type against `()` (this check should always hold). None if src == &hir::YieldSource::Await => { self.check_expr_coercable_to_type(&value, self.tcx.mk_unit()); + self.tcx.mk_unit() } _ => { struct_span_err!( @@ -1815,9 +1818,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { "yield expression outside of generator literal" ) .emit(); + self.tcx.mk_unit() } } - self.tcx.mk_unit() } } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 0a917a1853eb5..9612500e3b019 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -573,7 +573,7 @@ pub struct FnCtxt<'a, 'tcx> { /// First span of a return site that we find. Used in error messages. ret_coercion_span: RefCell>, - yield_ty: Option>, + resume_yield_tys: Option<(Ty<'tcx>, Ty<'tcx>)>, ps: RefCell, @@ -1248,6 +1248,9 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherLocalsVisitor<'a, 'tcx> { /// includes yield), it returns back some information about the yield /// points. struct GeneratorTypes<'tcx> { + /// Type of generator argument / values returned by `yield`. + resume_ty: Ty<'tcx>, + /// Type of value that is yielded. yield_ty: Ty<'tcx>, @@ -1308,7 +1311,11 @@ fn check_fn<'a, 'tcx>( let yield_ty = fcx .next_ty_var(TypeVariableOrigin { kind: TypeVariableOriginKind::TypeInference, span }); fcx.require_type_is_sized(yield_ty, span, traits::SizedYieldType); - fcx.yield_ty = Some(yield_ty); + + // Resume type defaults to `()` if the generator has no argument. + let resume_ty = fn_sig.inputs().get(0).map(|ty| *ty).unwrap_or_else(|| tcx.mk_unit()); + + fcx.resume_yield_tys = Some((resume_ty, yield_ty)); } let outer_def_id = tcx.closure_base_def_id(hir.local_def_id(fn_id)); @@ -1361,8 +1368,11 @@ fn check_fn<'a, 'tcx>( let interior = fcx .next_ty_var(TypeVariableOrigin { kind: TypeVariableOriginKind::MiscVariable, span }); fcx.deferred_generator_interiors.borrow_mut().push((body.id(), interior, gen_kind)); + + let (resume_ty, yield_ty) = fcx.resume_yield_tys.unwrap(); Some(GeneratorTypes { - yield_ty: fcx.yield_ty.unwrap(), + resume_ty, + yield_ty, interior, movability: can_be_generator.unwrap(), }) @@ -2764,7 +2774,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err_count_on_creation: inh.tcx.sess.err_count(), ret_coercion: None, ret_coercion_span: RefCell::new(None), - yield_ty: None, + resume_yield_tys: None, ps: RefCell::new(UnsafetyState::function(hir::Unsafety::Normal, hir::CRATE_HIR_ID)), diverges: Cell::new(Diverges::Maybe), has_errors: Cell::new(false), From 32005fe1957fc163036fbe0da8b12d39a9fb54cb Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sat, 25 Jan 2020 02:28:41 +0100 Subject: [PATCH 05/25] Allow 0 or 1 explicit generator parameters --- src/librustc_ast_lowering/expr.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index 5dc855e935c07..0c4cfa1f6505a 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -688,12 +688,12 @@ impl<'hir> LoweringContext<'_, 'hir> { ) -> Option { match generator_kind { Some(hir::GeneratorKind::Gen) => { - if !decl.inputs.is_empty() { + if decl.inputs.len() > 1 { struct_span_err!( self.sess, fn_decl_span, E0628, - "generators cannot have explicit parameters" + "too many parameters for generator (expected 0 or 1 parameters)" ) .emit(); } From 2101a1fec0e53677e32d1389b44f70a987a97c8d Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sat, 25 Jan 2020 02:29:09 +0100 Subject: [PATCH 06/25] Adjust tests to type inference changes This makes some error messages ungreat, but those seem to be preexisting bugs that also apply to closures / return position `impl Trait` in general. --- .../generator-yielding-or-returning-itself.rs | 2 +- ...erator-yielding-or-returning-itself.stderr | 21 ++++++------ .../generator/no-parameters-on-generators.rs | 4 ++- .../no-parameters-on-generators.stderr | 32 +++++++++++++++---- .../type-mismatch-signature-deduction.rs | 6 ++-- .../type-mismatch-signature-deduction.stderr | 25 +++++++++++---- 6 files changed, 60 insertions(+), 30 deletions(-) diff --git a/src/test/ui/generator-yielding-or-returning-itself.rs b/src/test/ui/generator-yielding-or-returning-itself.rs index fd52667981873..30788e3c1864b 100644 --- a/src/test/ui/generator-yielding-or-returning-itself.rs +++ b/src/test/ui/generator-yielding-or-returning-itself.rs @@ -13,7 +13,7 @@ pub fn want_cyclic_generator_return(_: T) fn supply_cyclic_generator_return() { want_cyclic_generator_return(|| { - //~^ ERROR closure/generator type that references itself + //~^ ERROR type mismatch if false { yield None.unwrap(); } None.unwrap() }) diff --git a/src/test/ui/generator-yielding-or-returning-itself.stderr b/src/test/ui/generator-yielding-or-returning-itself.stderr index c9a71e03858f1..1572219cf4ac8 100644 --- a/src/test/ui/generator-yielding-or-returning-itself.stderr +++ b/src/test/ui/generator-yielding-or-returning-itself.stderr @@ -1,13 +1,13 @@ -error[E0644]: closure/generator type that references itself - --> $DIR/generator-yielding-or-returning-itself.rs:15:34 +error[E0271]: type mismatch resolving `<[generator@$DIR/generator-yielding-or-returning-itself.rs:15:34: 19:6 _] as std::ops::Generator>::Return == [generator@$DIR/generator-yielding-or-returning-itself.rs:15:34: 19:6 _]` + --> $DIR/generator-yielding-or-returning-itself.rs:15:5 | -LL | want_cyclic_generator_return(|| { - | __________________________________^ -LL | | -LL | | if false { yield None.unwrap(); } -LL | | None.unwrap() -LL | | }) - | |_____^ cyclic type of infinite size +LL | pub fn want_cyclic_generator_return(_: T) + | ---------------------------- +LL | where T: Generator + | ---------- required by this bound in `want_cyclic_generator_return` +... +LL | want_cyclic_generator_return(|| { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cyclic type of infinite size | = note: closures cannot capture themselves or take themselves as argument; this error may be the result of a recent compiler bug-fix, @@ -30,5 +30,4 @@ LL | want_cyclic_generator_yield(|| { error: aborting due to 2 previous errors -Some errors have detailed explanations: E0271, E0644. -For more information about an error, try `rustc --explain E0271`. +For more information about this error, try `rustc --explain E0271`. diff --git a/src/test/ui/generator/no-parameters-on-generators.rs b/src/test/ui/generator/no-parameters-on-generators.rs index 6b5a557933953..cad004895349f 100644 --- a/src/test/ui/generator/no-parameters-on-generators.rs +++ b/src/test/ui/generator/no-parameters-on-generators.rs @@ -1,8 +1,10 @@ #![feature(generators)] fn main() { - let gen = |start| { //~ ERROR generators cannot have explicit parameters + let gen = |start| { //~^ ERROR type inside generator must be known in this context yield; + //~^ ERROR type inside generator must be known in this context + //~| ERROR type inside generator must be known in this context }; } diff --git a/src/test/ui/generator/no-parameters-on-generators.stderr b/src/test/ui/generator/no-parameters-on-generators.stderr index 5e8e043a391ce..f5f83b0476905 100644 --- a/src/test/ui/generator/no-parameters-on-generators.stderr +++ b/src/test/ui/generator/no-parameters-on-generators.stderr @@ -1,9 +1,3 @@ -error[E0628]: generators cannot have explicit parameters - --> $DIR/no-parameters-on-generators.rs:4:15 - | -LL | let gen = |start| { - | ^^^^^^^ - error[E0698]: type inside generator must be known in this context --> $DIR/no-parameters-on-generators.rs:4:16 | @@ -16,6 +10,30 @@ note: the type is part of the generator because of this `yield` LL | yield; | ^^^^^ -error: aborting due to 2 previous errors +error[E0698]: type inside generator must be known in this context + --> $DIR/no-parameters-on-generators.rs:6:9 + | +LL | yield; + | ^^^^^ cannot infer type + | +note: the type is part of the generator because of this `yield` + --> $DIR/no-parameters-on-generators.rs:6:9 + | +LL | yield; + | ^^^^^ + +error[E0698]: type inside generator must be known in this context + --> $DIR/no-parameters-on-generators.rs:6:9 + | +LL | yield; + | ^^^^^ cannot infer type + | +note: the type is part of the generator because of this `yield` + --> $DIR/no-parameters-on-generators.rs:6:9 + | +LL | yield; + | ^^^^^ + +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0698`. diff --git a/src/test/ui/generator/type-mismatch-signature-deduction.rs b/src/test/ui/generator/type-mismatch-signature-deduction.rs index b9c6bc5d0796e..7774ff48f56b7 100644 --- a/src/test/ui/generator/type-mismatch-signature-deduction.rs +++ b/src/test/ui/generator/type-mismatch-signature-deduction.rs @@ -2,15 +2,15 @@ use std::ops::Generator; -fn foo() -> impl Generator { +fn foo() -> impl Generator { //~ ERROR type mismatch || { if false { - return Ok(6); //~ ERROR mismatched types [E0308] + return Ok(6); } yield (); - 5 + 5 //~ ERROR mismatched types [E0308] } } diff --git a/src/test/ui/generator/type-mismatch-signature-deduction.stderr b/src/test/ui/generator/type-mismatch-signature-deduction.stderr index 8606ecd33dab4..8de77798ff48e 100644 --- a/src/test/ui/generator/type-mismatch-signature-deduction.stderr +++ b/src/test/ui/generator/type-mismatch-signature-deduction.stderr @@ -1,12 +1,23 @@ error[E0308]: mismatched types - --> $DIR/type-mismatch-signature-deduction.rs:8:20 + --> $DIR/type-mismatch-signature-deduction.rs:13:9 | -LL | return Ok(6); - | ^^^^^ expected `i32`, found enum `std::result::Result` +LL | 5 + | ^ expected enum `std::result::Result`, found integer | - = note: expected type `i32` - found enum `std::result::Result<{integer}, _>` + = note: expected type `std::result::Result<{integer}, _>` + found type `{integer}` -error: aborting due to previous error +error[E0271]: type mismatch resolving `<[generator@$DIR/type-mismatch-signature-deduction.rs:6:5: 14:6 _] as std::ops::Generator>::Return == i32` + --> $DIR/type-mismatch-signature-deduction.rs:5:13 + | +LL | fn foo() -> impl Generator { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected enum `std::result::Result`, found `i32` + | + = note: expected enum `std::result::Result<{integer}, _>` + found type `i32` + = note: the return type of a function must have a statically known size + +error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0308`. +Some errors have detailed explanations: E0271, E0308. +For more information about an error, try `rustc --explain E0271`. From f2c1468965a7af5887d353adf77427344327be0d Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sat, 25 Jan 2020 02:30:46 +0100 Subject: [PATCH 07/25] Add resume arg place to `Yield` MIR terminator --- src/librustc/mir/mod.rs | 11 ++++++++--- src/librustc/mir/visit.rs | 6 ++++++ src/librustc_mir/borrow_check/invalidation.rs | 4 +++- src/librustc_mir/borrow_check/mod.rs | 4 +++- src/librustc_mir/borrow_check/universal_regions.rs | 4 +++- src/librustc_mir/dataflow/move_paths/builder.rs | 4 +++- 6 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 4e9835a52b779..f6c7174649fe8 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -1120,6 +1120,8 @@ pub enum TerminatorKind<'tcx> { value: Operand<'tcx>, /// Where to resume to. resume: BasicBlock, + /// The place to store the resume argument in. + resume_arg: Place<'tcx>, /// Cleanup to be done if the generator is dropped at this suspend point. drop: Option, }, @@ -2645,9 +2647,12 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> { target, unwind, }, - Yield { ref value, resume, drop } => { - Yield { value: value.fold_with(folder), resume: resume, drop: drop } - } + Yield { ref value, resume, ref resume_arg, drop } => Yield { + value: value.fold_with(folder), + resume, + resume_arg: resume_arg.fold_with(folder), + drop, + }, Call { ref func, ref args, ref destination, cleanup, from_hir_call } => { let dest = destination.as_ref().map(|&(ref loc, dest)| (loc.fold_with(folder), dest)); diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index 4c5db1b07d225..2f094516a35d9 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -516,8 +516,14 @@ macro_rules! make_mir_visitor { TerminatorKind::Yield { value, resume: _, + resume_arg, drop: _, } => { + self.visit_place( + resume_arg, + PlaceContext::MutatingUse(MutatingUseContext::Store), + source_location, + ); self.visit_operand(value, source_location); } diff --git a/src/librustc_mir/borrow_check/invalidation.rs b/src/librustc_mir/borrow_check/invalidation.rs index bb56c11872a29..392f164d3148c 100644 --- a/src/librustc_mir/borrow_check/invalidation.rs +++ b/src/librustc_mir/borrow_check/invalidation.rs @@ -159,7 +159,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { self.consume_operand(location, index); } } - TerminatorKind::Yield { ref value, resume, drop: _ } => { + TerminatorKind::Yield { ref value, resume, resume_arg, drop: _ } => { self.consume_operand(location, value); // Invalidate all borrows of local places @@ -170,6 +170,8 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { self.all_facts.invalidates.push((resume, i)); } } + + self.mutate_place(location, resume_arg, Deep, JustWrite); } TerminatorKind::Resume | TerminatorKind::Return | TerminatorKind::GeneratorDrop => { // Invalidate all borrows of local places diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 717359d75c3be..e528159fcef17 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -684,7 +684,7 @@ impl<'cx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx } } - TerminatorKind::Yield { ref value, resume: _, drop: _ } => { + TerminatorKind::Yield { ref value, resume: _, ref resume_arg, drop: _ } => { self.consume_operand(loc, (value, span), flow_state); if self.movable_generator { @@ -697,6 +697,8 @@ impl<'cx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx } }); } + + self.mutate_place(loc, (resume_arg, span), Deep, JustWrite, flow_state); } TerminatorKind::Resume | TerminatorKind::Return | TerminatorKind::GeneratorDrop => { diff --git a/src/librustc_mir/borrow_check/universal_regions.rs b/src/librustc_mir/borrow_check/universal_regions.rs index 6e36508ed60af..f6e3ca2f80900 100644 --- a/src/librustc_mir/borrow_check/universal_regions.rs +++ b/src/librustc_mir/borrow_check/universal_regions.rs @@ -581,9 +581,11 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> { DefiningTy::Generator(def_id, substs, movability) => { assert_eq!(self.mir_def_id, def_id); + let resume_ty = substs.as_generator().resume_ty(def_id, tcx); let output = substs.as_generator().return_ty(def_id, tcx); let generator_ty = tcx.mk_generator(def_id, substs, movability); - let inputs_and_output = self.infcx.tcx.intern_type_list(&[generator_ty, output]); + let inputs_and_output = + self.infcx.tcx.intern_type_list(&[generator_ty, resume_ty, output]); ty::Binder::dummy(inputs_and_output) } diff --git a/src/librustc_mir/dataflow/move_paths/builder.rs b/src/librustc_mir/dataflow/move_paths/builder.rs index 62af196174fd7..6f8caca5e21ef 100644 --- a/src/librustc_mir/dataflow/move_paths/builder.rs +++ b/src/librustc_mir/dataflow/move_paths/builder.rs @@ -380,7 +380,9 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { self.gather_operand(discr); } - TerminatorKind::Yield { ref value, .. } => { + TerminatorKind::Yield { ref value, resume_arg: ref place, .. } => { + self.create_move_path(place); + self.gather_init(place.as_ref(), InitKind::Deep); self.gather_operand(value); } From 3c069a066e50598ef230ba71ed5c5bcf596beb90 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sat, 25 Jan 2020 02:31:32 +0100 Subject: [PATCH 08/25] Change MIR building to fill in the resume place This changes `Yield` from `as_rvalue` to `into` lowering, which could have a possible performance impact. I could imagine special-casing some resume types here to use a simpler lowering for them, but it's unclear if that makes sense at this stage. --- .../build/expr/as_rvalue.rs | 14 ++------- src/librustc_mir_build/build/expr/category.rs | 2 +- src/librustc_mir_build/build/expr/into.rs | 21 ++++++++++++-- src/librustc_mir_build/build/mod.rs | 29 ++++++++++++------- 4 files changed, 40 insertions(+), 26 deletions(-) diff --git a/src/librustc_mir_build/build/expr/as_rvalue.rs b/src/librustc_mir_build/build/expr/as_rvalue.rs index 16795b459b6bd..6f5c5f0dd4c50 100644 --- a/src/librustc_mir_build/build/expr/as_rvalue.rs +++ b/src/librustc_mir_build/build/expr/as_rvalue.rs @@ -230,18 +230,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block = unpack!(this.stmt_expr(block, expr, None)); block.and(this.unit_rvalue()) } - ExprKind::Yield { value } => { - let value = unpack!(block = this.as_operand(block, scope, value)); - let resume = this.cfg.start_new_block(); - let cleanup = this.generator_drop_cleanup(); - this.cfg.terminate( - block, - source_info, - TerminatorKind::Yield { value: value, resume: resume, drop: cleanup }, - ); - resume.and(this.unit_rvalue()) - } - ExprKind::Literal { .. } + ExprKind::Yield { .. } + | ExprKind::Literal { .. } | ExprKind::StaticRef { .. } | ExprKind::Block { .. } | ExprKind::Match { .. } diff --git a/src/librustc_mir_build/build/expr/category.rs b/src/librustc_mir_build/build/expr/category.rs index c4d340953c925..cc139dee63f92 100644 --- a/src/librustc_mir_build/build/expr/category.rs +++ b/src/librustc_mir_build/build/expr/category.rs @@ -50,6 +50,7 @@ impl Category { | ExprKind::Adt { .. } | ExprKind::Borrow { .. } | ExprKind::AddressOf { .. } + | ExprKind::Yield { .. } | ExprKind::Call { .. } => Some(Category::Rvalue(RvalueFunc::Into)), ExprKind::Array { .. } @@ -63,7 +64,6 @@ impl Category { | ExprKind::Repeat { .. } | ExprKind::Assign { .. } | ExprKind::AssignOp { .. } - | ExprKind::Yield { .. } | ExprKind::InlineAsm { .. } => Some(Category::Rvalue(RvalueFunc::AsRvalue)), ExprKind::Literal { .. } | ExprKind::StaticRef { .. } => Some(Category::Constant), diff --git a/src/librustc_mir_build/build/expr/into.rs b/src/librustc_mir_build/build/expr/into.rs index 5ef338c624da2..51b0b5bc7cb0b 100644 --- a/src/librustc_mir_build/build/expr/into.rs +++ b/src/librustc_mir_build/build/expr/into.rs @@ -365,6 +365,24 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block.unit() } + ExprKind::Yield { value } => { + let scope = this.local_scope(); + let value = unpack!(block = this.as_operand(block, scope, value)); + let resume = this.cfg.start_new_block(); + let cleanup = this.generator_drop_cleanup(); + this.cfg.terminate( + block, + source_info, + TerminatorKind::Yield { + value, + resume, + resume_arg: destination.clone(), + drop: cleanup, + }, + ); + resume.unit() + } + // these are the cases that are more naturally handled by some other mode ExprKind::Unary { .. } | ExprKind::Binary { .. } @@ -376,8 +394,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | ExprKind::Tuple { .. } | ExprKind::Closure { .. } | ExprKind::Literal { .. } - | ExprKind::StaticRef { .. } - | ExprKind::Yield { .. } => { + | ExprKind::StaticRef { .. } => { debug_assert!(match Category::of(&expr.kind).unwrap() { // should be handled above Category::Rvalue(RvalueFunc::Into) => false, diff --git a/src/librustc_mir_build/build/mod.rs b/src/librustc_mir_build/build/mod.rs index ab539e6179e3d..4c8d8e3a0eac8 100644 --- a/src/librustc_mir_build/build/mod.rs +++ b/src/librustc_mir_build/build/mod.rs @@ -68,6 +68,12 @@ fn mir_build(tcx: TyCtxt<'_>, def_id: DefId) -> BodyAndCache<'_> { let fn_sig = cx.tables().liberated_fn_sigs()[id]; let fn_def_id = tcx.hir().local_def_id(id); + let safety = match fn_sig.unsafety { + hir::Unsafety::Normal => Safety::Safe, + hir::Unsafety::Unsafe => Safety::FnUnsafe, + }; + + let body = tcx.hir().body(body_id); let ty = tcx.type_of(fn_def_id); let mut abi = fn_sig.abi; let implicit_argument = match ty.kind { @@ -77,22 +83,23 @@ fn mir_build(tcx: TyCtxt<'_>, def_id: DefId) -> BodyAndCache<'_> { abi = Abi::Rust; vec![ArgInfo(liberated_closure_env_ty(tcx, id, body_id), None, None, None)] } - ty::Generator(..) => { + ty::Generator(def_id, substs, _) => { let gen_ty = tcx.body_tables(body_id).node_type(id); - vec![ - ArgInfo(gen_ty, None, None, None), - ArgInfo(tcx.mk_unit(), None, None, None), - ] + let resume_ty = substs.as_generator().resume_ty(def_id, tcx); + + // The resume argument may be missing, in that case we need to provide it here. + if body.params.is_empty() { + vec![ + ArgInfo(gen_ty, None, None, None), + ArgInfo(resume_ty, None, None, None), + ] + } else { + vec![ArgInfo(gen_ty, None, None, None)] + } } _ => vec![], }; - let safety = match fn_sig.unsafety { - hir::Unsafety::Normal => Safety::Safe, - hir::Unsafety::Unsafe => Safety::FnUnsafe, - }; - - let body = tcx.hir().body(body_id); let explicit_arguments = body.params.iter().enumerate().map(|(index, arg)| { let owner_id = tcx.hir().body_owner(body_id); let opt_ty_info; From 3c22e51e7f6debd96af76f36aa8b090c40b8acb6 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sat, 25 Jan 2020 02:33:52 +0100 Subject: [PATCH 09/25] Make generator transform move resume arg around The resume arg is passed as argument `_2` and needs to be moved to the `Yield`s target `Place` --- src/librustc_mir/transform/generator.rs | 26 ++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index a8defd03f7177..c0fefd60f83bc 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -192,9 +192,10 @@ const RETURNED: usize = GeneratorSubsts::RETURNED; /// Generator has been poisoned const POISONED: usize = GeneratorSubsts::POISONED; -struct SuspensionPoint { +struct SuspensionPoint<'tcx> { state: usize, resume: BasicBlock, + resume_arg: Place<'tcx>, drop: Option, storage_liveness: liveness::LiveVarSet, } @@ -216,7 +217,7 @@ struct TransformVisitor<'tcx> { storage_liveness: FxHashMap, // A list of suspension points, generated during the transform - suspension_points: Vec, + suspension_points: Vec>, // The original RETURN_PLACE local new_ret_local: Local, @@ -303,8 +304,8 @@ impl MutVisitor<'tcx> for TransformVisitor<'tcx> { Operand::Move(Place::from(self.new_ret_local)), None, )), - TerminatorKind::Yield { ref value, resume, drop } => { - Some((VariantIdx::new(0), Some(resume), value.clone(), drop)) + TerminatorKind::Yield { ref value, resume, resume_arg, drop } => { + Some((VariantIdx::new(0), Some((resume, resume_arg)), value.clone(), drop)) } _ => None, }; @@ -319,13 +320,14 @@ impl MutVisitor<'tcx> for TransformVisitor<'tcx> { self.make_state(state_idx, v), )), }); - let state = if let Some(resume) = resume { + let state = if let Some((resume, resume_arg)) = resume { // Yield let state = 3 + self.suspension_points.len(); self.suspension_points.push(SuspensionPoint { state, resume, + resume_arg, drop, storage_liveness: self.storage_liveness.get(&block).unwrap().clone(), }); @@ -1063,7 +1065,7 @@ fn create_cases<'tcx, F>( target: F, ) -> Vec<(usize, BasicBlock)> where - F: Fn(&SuspensionPoint) -> Option, + F: Fn(&SuspensionPoint<'tcx>) -> Option, { let source_info = source_info(body); @@ -1085,6 +1087,16 @@ where } } + // Move the resume argument to the destination place of the `Yield` terminator + let resume_arg = Local::new(2); // 0 = return, 1 = self + statements.push(Statement { + source_info, + kind: StatementKind::Assign(box ( + point.resume_arg, + Rvalue::Use(Operand::Move(resume_arg.into())), + )), + }); + // Then jump to the real target body.basic_blocks_mut().push(BasicBlockData { statements, @@ -1163,7 +1175,7 @@ impl<'tcx> MirPass<'tcx> for StateTransform { }; transform.visit_body(body); - // Update our MIR struct to reflect the changed we've made + // Update our MIR struct to reflect the changes we've made body.yield_ty = None; body.arg_count = 2; // self, resume arg body.spread_arg = None; From 5b2059b2572cff9974e6820791c8ab57b6c50234 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 24 Jan 2020 00:49:44 +0100 Subject: [PATCH 10/25] Fix error message on type mismatch in generator Instead of "closure is expected to take 0 arguments" we now get the expected type mismatch error. --- src/librustc_typeck/check/closure.rs | 5 +++-- src/test/ui/generator/type-mismatch-error.rs | 22 +++++++++++++++++++ .../ui/generator/type-mismatch-error.stderr | 19 ++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/generator/type-mismatch-error.rs create mode 100644 src/test/ui/generator/type-mismatch-error.stderr diff --git a/src/librustc_typeck/check/closure.rs b/src/librustc_typeck/check/closure.rs index fd6be8520510b..26777b3b01048 100644 --- a/src/librustc_typeck/check/closure.rs +++ b/src/librustc_typeck/check/closure.rs @@ -265,8 +265,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { _ => return None, } } else { - // Generators cannot have explicit arguments. - vec![] + // Generators with a `()` resume type may be defined with 0 or 1 explicit arguments, + // else they must have exactly 1 argument. For now though, just give up in this case. + return None; }; let ret_param_ty = projection.skip_binder().ty; diff --git a/src/test/ui/generator/type-mismatch-error.rs b/src/test/ui/generator/type-mismatch-error.rs new file mode 100644 index 0000000000000..d39c788a84bd4 --- /dev/null +++ b/src/test/ui/generator/type-mismatch-error.rs @@ -0,0 +1,22 @@ +//! Test that we get the expected type mismatch error instead of "closure is expected to take 0 +//! arguments" (which got introduced after implementing resume arguments). + +#![feature(generators, generator_trait)] + +use std::ops::Generator; + +fn f(_: G, _: G::Return) {} + +fn main() { + f( + |a: u8| { + if false { + yield (); + } else { + a + //~^ error: `if` and `else` have incompatible types + } + }, + 0u8, + ); +} diff --git a/src/test/ui/generator/type-mismatch-error.stderr b/src/test/ui/generator/type-mismatch-error.stderr new file mode 100644 index 0000000000000..8f5949533e2c7 --- /dev/null +++ b/src/test/ui/generator/type-mismatch-error.stderr @@ -0,0 +1,19 @@ +error[E0308]: `if` and `else` have incompatible types + --> $DIR/type-mismatch-error.rs:16:17 + | +LL | / if false { +LL | | yield (); + | | --------- + | | | | + | | | help: consider removing this semicolon + | | expected because of this +LL | | } else { +LL | | a + | | ^ expected `()`, found `u8` +LL | | +LL | | } + | |_____________- `if` and `else` have incompatible types + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. From fca614eb578092fd869df57d6654ba0dcf92c6ef Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 24 Jan 2020 21:56:08 +0100 Subject: [PATCH 11/25] Add tests for generator resume arguments --- src/test/ui/generator/retain-resume-ref.rs | 25 +++++ .../ui/generator/retain-resume-ref.stderr | 13 +++ src/test/ui/generator/smoke-resume-args.rs | 97 +++++++++++++++++++ 3 files changed, 135 insertions(+) create mode 100644 src/test/ui/generator/retain-resume-ref.rs create mode 100644 src/test/ui/generator/retain-resume-ref.stderr create mode 100644 src/test/ui/generator/smoke-resume-args.rs diff --git a/src/test/ui/generator/retain-resume-ref.rs b/src/test/ui/generator/retain-resume-ref.rs new file mode 100644 index 0000000000000..0606ea71cdf37 --- /dev/null +++ b/src/test/ui/generator/retain-resume-ref.rs @@ -0,0 +1,25 @@ +//! This test ensures that a mutable reference cannot be passed as a resume argument twice. + +#![feature(generators, generator_trait)] + +use std::marker::Unpin; +use std::ops::{ + Generator, + GeneratorState::{self, *}, +}; +use std::pin::Pin; + +fn main() { + let mut thing = String::from("hello"); + + let mut gen = |r| { + if false { + yield r; + } + }; + + let mut gen = Pin::new(&mut gen); + gen.as_mut().resume(&mut thing); + gen.as_mut().resume(&mut thing); + //~^ cannot borrow `thing` as mutable more than once at a time +} diff --git a/src/test/ui/generator/retain-resume-ref.stderr b/src/test/ui/generator/retain-resume-ref.stderr new file mode 100644 index 0000000000000..e33310d12d9ef --- /dev/null +++ b/src/test/ui/generator/retain-resume-ref.stderr @@ -0,0 +1,13 @@ +error[E0499]: cannot borrow `thing` as mutable more than once at a time + --> $DIR/retain-resume-ref.rs:23:25 + | +LL | gen.as_mut().resume(&mut thing); + | ---------- first mutable borrow occurs here +LL | gen.as_mut().resume(&mut thing); + | ------ ^^^^^^^^^^ second mutable borrow occurs here + | | + | first borrow later used by call + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0499`. diff --git a/src/test/ui/generator/smoke-resume-args.rs b/src/test/ui/generator/smoke-resume-args.rs new file mode 100644 index 0000000000000..32f3ee32d77b9 --- /dev/null +++ b/src/test/ui/generator/smoke-resume-args.rs @@ -0,0 +1,97 @@ +// run-pass + +#![feature(generators, generator_trait)] + +use std::fmt::Debug; +use std::marker::Unpin; +use std::ops::{ + Generator, + GeneratorState::{self, *}, +}; +use std::pin::Pin; +use std::sync::atomic::{AtomicUsize, Ordering}; + +fn drain + Unpin, R, Y>( + gen: &mut G, + inout: Vec<(R, GeneratorState)>, +) where + Y: Debug + PartialEq, + G::Return: Debug + PartialEq, +{ + let mut gen = Pin::new(gen); + + for (input, out) in inout { + assert_eq!(gen.as_mut().resume(input), out); + } +} + +static DROPS: AtomicUsize = AtomicUsize::new(0); + +#[derive(Debug, PartialEq)] +struct DropMe; + +impl Drop for DropMe { + fn drop(&mut self) { + DROPS.fetch_add(1, Ordering::SeqCst); + } +} + +fn expect_drops(expected_drops: usize, f: impl FnOnce() -> T) -> T { + DROPS.store(0, Ordering::SeqCst); + + let res = f(); + + let actual_drops = DROPS.load(Ordering::SeqCst); + assert_eq!(actual_drops, expected_drops); + res +} + +fn main() { + drain( + &mut |mut b| { + while b != 0 { + b = yield (b + 1); + } + -1 + }, + vec![(1, Yielded(2)), (-45, Yielded(-44)), (500, Yielded(501)), (0, Complete(-1))], + ); + + expect_drops(2, || drain(&mut |a| yield a, vec![(DropMe, Yielded(DropMe))])); + + expect_drops(6, || { + drain( + &mut |a| yield yield a, + vec![(DropMe, Yielded(DropMe)), (DropMe, Yielded(DropMe)), (DropMe, Complete(DropMe))], + ) + }); + + #[allow(unreachable_code)] + expect_drops(2, || drain(&mut |a| yield return a, vec![(DropMe, Complete(DropMe))])); + + expect_drops(2, || { + drain( + &mut |a: DropMe| { + if false { yield () } else { a } + }, + vec![(DropMe, Complete(DropMe))], + ) + }); + + expect_drops(4, || { + drain( + #[allow(unused_assignments, unused_variables)] + &mut |mut a: DropMe| { + a = yield; + a = yield; + a = yield; + }, + vec![ + (DropMe, Yielded(())), + (DropMe, Yielded(())), + (DropMe, Yielded(())), + (DropMe, Complete(())), + ], + ) + }); +} From 4ee857c4c3ecb15d4bc5aebe4fa7ba67dce797b5 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 24 Jan 2020 23:00:45 +0100 Subject: [PATCH 12/25] Add test for E0628 (too many generator parameters) --- src/test/ui/generator/too-many-parameters.rs | 7 +++++++ src/test/ui/generator/too-many-parameters.stderr | 8 ++++++++ 2 files changed, 15 insertions(+) create mode 100644 src/test/ui/generator/too-many-parameters.rs create mode 100644 src/test/ui/generator/too-many-parameters.stderr diff --git a/src/test/ui/generator/too-many-parameters.rs b/src/test/ui/generator/too-many-parameters.rs new file mode 100644 index 0000000000000..a0a27d9068241 --- /dev/null +++ b/src/test/ui/generator/too-many-parameters.rs @@ -0,0 +1,7 @@ +#![feature(generators)] + +fn main() { + |(), ()| { //~ error: too many parameters for generator + yield; + }; +} diff --git a/src/test/ui/generator/too-many-parameters.stderr b/src/test/ui/generator/too-many-parameters.stderr new file mode 100644 index 0000000000000..0dbe5f3f6fde9 --- /dev/null +++ b/src/test/ui/generator/too-many-parameters.stderr @@ -0,0 +1,8 @@ +error[E0628]: too many parameters for generator (expected 0 or 1 parameters) + --> $DIR/too-many-parameters.rs:4:5 + | +LL | |(), ()| { + | ^^^^^^^^ + +error: aborting due to previous error + From 7a9709b08a57620a791259b53d71a59acf8f7b5c Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 24 Jan 2020 23:55:54 +0100 Subject: [PATCH 13/25] Fix bootstrap rustc build --- src/librustc_data_structures/box_region.rs | 39 ++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/librustc_data_structures/box_region.rs b/src/librustc_data_structures/box_region.rs index 94abb89503c03..dbc54291f4087 100644 --- a/src/librustc_data_structures/box_region.rs +++ b/src/librustc_data_structures/box_region.rs @@ -25,6 +25,7 @@ pub struct PinnedGenerator { } impl PinnedGenerator { + #[cfg(bootstrap)] pub fn new, Return = R> + 'static>( generator: T, ) -> (I, Self) { @@ -39,6 +40,22 @@ impl PinnedGenerator { (init, result) } + #[cfg(not(bootstrap))] + pub fn new, Return = R> + 'static>( + generator: T, + ) -> (I, Self) { + let mut result = PinnedGenerator { generator: Box::pin(generator) }; + + // Run it to the first yield to set it up + let init = match Pin::new(&mut result.generator).resume(()) { + GeneratorState::Yielded(YieldType::Initial(y)) => y, + _ => panic!(), + }; + + (init, result) + } + + #[cfg(bootstrap)] pub unsafe fn access(&mut self, closure: *mut dyn FnMut()) { BOX_REGION_ARG.with(|i| { i.set(Action::Access(AccessAction(closure))); @@ -50,6 +67,19 @@ impl PinnedGenerator { } } + #[cfg(not(bootstrap))] + pub unsafe fn access(&mut self, closure: *mut dyn FnMut()) { + BOX_REGION_ARG.with(|i| { + i.set(Action::Access(AccessAction(closure))); + }); + + // Call the generator, which in turn will call the closure in BOX_REGION_ARG + if let GeneratorState::Complete(_) = Pin::new(&mut self.generator).resume(()) { + panic!() + } + } + + #[cfg(bootstrap)] pub fn complete(&mut self) -> R { // Tell the generator we want it to complete, consuming it and yielding a result BOX_REGION_ARG.with(|i| i.set(Action::Complete)); @@ -57,6 +87,15 @@ impl PinnedGenerator { let result = Pin::new(&mut self.generator).resume(); if let GeneratorState::Complete(r) = result { r } else { panic!() } } + + #[cfg(not(bootstrap))] + pub fn complete(&mut self) -> R { + // Tell the generator we want it to complete, consuming it and yielding a result + BOX_REGION_ARG.with(|i| i.set(Action::Complete)); + + let result = Pin::new(&mut self.generator).resume(()); + if let GeneratorState::Complete(r) = result { r } else { panic!() } + } } #[derive(PartialEq)] From 3bb8ecb512a632fb3f5568e4e443cfe46f2f3ea6 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sat, 25 Jan 2020 16:03:44 +0100 Subject: [PATCH 14/25] Adjust mir-opt tests to new `yield` lowering --- src/test/mir-opt/generator-drop-cleanup.rs | 8 +-- .../mir-opt/generator-storage-dead-unwind.rs | 52 +++++++++---------- 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/src/test/mir-opt/generator-drop-cleanup.rs b/src/test/mir-opt/generator-drop-cleanup.rs index f97e1ba6c89d4..307370f14358e 100644 --- a/src/test/mir-opt/generator-drop-cleanup.rs +++ b/src/test/mir-opt/generator-drop-cleanup.rs @@ -13,12 +13,12 @@ fn main() { // START rustc.main-{{closure}}.generator_drop.0.mir // bb0: { -// _5 = discriminant((*_1)); -// switchInt(move _5) -> [0u32: bb4, 3u32: bb7, otherwise: bb8]; +// _6 = discriminant((*_1)); +// switchInt(move _6) -> [0u32: bb4, 3u32: bb7, otherwise: bb8]; // } // bb1: { +// StorageDead(_4); // StorageDead(_3); -// StorageDead(_2); // goto -> bb5; // } // bb2: { @@ -39,6 +39,8 @@ fn main() { // bb7: { // StorageLive(_2); // StorageLive(_3); +// StorageLive(_4); +// _3 = move _2; // goto -> bb1; // } // bb8: { diff --git a/src/test/mir-opt/generator-storage-dead-unwind.rs b/src/test/mir-opt/generator-storage-dead-unwind.rs index ecce0a08c7bf0..4442fa5f52126 100644 --- a/src/test/mir-opt/generator-storage-dead-unwind.rs +++ b/src/test/mir-opt/generator-storage-dead-unwind.rs @@ -31,81 +31,81 @@ fn main() { // START rustc.main-{{closure}}.StateTransform.before.mir // ... -// let _2: Foo; +// let _3: Foo; // ... -// let mut _7: Foo; +// let mut _8: Foo; // ... -// let mut _9: Bar; +// let mut _10: Bar; // scope 1 { -// debug a => _2; -// let _3: Bar; +// debug a => _3; +// let _4: Bar; // scope 2 { -// debug b => _3; +// debug b => _4; // } // } // bb0: { -// StorageLive(_2); -// _2 = Foo(const 5i32,); // StorageLive(_3); -// _3 = Bar(const 6i32,); +// _3 = Foo(const 5i32,); +// StorageLive(_4); +// _4 = Bar(const 6i32,); // ... -// _1 = suspend(move _5) -> [resume: bb2, drop: bb4]; +// _1 = suspend(move _6) -> [resume: bb2, drop: bb4]; // } // bb1 (cleanup): { // resume; // } // bb2: { // ... -// StorageLive(_6); // StorageLive(_7); -// _7 = move _2; -// _6 = const take::(move _7) -> [return: bb7, unwind: bb9]; +// StorageLive(_8); +// _8 = move _3; +// _7 = const take::(move _8) -> [return: bb7, unwind: bb9]; // } // bb3 (cleanup): { -// StorageDead(_2); +// StorageDead(_3); // drop(_1) -> bb1; // } // bb4: { // ... -// StorageDead(_3); -// drop(_2) -> [return: bb5, unwind: bb3]; +// StorageDead(_4); +// drop(_3) -> [return: bb5, unwind: bb3]; // } // bb5: { -// StorageDead(_2); +// StorageDead(_3); // drop(_1) -> [return: bb6, unwind: bb1]; // } // bb6: { // generator_drop; // } // bb7: { +// StorageDead(_8); // StorageDead(_7); -// StorageDead(_6); -// StorageLive(_8); // StorageLive(_9); -// _9 = move _3; -// _8 = const take::(move _9) -> [return: bb10, unwind: bb11]; +// StorageLive(_10); +// _10 = move _4; +// _9 = const take::(move _10) -> [return: bb10, unwind: bb11]; // } // bb8 (cleanup): { +// StorageDead(_4); // StorageDead(_3); -// StorageDead(_2); // drop(_1) -> bb1; // } // bb9 (cleanup): { +// StorageDead(_8); // StorageDead(_7); -// StorageDead(_6); // goto -> bb8; // } // bb10: { +// StorageDead(_10); // StorageDead(_9); -// StorageDead(_8); // ... +// StorageDead(_4); // StorageDead(_3); -// StorageDead(_2); // drop(_1) -> [return: bb12, unwind: bb1]; // } // bb11 (cleanup): { +// StorageDead(_10); // StorageDead(_9); -// StorageDead(_8); // goto -> bb8; // } // bb12: { From aae0f543cff16afe27384171e83de91887bc9f4d Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 27 Jan 2020 22:32:57 +0100 Subject: [PATCH 15/25] No resume argument in the drop shim --- src/librustc_mir/transform/generator.rs | 51 ++++++++++++++-------- src/test/mir-opt/generator-drop-cleanup.rs | 1 - 2 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index c0fefd60f83bc..b5edcbe457b63 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -891,7 +891,7 @@ fn create_generator_drop_shim<'tcx>( let source_info = source_info(&body); - let mut cases = create_cases(&mut body, transform, |point| point.drop); + let mut cases = create_cases(&mut body, transform, Operation::Drop); cases.insert(0, (UNRESUMED, drop_clean)); @@ -1009,7 +1009,7 @@ fn create_generator_resume_function<'tcx>( } } - let mut cases = create_cases(body, &transform, |point| Some(point.resume)); + let mut cases = create_cases(body, &transform, Operation::Resume); use rustc::mir::interpret::PanicInfo::{ResumedAfterPanic, ResumedAfterReturn}; @@ -1059,14 +1059,27 @@ fn insert_clean_drop(body: &mut BodyAndCache<'_>) -> BasicBlock { drop_clean } -fn create_cases<'tcx, F>( +/// An operation that can be performed on a generator. +#[derive(PartialEq, Copy, Clone)] +enum Operation { + Resume, + Drop, +} + +impl Operation { + fn target_block(self, point: &SuspensionPoint<'_>) -> Option { + match self { + Operation::Resume => Some(point.resume), + Operation::Drop => point.drop, + } + } +} + +fn create_cases<'tcx>( body: &mut BodyAndCache<'tcx>, transform: &TransformVisitor<'tcx>, - target: F, -) -> Vec<(usize, BasicBlock)> -where - F: Fn(&SuspensionPoint<'tcx>) -> Option, -{ + operation: Operation, +) -> Vec<(usize, BasicBlock)> { let source_info = source_info(body); transform @@ -1074,7 +1087,7 @@ where .iter() .filter_map(|point| { // Find the target for this suspension point, if applicable - target(point).map(|target| { + operation.target_block(point).map(|target| { let block = BasicBlock::new(body.basic_blocks().len()); let mut statements = Vec::new(); @@ -1087,15 +1100,17 @@ where } } - // Move the resume argument to the destination place of the `Yield` terminator - let resume_arg = Local::new(2); // 0 = return, 1 = self - statements.push(Statement { - source_info, - kind: StatementKind::Assign(box ( - point.resume_arg, - Rvalue::Use(Operand::Move(resume_arg.into())), - )), - }); + if operation == Operation::Resume { + // Move the resume argument to the destination place of the `Yield` terminator + let resume_arg = Local::new(2); // 0 = return, 1 = self + statements.push(Statement { + source_info, + kind: StatementKind::Assign(box ( + point.resume_arg, + Rvalue::Use(Operand::Move(resume_arg.into())), + )), + }); + } // Then jump to the real target body.basic_blocks_mut().push(BasicBlockData { diff --git a/src/test/mir-opt/generator-drop-cleanup.rs b/src/test/mir-opt/generator-drop-cleanup.rs index 307370f14358e..e5f3f9221c098 100644 --- a/src/test/mir-opt/generator-drop-cleanup.rs +++ b/src/test/mir-opt/generator-drop-cleanup.rs @@ -40,7 +40,6 @@ fn main() { // StorageLive(_2); // StorageLive(_3); // StorageLive(_4); -// _3 = move _2; // goto -> bb1; // } // bb8: { From 9fa46fe15367a8154f38371d0d913fa2f97f6416 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Mon, 27 Jan 2020 23:26:37 +0100 Subject: [PATCH 16/25] Teach dropck about resume arguments --- src/librustc_traits/dropck_outlives.rs | 5 +-- src/test/ui/generator/dropck-resume.rs | 33 +++++++++++++++++++ src/test/ui/generator/dropck-resume.stderr | 15 +++++++++ .../ui/generator/retain-resume-ref.stderr | 7 ++-- 4 files changed, 55 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/generator/dropck-resume.rs create mode 100644 src/test/ui/generator/dropck-resume.stderr diff --git a/src/librustc_traits/dropck_outlives.rs b/src/librustc_traits/dropck_outlives.rs index bc4d03cca7fee..346d2a931d10b 100644 --- a/src/librustc_traits/dropck_outlives.rs +++ b/src/librustc_traits/dropck_outlives.rs @@ -227,8 +227,8 @@ fn dtorck_constraint_for_ty<'tcx>( // In particular, skipping over `_interior` is safe // because any side-effects from dropping `_interior` can // only take place through references with lifetimes - // derived from lifetimes attached to the upvars, and we - // *do* incorporate the upvars here. + // derived from lifetimes attached to the upvars and resume + // argument, and we *do* incorporate those here. constraints.outlives.extend( substs @@ -236,6 +236,7 @@ fn dtorck_constraint_for_ty<'tcx>( .upvar_tys(def_id, tcx) .map(|t| -> ty::subst::GenericArg<'tcx> { t.into() }), ); + constraints.outlives.push(substs.as_generator().resume_ty(def_id, tcx).into()); } ty::Adt(def, substs) => { diff --git a/src/test/ui/generator/dropck-resume.rs b/src/test/ui/generator/dropck-resume.rs new file mode 100644 index 0000000000000..4c18077f33573 --- /dev/null +++ b/src/test/ui/generator/dropck-resume.rs @@ -0,0 +1,33 @@ +#![feature(generators, generator_trait)] + +use std::ops::{Generator, GeneratorState}; +use std::pin::Pin; + +struct SetToNone<'a: 'b, 'b>(&'b mut Option<&'a i32>); + +impl<'a, 'b> Drop for SetToNone<'a, 'b> { + fn drop(&mut self) { + *self.0 = None; + } +} + +fn drop_using_generator() -> i32 { + let mut y = Some(&0); + let z = &mut y; + let r; + { + let mut g = move |r| { + let _s = SetToNone(r); + yield; + }; + let mut g = Pin::new(&mut g); + g.as_mut().resume(z); + r = y.as_ref().unwrap(); + //~^ ERROR cannot borrow `y` as immutable because it is also borrowed as mutable + } + **r +} + +fn main() { + println!("{}", drop_using_generator()); +} diff --git a/src/test/ui/generator/dropck-resume.stderr b/src/test/ui/generator/dropck-resume.stderr new file mode 100644 index 0000000000000..ecf92e7e3ae79 --- /dev/null +++ b/src/test/ui/generator/dropck-resume.stderr @@ -0,0 +1,15 @@ +error[E0502]: cannot borrow `y` as immutable because it is also borrowed as mutable + --> $DIR/dropck-resume.rs:25:13 + | +LL | let z = &mut y; + | ------ mutable borrow occurs here +... +LL | r = y.as_ref().unwrap(); + | ^ immutable borrow occurs here +LL | +LL | } + | - mutable borrow might be used here, when `g` is dropped and runs the destructor for generator + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0502`. diff --git a/src/test/ui/generator/retain-resume-ref.stderr b/src/test/ui/generator/retain-resume-ref.stderr index e33310d12d9ef..bc715c7030eb3 100644 --- a/src/test/ui/generator/retain-resume-ref.stderr +++ b/src/test/ui/generator/retain-resume-ref.stderr @@ -4,9 +4,10 @@ error[E0499]: cannot borrow `thing` as mutable more than once at a time LL | gen.as_mut().resume(&mut thing); | ---------- first mutable borrow occurs here LL | gen.as_mut().resume(&mut thing); - | ------ ^^^^^^^^^^ second mutable borrow occurs here - | | - | first borrow later used by call + | ^^^^^^^^^^ second mutable borrow occurs here +LL | +LL | } + | - first borrow might be used here, when `gen` is dropped and runs the destructor for generator error: aborting due to previous error From 392e59500a96be718383e127d38bf74300f521c0 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Sun, 2 Feb 2020 22:46:06 +0100 Subject: [PATCH 17/25] Fix miscompilation --- src/librustc_mir/transform/generator.rs | 63 ++++++++++++++++--- src/test/mir-opt/generator-drop-cleanup.rs | 5 +- .../ui/generator/resume-live-across-yield.rs | 45 +++++++++++++ 3 files changed, 100 insertions(+), 13 deletions(-) create mode 100644 src/test/ui/generator/resume-live-across-yield.rs diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index b5edcbe457b63..414d91a18be12 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -380,28 +380,35 @@ fn make_generator_state_argument_pinned<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body PinArgVisitor { ref_gen_ty, tcx }.visit_body(body); } -fn replace_result_variable<'tcx>( - ret_ty: Ty<'tcx>, +/// Allocates a new local and replaces all references of `local` with it. Returns the new local. +/// +/// `local` will be changed to a new local decl with type `ty`. +/// +/// Note that the new local will be uninitialized. It is the caller's responsibility to assign some +/// valid value to it before its first use. +fn replace_local<'tcx>( + local: Local, + ty: Ty<'tcx>, body: &mut BodyAndCache<'tcx>, tcx: TyCtxt<'tcx>, ) -> Local { let source_info = source_info(body); - let new_ret = LocalDecl { + let new_decl = LocalDecl { mutability: Mutability::Mut, - ty: ret_ty, + ty, user_ty: UserTypeProjections::none(), source_info, internal: false, is_block_tail: None, local_info: LocalInfo::Other, }; - let new_ret_local = Local::new(body.local_decls.len()); - body.local_decls.push(new_ret); - body.local_decls.swap(RETURN_PLACE, new_ret_local); + let new_local = Local::new(body.local_decls.len()); + body.local_decls.push(new_decl); + body.local_decls.swap(local, new_local); - RenameLocalVisitor { from: RETURN_PLACE, to: new_ret_local, tcx }.visit_body(body); + RenameLocalVisitor { from: local, to: new_local, tcx }.visit_body(body); - new_ret_local + new_local } struct StorageIgnored(liveness::LiveVarSet); @@ -794,6 +801,10 @@ fn compute_layout<'tcx>( (remap, layout, storage_liveness) } +/// Replaces the entry point of `body` with a block that switches on the generator discriminant and +/// dispatches to blocks according to `cases`. +/// +/// After this function, the former entry point of the function will be bb1. fn insert_switch<'tcx>( body: &mut BodyAndCache<'tcx>, cases: Vec<(usize, BasicBlock)>, @@ -1093,6 +1104,13 @@ fn create_cases<'tcx>( // Create StorageLive instructions for locals with live storage for i in 0..(body.local_decls.len()) { + if i == 2 { + // The resume argument is live on function entry. Don't insert a + // `StorageLive`, or the following `Assign` will read from uninitialized + // memory. + continue; + } + let l = Local::new(i); if point.storage_liveness.contains(l) && !transform.remap.contains_key(&l) { statements @@ -1110,6 +1128,10 @@ fn create_cases<'tcx>( Rvalue::Use(Operand::Move(resume_arg.into())), )), }); + statements.push(Statement { + source_info, + kind: StatementKind::StorageDead(resume_arg), + }); } // Then jump to the real target @@ -1166,7 +1188,28 @@ impl<'tcx> MirPass<'tcx> for StateTransform { // We rename RETURN_PLACE which has type mir.return_ty to new_ret_local // RETURN_PLACE then is a fresh unused local with type ret_ty. - let new_ret_local = replace_result_variable(ret_ty, body, tcx); + let new_ret_local = replace_local(RETURN_PLACE, ret_ty, body, tcx); + + // We also replace the resume argument and insert an `Assign`. + // This is needed because the resume argument might be live across a `yield`, and the + // transform assumes that any local live across a `yield` is assigned to before that. + let resume_local = Local::new(2); + let new_resume_local = + replace_local(resume_local, body.local_decls[resume_local].ty, body, tcx); + + // When first entering the generator, move the resume argument into its new local. + let source_info = source_info(body); + let stmts = &mut body.basic_blocks_mut()[BasicBlock::new(0)].statements; + stmts.insert( + 0, + Statement { + source_info, + kind: StatementKind::Assign(box ( + new_resume_local.into(), + Rvalue::Use(Operand::Move(resume_local.into())), + )), + }, + ); // Extract locals which are live across suspension point into `layout` // `remap` gives a mapping from local indices onto generator struct indices diff --git a/src/test/mir-opt/generator-drop-cleanup.rs b/src/test/mir-opt/generator-drop-cleanup.rs index e5f3f9221c098..278dc49c92605 100644 --- a/src/test/mir-opt/generator-drop-cleanup.rs +++ b/src/test/mir-opt/generator-drop-cleanup.rs @@ -13,8 +13,8 @@ fn main() { // START rustc.main-{{closure}}.generator_drop.0.mir // bb0: { -// _6 = discriminant((*_1)); -// switchInt(move _6) -> [0u32: bb4, 3u32: bb7, otherwise: bb8]; +// _7 = discriminant((*_1)); +// switchInt(move _7) -> [0u32: bb4, 3u32: bb7, otherwise: bb8]; // } // bb1: { // StorageDead(_4); @@ -37,7 +37,6 @@ fn main() { // goto -> bb3; // } // bb7: { -// StorageLive(_2); // StorageLive(_3); // StorageLive(_4); // goto -> bb1; diff --git a/src/test/ui/generator/resume-live-across-yield.rs b/src/test/ui/generator/resume-live-across-yield.rs new file mode 100644 index 0000000000000..4c4cf117a5563 --- /dev/null +++ b/src/test/ui/generator/resume-live-across-yield.rs @@ -0,0 +1,45 @@ +// run-pass + +#![feature(generators, generator_trait)] + +use std::ops::{Generator, GeneratorState}; +use std::pin::Pin; +use std::sync::atomic::{AtomicUsize, Ordering}; + +static DROP: AtomicUsize = AtomicUsize::new(0); + +#[derive(PartialEq, Eq, Debug)] +struct Dropper(String); + +impl Drop for Dropper { + fn drop(&mut self) { + DROP.fetch_add(1, Ordering::SeqCst); + } +} + +fn main() { + let mut g = |mut _d| { + _d = yield; + _d + }; + + let mut g = Pin::new(&mut g); + + assert_eq!( + g.as_mut().resume(Dropper(String::from("Hello world!"))), + GeneratorState::Yielded(()) + ); + assert_eq!(DROP.load(Ordering::Acquire), 0); + match g.as_mut().resume(Dropper(String::from("Number Two"))) { + GeneratorState::Complete(dropper) => { + assert_eq!(DROP.load(Ordering::Acquire), 1); + assert_eq!(dropper.0, "Number Two"); + drop(dropper); + assert_eq!(DROP.load(Ordering::Acquire), 2); + } + _ => unreachable!(), + } + + drop(g); + assert_eq!(DROP.load(Ordering::Acquire), 2); +} From 341eaf5f55c2fcf5f58a04cb4184306d0263b4f5 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 4 Feb 2020 13:18:29 +0100 Subject: [PATCH 18/25] Add more tests for generator resume arguments --- src/test/ui/generator/panic-drops-resume.rs | 35 +++++++++++++++++++ .../ui/generator/resume-arg-late-bound.rs | 17 +++++++++ .../ui/generator/resume-arg-late-bound.stderr | 15 ++++++++ 3 files changed, 67 insertions(+) create mode 100644 src/test/ui/generator/panic-drops-resume.rs create mode 100644 src/test/ui/generator/resume-arg-late-bound.rs create mode 100644 src/test/ui/generator/resume-arg-late-bound.stderr diff --git a/src/test/ui/generator/panic-drops-resume.rs b/src/test/ui/generator/panic-drops-resume.rs new file mode 100644 index 0000000000000..4a482d3f6df39 --- /dev/null +++ b/src/test/ui/generator/panic-drops-resume.rs @@ -0,0 +1,35 @@ +//! Tests that panics inside a generator will correctly drop the initial resume argument. + +// run-pass + +#![feature(generators, generator_trait)] + +use std::ops::Generator; +use std::panic::{catch_unwind, AssertUnwindSafe}; +use std::pin::Pin; +use std::sync::atomic::{AtomicUsize, Ordering}; + +static DROP: AtomicUsize = AtomicUsize::new(0); + +struct Dropper {} + +impl Drop for Dropper { + fn drop(&mut self) { + DROP.fetch_add(1, Ordering::SeqCst); + } +} + +fn main() { + let mut gen = |_arg| { + if true { + panic!(); + } + yield (); + }; + let mut gen = Pin::new(&mut gen); + + assert_eq!(DROP.load(Ordering::Acquire), 0); + let res = catch_unwind(AssertUnwindSafe(|| gen.as_mut().resume(Dropper {}))); + assert!(res.is_err()); + assert_eq!(DROP.load(Ordering::Acquire), 1); +} diff --git a/src/test/ui/generator/resume-arg-late-bound.rs b/src/test/ui/generator/resume-arg-late-bound.rs new file mode 100644 index 0000000000000..87b1f1a065bc8 --- /dev/null +++ b/src/test/ui/generator/resume-arg-late-bound.rs @@ -0,0 +1,17 @@ +//! Tests that we cannot produce a generator that accepts a resume argument +//! with any lifetime and then stores it across a `yield`. + +#![feature(generators, generator_trait)] + +use std::ops::Generator; + +fn test(a: impl for<'a> Generator<&'a mut bool>) {} + +fn main() { + let gen = |arg: &mut bool| { + yield (); + *arg = true; + }; + test(gen); + //~^ ERROR type mismatch in function arguments +} diff --git a/src/test/ui/generator/resume-arg-late-bound.stderr b/src/test/ui/generator/resume-arg-late-bound.stderr new file mode 100644 index 0000000000000..7719d5123f466 --- /dev/null +++ b/src/test/ui/generator/resume-arg-late-bound.stderr @@ -0,0 +1,15 @@ +error[E0631]: type mismatch in function arguments + --> $DIR/resume-arg-late-bound.rs:15:10 + | +LL | fn test(a: impl for<'a> Generator<&'a mut bool>) {} + | ---- ------------------------------- required by this bound in `test` +... +LL | test(gen); + | ^^^ + | | + | expected signature of `for<'a> fn(&'a mut bool) -> _` + | found signature of `fn(&mut bool) -> _` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0631`. From cc66d29e43b0fa9e49e054ade0b2b2299203afab Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 4 Feb 2020 13:35:38 +0100 Subject: [PATCH 19/25] Update error message with too many parameters --- src/librustc_ast_lowering/expr.rs | 2 +- src/test/ui/generator/too-many-parameters.rs | 3 ++- src/test/ui/generator/too-many-parameters.stderr | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index 0c4cfa1f6505a..dd3316979f6a2 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -693,7 +693,7 @@ impl<'hir> LoweringContext<'_, 'hir> { self.sess, fn_decl_span, E0628, - "too many parameters for generator (expected 0 or 1 parameters)" + "too many parameters for a generator (expected 0 or 1 parameters)" ) .emit(); } diff --git a/src/test/ui/generator/too-many-parameters.rs b/src/test/ui/generator/too-many-parameters.rs index a0a27d9068241..7a353ea298b26 100644 --- a/src/test/ui/generator/too-many-parameters.rs +++ b/src/test/ui/generator/too-many-parameters.rs @@ -1,7 +1,8 @@ #![feature(generators)] fn main() { - |(), ()| { //~ error: too many parameters for generator + |(), ()| { + //~^ error: too many parameters for a generator yield; }; } diff --git a/src/test/ui/generator/too-many-parameters.stderr b/src/test/ui/generator/too-many-parameters.stderr index 0dbe5f3f6fde9..a297ee43de969 100644 --- a/src/test/ui/generator/too-many-parameters.stderr +++ b/src/test/ui/generator/too-many-parameters.stderr @@ -1,4 +1,4 @@ -error[E0628]: too many parameters for generator (expected 0 or 1 parameters) +error[E0628]: too many parameters for a generator (expected 0 or 1 parameters) --> $DIR/too-many-parameters.rs:4:5 | LL | |(), ()| { From 72776e6b5d355112ad5d320789cc2b3c6232b953 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 4 Feb 2020 13:35:43 +0100 Subject: [PATCH 20/25] Remove obsolete test --- .../generator/no-parameters-on-generators.rs | 10 ----- .../no-parameters-on-generators.stderr | 39 ------------------- 2 files changed, 49 deletions(-) delete mode 100644 src/test/ui/generator/no-parameters-on-generators.rs delete mode 100644 src/test/ui/generator/no-parameters-on-generators.stderr diff --git a/src/test/ui/generator/no-parameters-on-generators.rs b/src/test/ui/generator/no-parameters-on-generators.rs deleted file mode 100644 index cad004895349f..0000000000000 --- a/src/test/ui/generator/no-parameters-on-generators.rs +++ /dev/null @@ -1,10 +0,0 @@ -#![feature(generators)] - -fn main() { - let gen = |start| { - //~^ ERROR type inside generator must be known in this context - yield; - //~^ ERROR type inside generator must be known in this context - //~| ERROR type inside generator must be known in this context - }; -} diff --git a/src/test/ui/generator/no-parameters-on-generators.stderr b/src/test/ui/generator/no-parameters-on-generators.stderr deleted file mode 100644 index f5f83b0476905..0000000000000 --- a/src/test/ui/generator/no-parameters-on-generators.stderr +++ /dev/null @@ -1,39 +0,0 @@ -error[E0698]: type inside generator must be known in this context - --> $DIR/no-parameters-on-generators.rs:4:16 - | -LL | let gen = |start| { - | ^^^^^ cannot infer type - | -note: the type is part of the generator because of this `yield` - --> $DIR/no-parameters-on-generators.rs:6:9 - | -LL | yield; - | ^^^^^ - -error[E0698]: type inside generator must be known in this context - --> $DIR/no-parameters-on-generators.rs:6:9 - | -LL | yield; - | ^^^^^ cannot infer type - | -note: the type is part of the generator because of this `yield` - --> $DIR/no-parameters-on-generators.rs:6:9 - | -LL | yield; - | ^^^^^ - -error[E0698]: type inside generator must be known in this context - --> $DIR/no-parameters-on-generators.rs:6:9 - | -LL | yield; - | ^^^^^ cannot infer type - | -note: the type is part of the generator because of this `yield` - --> $DIR/no-parameters-on-generators.rs:6:9 - | -LL | yield; - | ^^^^^ - -error: aborting due to 3 previous errors - -For more information about this error, try `rustc --explain E0698`. From 895aab22633abe3e8617c3e2aaa246a7b2ff1492 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 4 Feb 2020 13:52:09 +0100 Subject: [PATCH 21/25] Take resume argument from the right generator type I suppose we could also just put `tcx.mk_unit()` here, but this works too --- src/librustc_mir_build/build/mod.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir_build/build/mod.rs b/src/librustc_mir_build/build/mod.rs index 4c8d8e3a0eac8..ccfee2d2bb2ee 100644 --- a/src/librustc_mir_build/build/mod.rs +++ b/src/librustc_mir_build/build/mod.rs @@ -83,12 +83,18 @@ fn mir_build(tcx: TyCtxt<'_>, def_id: DefId) -> BodyAndCache<'_> { abi = Abi::Rust; vec![ArgInfo(liberated_closure_env_ty(tcx, id, body_id), None, None, None)] } - ty::Generator(def_id, substs, _) => { + ty::Generator(def_id, _, _) => { let gen_ty = tcx.body_tables(body_id).node_type(id); - let resume_ty = substs.as_generator().resume_ty(def_id, tcx); // The resume argument may be missing, in that case we need to provide it here. if body.params.is_empty() { + let resume_ty = match gen_ty.kind { + ty::Generator(_, substs, _) => { + substs.as_generator().resume_ty(def_id, tcx) + } + _ => bug!(), + }; + vec![ ArgInfo(gen_ty, None, None, None), ArgInfo(resume_ty, None, None, None), From fb66b9ee3bd46448480560110ff3095e4f7488a0 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Tue, 4 Feb 2020 14:25:55 +0100 Subject: [PATCH 22/25] Don't emit StorageDead for the resume argument --- src/librustc_mir/transform/generator.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 414d91a18be12..267e77ba48128 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -1128,10 +1128,6 @@ fn create_cases<'tcx>( Rvalue::Use(Operand::Move(resume_arg.into())), )), }); - statements.push(Statement { - source_info, - kind: StatementKind::StorageDead(resume_arg), - }); } // Then jump to the real target From 84dd07a2c46ed15bf72f4fb552fcf5d89885b13f Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Thu, 6 Feb 2020 12:27:35 +0100 Subject: [PATCH 23/25] Simplify implicit resume argument --- src/librustc_mir_build/build/mod.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir_build/build/mod.rs b/src/librustc_mir_build/build/mod.rs index ccfee2d2bb2ee..28e6ae9e6db82 100644 --- a/src/librustc_mir_build/build/mod.rs +++ b/src/librustc_mir_build/build/mod.rs @@ -83,21 +83,15 @@ fn mir_build(tcx: TyCtxt<'_>, def_id: DefId) -> BodyAndCache<'_> { abi = Abi::Rust; vec![ArgInfo(liberated_closure_env_ty(tcx, id, body_id), None, None, None)] } - ty::Generator(def_id, _, _) => { + ty::Generator(..) => { let gen_ty = tcx.body_tables(body_id).node_type(id); // The resume argument may be missing, in that case we need to provide it here. + // It will always be `()` in this case. if body.params.is_empty() { - let resume_ty = match gen_ty.kind { - ty::Generator(_, substs, _) => { - substs.as_generator().resume_ty(def_id, tcx) - } - _ => bug!(), - }; - vec![ ArgInfo(gen_ty, None, None, None), - ArgInfo(resume_ty, None, None, None), + ArgInfo(tcx.mk_unit(), None, None, None), ] } else { vec![ArgInfo(gen_ty, None, None, None)] From 732913afcbc37a061dfdc3097cbdc90823386c11 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Thu, 6 Feb 2020 12:34:31 +0100 Subject: [PATCH 24/25] Clarify comment about `_2` living across a yield --- src/librustc_mir/transform/generator.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index 267e77ba48128..a6fc65731780a 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -1187,8 +1187,9 @@ impl<'tcx> MirPass<'tcx> for StateTransform { let new_ret_local = replace_local(RETURN_PLACE, ret_ty, body, tcx); // We also replace the resume argument and insert an `Assign`. - // This is needed because the resume argument might be live across a `yield`, and the - // transform assumes that any local live across a `yield` is assigned to before that. + // This is needed because the resume argument `_2` might be live across a `yield`, in which + // case there is no `Assign` to it that the transform can turn into a store to the generator + // state. After the yield the slot in the generator state would then be uninitialized. let resume_local = Local::new(2); let new_resume_local = replace_local(resume_local, body.local_decls[resume_local].ty, body, tcx); From 9d7b214ac6cb50a1b5454e0ae904a6479b54261c Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Thu, 6 Feb 2020 14:59:51 +0100 Subject: [PATCH 25/25] Ignore panic-drops-resume.rs on wasm/emscripten It does not have unwinding support --- src/test/ui/generator/panic-drops-resume.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/ui/generator/panic-drops-resume.rs b/src/test/ui/generator/panic-drops-resume.rs index 4a482d3f6df39..29f4788b2757f 100644 --- a/src/test/ui/generator/panic-drops-resume.rs +++ b/src/test/ui/generator/panic-drops-resume.rs @@ -1,6 +1,8 @@ //! Tests that panics inside a generator will correctly drop the initial resume argument. // run-pass +// ignore-wasm no unwind support +// ignore-emscripten no unwind support #![feature(generators, generator_trait)]