From ec41fda58bd7cf35f3506bba97d2a98a64b09498 Mon Sep 17 00:00:00 2001 From: David Haig Date: Mon, 25 Nov 2019 12:58:40 +0000 Subject: [PATCH] Squash --- src/librustc/mir/interpret/error.rs | 6 +++ src/librustc/mir/mod.rs | 14 ++++-- src/librustc/mir/visit.rs | 3 +- src/librustc_mir/build/mod.rs | 15 +++--- src/librustc_mir/build/scope.rs | 40 ++++++++------- src/librustc_mir/interpret/terminator.rs | 2 + src/librustc_mir/shim.rs | 4 ++ src/librustc_mir/transform/const_prop.rs | 1 + src/librustc_mir/transform/generator.rs | 23 +++++++-- src/librustc_mir/transform/promote_consts.rs | 1 + ...-65419-async-fn-resume-after-completion.rs | 44 ++++++++++++++++ ...issue-65419-async-fn-resume-after-panic.rs | 50 +++++++++++++++++++ ...65419-generator-resume-after-completion.rs | 22 ++++++++ 13 files changed, 192 insertions(+), 33 deletions(-) create mode 100644 src/test/ui/issues/issue-65419/issue-65419-async-fn-resume-after-completion.rs create mode 100644 src/test/ui/issues/issue-65419/issue-65419-async-fn-resume-after-panic.rs create mode 100644 src/test/ui/issues/issue-65419/issue-65419-generator-resume-after-completion.rs diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index 7fb669314eb..93543bc3616 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -266,6 +266,8 @@ pub enum PanicInfo { RemainderByZero, GeneratorResumedAfterReturn, GeneratorResumedAfterPanic, + AsyncResumedAfterReturn, + AsyncResumedAfterPanic, } /// Type for MIR `Assert` terminator error messages. @@ -304,6 +306,10 @@ impl PanicInfo { "generator resumed after completion", GeneratorResumedAfterPanic => "generator resumed after panicking", + AsyncResumedAfterReturn => + "`async fn` resumed after completion", + AsyncResumedAfterPanic => + "`async fn` resumed after panic", Panic { .. } | BoundsCheck { .. } => bug!("Unexpected PanicInfo"), } diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index c745dd9444c..d6048a1b394 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -6,7 +6,7 @@ use crate::hir::def::{CtorKind, Namespace}; use crate::hir::def_id::DefId; -use crate::hir; +use crate::hir::GeneratorKind; use crate::mir::interpret::{GlobalAlloc, PanicInfo, Scalar}; use crate::mir::visit::MirVisitable; use crate::ty::adjustment::PointerCast; @@ -117,6 +117,10 @@ pub struct Body<'tcx> { /// The layout of a generator. Produced by the state transformation. pub generator_layout: Option>, + /// If this is a generator then record the type of source expression that caused this generator + /// to be created. + pub generator_kind: Option, + /// Declarations of locals. /// /// The first local is the return value pointer, followed by `arg_count` @@ -170,6 +174,7 @@ impl<'tcx> Body<'tcx> { var_debug_info: Vec>, span: Span, control_flow_destroyed: Vec<(Span, String)>, + generator_kind : Option, ) -> Self { // We need `arg_count` locals, and one for the return place. assert!( @@ -187,6 +192,7 @@ impl<'tcx> Body<'tcx> { yield_ty: None, generator_drop: None, generator_layout: None, + generator_kind, local_decls, user_type_annotations, arg_count, @@ -2975,7 +2981,8 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> { index: index.fold_with(folder), }, Panic { .. } | Overflow(_) | OverflowNeg | DivisionByZero | RemainderByZero | - GeneratorResumedAfterReturn | GeneratorResumedAfterPanic => + GeneratorResumedAfterReturn | GeneratorResumedAfterPanic | + AsyncResumedAfterReturn | AsyncResumedAfterPanic => msg.clone(), }; Assert { cond: cond.fold_with(folder), expected, msg, target, cleanup } @@ -3021,7 +3028,8 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> { len.visit_with(visitor) || index.visit_with(visitor), Panic { .. } | Overflow(_) | OverflowNeg | DivisionByZero | RemainderByZero | - GeneratorResumedAfterReturn | GeneratorResumedAfterPanic => + GeneratorResumedAfterReturn | GeneratorResumedAfterPanic | + AsyncResumedAfterReturn | AsyncResumedAfterPanic => false } } else { diff --git a/src/librustc/mir/visit.rs b/src/librustc/mir/visit.rs index 464d4c74366..cce1ef9eb8b 100644 --- a/src/librustc/mir/visit.rs +++ b/src/librustc/mir/visit.rs @@ -517,7 +517,8 @@ macro_rules! make_mir_visitor { self.visit_operand(index, location); } Panic { .. } | Overflow(_) | OverflowNeg | DivisionByZero | RemainderByZero | - GeneratorResumedAfterReturn | GeneratorResumedAfterPanic => { + GeneratorResumedAfterReturn | GeneratorResumedAfterPanic | + AsyncResumedAfterReturn | AsyncResumedAfterPanic => { // Nothing to visit } } diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 180f2cc089f..91ddd5a5623 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -5,7 +5,7 @@ use crate::hair::{LintLevel, BindingMode, PatKind}; use crate::transform::MirSource; use crate::util as mir_util; use rustc::hir; -use rustc::hir::Node; +use rustc::hir::{Node, GeneratorKind}; use rustc::hir::def_id::DefId; use rustc::middle::lang_items; use rustc::middle::region; @@ -279,7 +279,7 @@ struct Builder<'a, 'tcx> { fn_span: Span, arg_count: usize, - is_generator: bool, + generator_kind: Option, /// The current set of scopes, updated as we traverse; /// see the `scope` module for more details. @@ -570,7 +570,7 @@ where safety, return_ty, return_ty_span, - body.generator_kind.is_some()); + body.generator_kind); let call_site_scope = region::Scope { id: body.value.hir_id.local_id, @@ -647,7 +647,7 @@ fn construct_const<'a, 'tcx>( Safety::Safe, const_ty, const_ty_span, - false, + None, ); let mut block = START_BLOCK; @@ -678,7 +678,7 @@ fn construct_error<'a, 'tcx>( let owner_id = hir.tcx().hir().body_owner(body_id); let span = hir.tcx().hir().span(owner_id); let ty = hir.tcx().types.err; - let mut builder = Builder::new(hir, span, 0, Safety::Safe, ty, span, false); + let mut builder = Builder::new(hir, span, 0, Safety::Safe, ty, span, None); let source_info = builder.source_info(span); builder.cfg.terminate(START_BLOCK, source_info, TerminatorKind::Unreachable); builder.finish() @@ -691,7 +691,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { safety: Safety, return_ty: Ty<'tcx>, return_span: Span, - is_generator: bool) + generator_kind: Option) -> Builder<'a, 'tcx> { let lint_level = LintLevel::Explicit(hir.root_lint_level); let mut builder = Builder { @@ -699,7 +699,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { cfg: CFG { basic_blocks: IndexVec::new() }, fn_span: span, arg_count, - is_generator, + generator_kind, scopes: Default::default(), block_context: BlockContext::new(), source_scopes: IndexVec::new(), @@ -748,6 +748,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.var_debug_info, self.fn_span, self.hir.control_flow_destroyed(), + self.generator_kind ) } diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index 1b3d8641f20..bb25b285269 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -91,6 +91,7 @@ use syntax_pos::{DUMMY_SP, Span}; use rustc_data_structures::fx::FxHashMap; use std::collections::hash_map::Entry; use std::mem; +use rustc::hir::GeneratorKind; #[derive(Debug)] struct Scope { @@ -219,7 +220,12 @@ impl Scope { /// `storage_only` controls whether to invalidate only drop paths that run `StorageDead`. /// `this_scope_only` controls whether to invalidate only drop paths that refer to the current /// top-of-scope (as opposed to dependent scopes). - fn invalidate_cache(&mut self, storage_only: bool, is_generator: bool, this_scope_only: bool) { + fn invalidate_cache( + &mut self, + storage_only: bool, + generator_kind: Option, + this_scope_only: bool + ) { // FIXME: maybe do shared caching of `cached_exits` etc. to handle functions // with lots of `try!`? @@ -229,7 +235,7 @@ impl Scope { // the current generator drop and unwind refer to top-of-scope self.cached_generator_drop = None; - let ignore_unwinds = storage_only && !is_generator; + let ignore_unwinds = storage_only && generator_kind.is_none(); if !ignore_unwinds { self.cached_unwind.invalidate(); } @@ -481,7 +487,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { unpack!(block = build_scope_drops( &mut self.cfg, - self.is_generator, + self.generator_kind, &scope, block, unwind_to, @@ -574,7 +580,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { unpack!(block = build_scope_drops( &mut self.cfg, - self.is_generator, + self.generator_kind, scope, block, unwind_to, @@ -625,7 +631,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { unpack!(block = build_scope_drops( &mut self.cfg, - self.is_generator, + self.generator_kind, scope, block, unwind_to, @@ -809,7 +815,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // invalidating caches of each scope visited. This way bare minimum of the // caches gets invalidated. i.e., if a new drop is added into the middle scope, the // cache of outer scope stays intact. - scope.invalidate_cache(!needs_drop, self.is_generator, this_scope); + scope.invalidate_cache(!needs_drop, self.generator_kind, this_scope); if this_scope { let region_scope_span = region_scope.span(self.hir.tcx(), &self.hir.region_scope_tree); @@ -958,7 +964,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } - top_scope.invalidate_cache(true, self.is_generator, true); + top_scope.invalidate_cache(true, self.generator_kind, true); } else { bug!("Expected as_local_operand to produce a temporary"); } @@ -1016,7 +1022,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { for scope in self.scopes.top_scopes(first_uncached) { target = build_diverge_scope(&mut self.cfg, scope.region_scope_span, - scope, target, generator_drop, self.is_generator); + scope, target, generator_drop, self.generator_kind); } target @@ -1079,14 +1085,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { assert_eq!(top_scope.region_scope, region_scope); top_scope.drops.clear(); - top_scope.invalidate_cache(false, self.is_generator, true); + top_scope.invalidate_cache(false, self.generator_kind, true); } } /// Builds drops for pop_scope and exit_scope. fn build_scope_drops<'tcx>( cfg: &mut CFG<'tcx>, - is_generator: bool, + generator_kind: Option, scope: &Scope, mut block: BasicBlock, last_unwind_to: BasicBlock, @@ -1130,7 +1136,7 @@ fn build_scope_drops<'tcx>( continue; } - let unwind_to = get_unwind_to(scope, is_generator, drop_idx, generator_drop) + let unwind_to = get_unwind_to(scope, generator_kind, drop_idx, generator_drop) .unwrap_or(last_unwind_to); let next = cfg.start_new_block(); @@ -1156,19 +1162,19 @@ fn build_scope_drops<'tcx>( fn get_unwind_to( scope: &Scope, - is_generator: bool, + generator_kind: Option, unwind_from: usize, generator_drop: bool, ) -> Option { for drop_idx in (0..unwind_from).rev() { let drop_data = &scope.drops[drop_idx]; - match (is_generator, &drop_data.kind) { - (true, DropKind::Storage) => { + match (generator_kind, &drop_data.kind) { + (Some(_), DropKind::Storage) => { return Some(drop_data.cached_block.get(generator_drop).unwrap_or_else(|| { span_bug!(drop_data.span, "cached block not present for {:?}", drop_data) })); } - (false, DropKind::Value) => { + (None, DropKind::Value) => { return Some(drop_data.cached_block.get(generator_drop).unwrap_or_else(|| { span_bug!(drop_data.span, "cached block not present for {:?}", drop_data) })); @@ -1184,7 +1190,7 @@ fn build_diverge_scope<'tcx>(cfg: &mut CFG<'tcx>, scope: &mut Scope, mut target: BasicBlock, generator_drop: bool, - is_generator: bool) + generator_kind: Option) -> BasicBlock { // Build up the drops in **reverse** order. The end result will @@ -1224,7 +1230,7 @@ fn build_diverge_scope<'tcx>(cfg: &mut CFG<'tcx>, // match the behavior of clang, but on inspection eddyb says // this is not what clang does. match drop_data.kind { - DropKind::Storage if is_generator => { + DropKind::Storage if generator_kind.is_some() => { storage_deads.push(Statement { source_info: source_info(drop_data.span), kind: StatementKind::StorageDead(drop_data.local) diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index daa0a5e1bc4..59b721b73b5 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -144,6 +144,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { RemainderByZero => err_panic!(RemainderByZero), GeneratorResumedAfterReturn => err_panic!(GeneratorResumedAfterReturn), GeneratorResumedAfterPanic => err_panic!(GeneratorResumedAfterPanic), + AsyncResumedAfterReturn => err_panic!(AsyncResumedAfterReturn), + AsyncResumedAfterPanic => err_panic!(AsyncResumedAfterPanic), Panic { .. } => bug!("`Panic` variant cannot occur in MIR"), } .into()); diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index b5cb6a92816..d623e7870e1 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -208,6 +208,7 @@ fn build_drop_shim<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, ty: Option>) vec![], span, vec![], + None, ); if let Some(..) = ty { @@ -374,6 +375,7 @@ impl CloneShimBuilder<'tcx> { vec![], self.span, vec![], + None, ) } @@ -834,6 +836,7 @@ fn build_call_shim<'tcx>( vec![], span, vec![], + None, ); if let Abi::RustCall = sig.abi { body.spread_arg = Some(Local::new(sig.inputs().len())); @@ -920,6 +923,7 @@ pub fn build_adt_ctor(tcx: TyCtxt<'_>, ctor_id: DefId) -> &Body<'_> { vec![], span, vec![], + None, ); crate::util::dump_mir( diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 8de16308e83..d2740cf3771 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -91,6 +91,7 @@ impl<'tcx> MirPass<'tcx> for ConstProp { Default::default(), tcx.def_span(source.def_id()), Default::default(), + body.generator_kind, ); // FIXME(oli-obk, eddyb) Optimize locals (or even local paths) to hold diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index a904c6a3ada..c6fe877ec59 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -50,7 +50,7 @@ //! Otherwise it drops all the values in scope at the last suspension point. use rustc::hir; -use rustc::hir::def_id::DefId; +use rustc::hir::{def_id::DefId, GeneratorKind}; use rustc::mir::*; use rustc::mir::visit::{PlaceContext, Visitor, MutVisitor}; use rustc::ty::{self, TyCtxt, AdtDef, Ty}; @@ -1058,14 +1058,27 @@ fn create_generator_resume_function<'tcx>( use rustc::mir::interpret::PanicInfo::{ GeneratorResumedAfterPanic, GeneratorResumedAfterReturn, + AsyncResumedAfterReturn, + AsyncResumedAfterPanic, }; // Jump to the entry point on the unresumed cases.insert(0, (UNRESUMED, BasicBlock::new(0))); - // Panic when resumed on the returned state - cases.insert(1, (RETURNED, insert_panic_block(tcx, body, GeneratorResumedAfterReturn))); - // Panic when resumed on the poisoned state - cases.insert(2, (POISONED, insert_panic_block(tcx, body, GeneratorResumedAfterPanic))); + + // Panic when resumed on the returned or poisoned state + match body.generator_kind { + Some(GeneratorKind::Async(_)) => { + cases.insert(1, (RETURNED, insert_panic_block(tcx, body, AsyncResumedAfterReturn))); + cases.insert(2, (POISONED, insert_panic_block(tcx, body, AsyncResumedAfterPanic))); + }, + Some(GeneratorKind::Gen) => { + cases.insert(1, (RETURNED, insert_panic_block(tcx, body, GeneratorResumedAfterReturn))); + cases.insert(2, (POISONED, insert_panic_block(tcx, body, GeneratorResumedAfterPanic))); + }, + None => { + // N/A because we would never create a resume function if there was no generator_kind + } + }; insert_switch(body, cases, &transform, TerminatorKind::Unreachable); diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index 2c3aec103a5..309b863bc0c 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -1104,6 +1104,7 @@ pub fn promote_candidates<'tcx>( vec![], body.span, vec![], + body.generator_kind, ), tcx, source: body, diff --git a/src/test/ui/issues/issue-65419/issue-65419-async-fn-resume-after-completion.rs b/src/test/ui/issues/issue-65419/issue-65419-async-fn-resume-after-completion.rs new file mode 100644 index 00000000000..54df1a5664e --- /dev/null +++ b/src/test/ui/issues/issue-65419/issue-65419-async-fn-resume-after-completion.rs @@ -0,0 +1,44 @@ +// issue 65419 - Attempting to run an async fn after completion mentions generators when it should +// be talking about `async fn`s instead. + +// run-fail +// error-pattern: thread 'main' panicked at '`async fn` resumed after completion' +// compile-flags: --edition 2018 + +#![feature(generators, generator_trait)] + +async fn foo() { +} + +fn main() { + let mut future = Box::pin(foo()); + executor::block_on(future.as_mut()); + executor::block_on(future.as_mut()); +} + +mod executor { + use core::{ + future::Future, + pin::Pin, + task::{Context, Poll, RawWaker, RawWakerVTable, Waker}, + }; + + pub fn block_on(mut future: F) -> F::Output { + let mut future = unsafe { Pin::new_unchecked(&mut future) }; + + static VTABLE: RawWakerVTable = RawWakerVTable::new( + |_| unimplemented!("clone"), + |_| unimplemented!("wake"), + |_| unimplemented!("wake_by_ref"), + |_| (), + ); + let waker = unsafe { Waker::from_raw(RawWaker::new(core::ptr::null(), &VTABLE)) }; + let mut context = Context::from_waker(&waker); + + loop { + if let Poll::Ready(val) = future.as_mut().poll(&mut context) { + break val; + } + } + } +} diff --git a/src/test/ui/issues/issue-65419/issue-65419-async-fn-resume-after-panic.rs b/src/test/ui/issues/issue-65419/issue-65419-async-fn-resume-after-panic.rs new file mode 100644 index 00000000000..f5d7af2c87e --- /dev/null +++ b/src/test/ui/issues/issue-65419/issue-65419-async-fn-resume-after-panic.rs @@ -0,0 +1,50 @@ +// issue 65419 - Attempting to run an async fn after completion mentions generators when it should +// be talking about `async fn`s instead. Should also test what happens when it panics. + +// run-fail +// error-pattern: thread 'main' panicked at '`async fn` resumed after panic' +// compile-flags: --edition 2018 + +#![feature(generators, generator_trait)] + +use std::panic; + +async fn foo() { + panic!(); +} + +fn main() { + let mut future = Box::pin(foo()); + panic::catch_unwind(panic::AssertUnwindSafe(|| { + executor::block_on(future.as_mut()); + })); + + executor::block_on(future.as_mut()); +} + +mod executor { + use core::{ + future::Future, + pin::Pin, + task::{Context, Poll, RawWaker, RawWakerVTable, Waker}, + }; + + pub fn block_on(mut future: F) -> F::Output { + let mut future = unsafe { Pin::new_unchecked(&mut future) }; + + static VTABLE: RawWakerVTable = RawWakerVTable::new( + |_| unimplemented!("clone"), + |_| unimplemented!("wake"), + |_| unimplemented!("wake_by_ref"), + |_| (), + ); + let waker = unsafe { Waker::from_raw(RawWaker::new(core::ptr::null(), &VTABLE)) }; + let mut context = Context::from_waker(&waker); + + loop { + if let Poll::Ready(val) = future.as_mut().poll(&mut context) { + break val; + } + } + } +} diff --git a/src/test/ui/issues/issue-65419/issue-65419-generator-resume-after-completion.rs b/src/test/ui/issues/issue-65419/issue-65419-generator-resume-after-completion.rs new file mode 100644 index 00000000000..456c1d38e8c --- /dev/null +++ b/src/test/ui/issues/issue-65419/issue-65419-generator-resume-after-completion.rs @@ -0,0 +1,22 @@ +// issue 65419 - Attempting to run an `async fn` after completion mentions generators when it should +// be talking about `async fn`s instead. Regression test added to make sure generators still +// panic when resumed after completion. + +// run-fail +// error-pattern:generator resumed after completion + +#![feature(generators, generator_trait)] + +use std::{ + ops::Generator, + pin::Pin, +}; + +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. +}