diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index 9430b3ee54484..a251518d14575 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -1107,8 +1107,10 @@ impl<'tcx> Ty<'tcx> { // This doesn't depend on regions, so try to minimize distinct // query keys used. // If normalization fails, we just use `query_ty`. - let query_ty = - tcx.try_normalize_erasing_regions(param_env, query_ty).unwrap_or(query_ty); + debug_assert!(!param_env.has_infer()); + let query_ty = tcx + .try_normalize_erasing_regions(param_env, query_ty) + .unwrap_or_else(|_| tcx.erase_regions(query_ty)); tcx.needs_drop_raw(param_env.and(query_ty)) } @@ -1297,7 +1299,6 @@ pub fn needs_drop_components<'tcx>( | ty::FnDef(..) | ty::FnPtr(_) | ty::Char - | ty::CoroutineWitness(..) | ty::RawPtr(_) | ty::Ref(..) | ty::Str => Ok(SmallVec::new()), @@ -1337,7 +1338,8 @@ pub fn needs_drop_components<'tcx>( | ty::Placeholder(..) | ty::Infer(_) | ty::Closure(..) - | ty::Coroutine(..) => Ok(smallvec![ty]), + | ty::Coroutine(..) + | ty::CoroutineWitness(..) => Ok(smallvec![ty]), } } diff --git a/compiler/rustc_trait_selection/src/traits/query/dropck_outlives.rs b/compiler/rustc_trait_selection/src/traits/query/dropck_outlives.rs index 0783eec6fe8c0..c5fcedcac9905 100644 --- a/compiler/rustc_trait_selection/src/traits/query/dropck_outlives.rs +++ b/compiler/rustc_trait_selection/src/traits/query/dropck_outlives.rs @@ -133,7 +133,7 @@ pub fn compute_dropck_outlives_inner<'tcx>( result.overflows.len(), ty_stack.len() ); - dtorck_constraint_for_ty_inner(tcx, DUMMY_SP, for_ty, depth, ty, &mut constraints)?; + dtorck_constraint_for_ty_inner(tcx, param_env, DUMMY_SP, depth, ty, &mut constraints)?; // "outlives" represent types/regions that may be touched // by a destructor. @@ -185,16 +185,15 @@ pub fn compute_dropck_outlives_inner<'tcx>( /// Returns a set of constraints that needs to be satisfied in /// order for `ty` to be valid for destruction. +#[instrument(level = "debug", skip(tcx, param_env, span, constraints))] pub fn dtorck_constraint_for_ty_inner<'tcx>( tcx: TyCtxt<'tcx>, + param_env: ty::ParamEnv<'tcx>, span: Span, - for_ty: Ty<'tcx>, depth: usize, ty: Ty<'tcx>, constraints: &mut DropckConstraint<'tcx>, ) -> Result<(), NoSolution> { - debug!("dtorck_constraint_for_ty_inner({:?}, {:?}, {:?}, {:?})", span, for_ty, depth, ty); - if !tcx.recursion_limit().value_within_limit(depth) { constraints.overflows.push(ty); return Ok(()); @@ -224,13 +223,13 @@ pub fn dtorck_constraint_for_ty_inner<'tcx>( ty::Array(ety, _) | ty::Slice(ety) => { // single-element containers, behave like their element rustc_data_structures::stack::ensure_sufficient_stack(|| { - dtorck_constraint_for_ty_inner(tcx, span, for_ty, depth + 1, *ety, constraints) + dtorck_constraint_for_ty_inner(tcx, param_env, span, depth + 1, *ety, constraints) })?; } ty::Tuple(tys) => rustc_data_structures::stack::ensure_sufficient_stack(|| { for ty in tys.iter() { - dtorck_constraint_for_ty_inner(tcx, span, for_ty, depth + 1, ty, constraints)?; + dtorck_constraint_for_ty_inner(tcx, param_env, span, depth + 1, ty, constraints)?; } Ok::<_, NoSolution>(()) })?, @@ -249,7 +248,14 @@ pub fn dtorck_constraint_for_ty_inner<'tcx>( rustc_data_structures::stack::ensure_sufficient_stack(|| { for ty in args.as_closure().upvar_tys() { - dtorck_constraint_for_ty_inner(tcx, span, for_ty, depth + 1, ty, constraints)?; + dtorck_constraint_for_ty_inner( + tcx, + param_env, + span, + depth + 1, + ty, + constraints, + )?; } Ok::<_, NoSolution>(()) })? @@ -278,8 +284,8 @@ pub fn dtorck_constraint_for_ty_inner<'tcx>( // only take place through references with lifetimes // derived from lifetimes attached to the upvars and resume // argument, and we *do* incorporate those here. - - if !args.as_coroutine().is_valid() { + let args = args.as_coroutine(); + if !args.is_valid() { // By the time this code runs, all type variables ought to // be fully resolved. tcx.sess.delay_span_bug( @@ -289,10 +295,13 @@ pub fn dtorck_constraint_for_ty_inner<'tcx>( return Err(NoSolution); } - constraints - .outlives - .extend(args.as_coroutine().upvar_tys().iter().map(ty::GenericArg::from)); - constraints.outlives.push(args.as_coroutine().resume_ty().into()); + // While we conservatively assume that all coroutines require drop + // to avoid query cycles during MIR building, we can check the actual + // witness during borrowck to avoid unnecessary liveness constraints. + if args.witness().needs_drop(tcx, tcx.erase_regions(param_env)) { + constraints.outlives.extend(args.upvar_tys().iter().map(ty::GenericArg::from)); + constraints.outlives.push(args.resume_ty().into()); + } } ty::Adt(def, args) => { diff --git a/compiler/rustc_traits/src/dropck_outlives.rs b/compiler/rustc_traits/src/dropck_outlives.rs index 074764f0c4e2e..f40c3614e1c00 100644 --- a/compiler/rustc_traits/src/dropck_outlives.rs +++ b/compiler/rustc_traits/src/dropck_outlives.rs @@ -34,6 +34,7 @@ pub(crate) fn adt_dtorck_constraint( ) -> Result<&DropckConstraint<'_>, NoSolution> { let def = tcx.adt_def(def_id); let span = tcx.def_span(def_id); + let param_env = tcx.param_env(def_id); debug!("dtorck_constraint: {:?}", def); if def.is_manually_drop() { @@ -55,7 +56,7 @@ pub(crate) fn adt_dtorck_constraint( let mut result = DropckConstraint::empty(); for field in def.all_fields() { let fty = tcx.type_of(field.did).instantiate_identity(); - dtorck_constraint_for_ty_inner(tcx, span, fty, 0, fty, &mut result)?; + dtorck_constraint_for_ty_inner(tcx, param_env, span, 0, fty, &mut result)?; } result.outlives.extend(tcx.destructor_constraints(def)); dedup_dtorck_constraint(&mut result); diff --git a/compiler/rustc_ty_utils/src/needs_drop.rs b/compiler/rustc_ty_utils/src/needs_drop.rs index 0812730474123..67ccfe7e78abb 100644 --- a/compiler/rustc_ty_utils/src/needs_drop.rs +++ b/compiler/rustc_ty_utils/src/needs_drop.rs @@ -66,6 +66,9 @@ fn has_significant_drop_raw<'tcx>( struct NeedsDropTypes<'tcx, F> { tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, + // Whether to reveal coroutine witnesses, this is set + // to `false` unless we compute `needs_drop` for a coroutine witness. + reveal_coroutine_witnesses: bool, query_ty: Ty<'tcx>, seen_tys: FxHashSet>, /// A stack of types left to process, and the recursion depth when we @@ -89,6 +92,7 @@ impl<'tcx, F> NeedsDropTypes<'tcx, F> { Self { tcx, param_env, + reveal_coroutine_witnesses: false, seen_tys, query_ty: ty, unchecked_tys: vec![(ty, 0)], @@ -133,8 +137,31 @@ where // The information required to determine whether a coroutine has drop is // computed on MIR, while this very method is used to build MIR. // To avoid cycles, we consider that coroutines always require drop. - ty::Coroutine(..) => { - return Some(Err(AlwaysRequiresDrop)); + // + // HACK: Because we erase regions contained in the coroutine witness, we + // have to conservatively assume that every region captured by the + // coroutine has to be live when dropped. This results in a lot of + // undesirable borrowck errors. During borrowck, we call `needs_drop` + // for the coroutine witness and check whether any of the contained types + // need to be dropped, and only require the captured types to be live + // if they do. + ty::Coroutine(_, args, _) => { + if self.reveal_coroutine_witnesses { + queue_type(self, args.as_coroutine().witness()); + } else { + return Some(Err(AlwaysRequiresDrop)); + } + } + ty::CoroutineWitness(def_id, args) => { + if let Some(witness) = tcx.mir_coroutine_witnesses(def_id) { + self.reveal_coroutine_witnesses = true; + for field_ty in &witness.field_tys { + queue_type( + self, + EarlyBinder::bind(field_ty.ty).instantiate(tcx, args), + ); + } + } } _ if component.is_copy_modulo_regions(tcx, self.param_env) => (), @@ -191,7 +218,6 @@ where | ty::FnPtr(..) | ty::Tuple(_) | ty::Bound(..) - | ty::CoroutineWitness(..) | ty::Never | ty::Infer(_) | ty::Error(_) => { diff --git a/compiler/rustc_type_ir/src/ty_kind.rs b/compiler/rustc_type_ir/src/ty_kind.rs index dceb5ba0560b1..2138c27334134 100644 --- a/compiler/rustc_type_ir/src/ty_kind.rs +++ b/compiler/rustc_type_ir/src/ty_kind.rs @@ -218,7 +218,6 @@ pub enum TyKind { /// the type of the coroutine, we convert them to higher ranked /// lifetimes bound by the witness itself. /// - /// This variant is only using when `drop_tracking_mir` is set. /// This contains the `DefId` and the `GenericArgsRef` of the coroutine. /// The actual witness types are computed on MIR by the `mir_coroutine_witnesses` query. /// diff --git a/tests/ui/coroutine/borrowing.stderr b/tests/ui/coroutine/borrowing.stderr index 192ffaaa26b3e..acd4cdafdfdf7 100644 --- a/tests/ui/coroutine/borrowing.stderr +++ b/tests/ui/coroutine/borrowing.stderr @@ -1,24 +1,16 @@ error[E0597]: `a` does not live long enough --> $DIR/borrowing.rs:9:33 | +LL | let _b = { + | -- borrow later stored here +LL | let a = 3; LL | Pin::new(&mut || yield &a).resume(()) - | ----------^ - | | | - | | borrowed value does not live long enough + | -- ^ borrowed value does not live long enough + | | | value captured here by coroutine - | a temporary with access to the borrow is created here ... LL | LL | }; - | -- ... and the borrow might be used here, when that temporary is dropped and runs the destructor for coroutine - | | - | `a` dropped here while still borrowed - | - = note: the temporary is part of an expression at the end of a block; - consider forcing this temporary to be dropped sooner, before the block's local variables are dropped -help: for example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block - | -LL | let x = Pin::new(&mut || yield &a).resume(()); x - | +++++++ +++ + | - `a` dropped here while still borrowed error[E0597]: `a` does not live long enough --> $DIR/borrowing.rs:16:20 diff --git a/tests/ui/coroutine/issue-110929-coroutine-conflict-error-ice.rs b/tests/ui/coroutine/issue-110929-coroutine-conflict-error-ice.rs index feaaa71ea9c4e..ad39b71b0eb02 100644 --- a/tests/ui/coroutine/issue-110929-coroutine-conflict-error-ice.rs +++ b/tests/ui/coroutine/issue-110929-coroutine-conflict-error-ice.rs @@ -1,4 +1,5 @@ // edition:2021 +// check-pass #![feature(coroutines)] fn main() { @@ -6,6 +7,5 @@ fn main() { || { let _c = || yield *&mut *x; || _ = &mut *x; - //~^ cannot borrow `*x` as mutable more than once at a time }; } diff --git a/tests/ui/coroutine/issue-110929-coroutine-conflict-error-ice.stderr b/tests/ui/coroutine/issue-110929-coroutine-conflict-error-ice.stderr deleted file mode 100644 index 77da6c4cdc941..0000000000000 --- a/tests/ui/coroutine/issue-110929-coroutine-conflict-error-ice.stderr +++ /dev/null @@ -1,18 +0,0 @@ -error[E0499]: cannot borrow `*x` as mutable more than once at a time - --> $DIR/issue-110929-coroutine-conflict-error-ice.rs:8:9 - | -LL | let _c = || yield *&mut *x; - | -- -- first borrow occurs due to use of `*x` in coroutine - | | - | first mutable borrow occurs here -LL | || _ = &mut *x; - | ^^ -- second borrow occurs due to use of `*x` in closure - | | - | second mutable borrow occurs here -LL | -LL | }; - | - first borrow might be used here, when `_c` is dropped and runs the destructor for coroutine - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0499`. diff --git a/tests/ui/coroutine/retain-resume-ref.stderr b/tests/ui/coroutine/retain-resume-ref.stderr index 983443bbfeb37..e33310d12d9ef 100644 --- a/tests/ui/coroutine/retain-resume-ref.stderr +++ b/tests/ui/coroutine/retain-resume-ref.stderr @@ -4,10 +4,9 @@ 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 -LL | -LL | } - | - first borrow might be used here, when `gen` is dropped and runs the destructor for coroutine + | ------ ^^^^^^^^^^ second mutable borrow occurs here + | | + | first borrow later used by call error: aborting due to previous error diff --git a/tests/ui/dropck/coroutine-liveness-1.rs b/tests/ui/dropck/coroutine-liveness-1.rs new file mode 100644 index 0000000000000..aea4d15ad90e2 --- /dev/null +++ b/tests/ui/dropck/coroutine-liveness-1.rs @@ -0,0 +1,18 @@ +// check-pass +// edition: 2021 + +// regression test for #116242. +use std::future; + +fn main() { + let mut recv = future::ready(()); + let _combined_fut = async { + let _ = || read(&mut recv); + }; + + drop(recv); +} + +fn read(_: &mut F) -> F::Output { + todo!() +} diff --git a/tests/ui/dropck/coroutine-liveness-2.rs b/tests/ui/dropck/coroutine-liveness-2.rs new file mode 100644 index 0000000000000..416a073c6b90a --- /dev/null +++ b/tests/ui/dropck/coroutine-liveness-2.rs @@ -0,0 +1,23 @@ +// check-pass +// edition: 2021 + +// regression test found while working on #117134. +use std::future; + +fn main() { + let mut recv = future::ready(()); + let _combined_fut = async { + let _ = || read(&mut recv); + }; + + let _uwu = (String::new(), _combined_fut); + // Dropping a coroutine as part of a more complex + // types should not add unnecessary liveness + // constraints. + + drop(recv); +} + +fn read(_: &mut F) -> F::Output { + todo!() +}