dropck_outlives check generator witness needs_drop

Author: lcnr

compiler/rustc_middle/src/ty/util.rs

@@ -1097,8 +1097,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);
+ let param_env = tcx.erase_regions(param_env);
+ 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))
  }
@@ -1287,7 +1289,6 @@ pub fn needs_drop_components<'tcx>(
  | ty::FnDef(..)
  | ty::FnPtr(_)
  | ty::Char
- | ty::CoroutineWitness(..)
  | ty::RawPtr(_)
  | ty::Ref(..)
  | ty::Str => Ok(SmallVec::new()),
@@ -1327,7 +1328,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]),
  }
 }
 

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, param_env) {
+ constraints.outlives.extend(args.upvar_tys().iter().map(ty::GenericArg::from));
+ constraints.outlives.push(args.resume_ty().into());
+ }
  }
 
  ty::Adt(def, args) => {

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);

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 generator witness.
+ reveal_coroutine_witnesses: bool,
  query_ty: Ty<'tcx>,
  seen_tys: FxHashSet<Ty<'tcx>>,
  /// 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 generator witness, we
+ // have to conservatively assume that every region captured by the
+ // generator has to be live when dropped. This results in a lot of
+ // undesirable borrowck errors. During borrowck, we call `needs_drop`
+ // for the generator 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(_) => {

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

tests/ui/coroutine/issue-110929-coroutine-conflict-error-ice.rs

@@ -1,11 +1,11 @@
 // edition:2021
+// check-pass
 #![feature(coroutines)]
 
 fn main() {
  let x = &mut ();
  || {
  let _c = || yield *&mut *x;
  || _ = &mut *x;
- //~^ cannot borrow `*x` as mutable more than once at a time
  };
 }

tests/ui/coroutine/issue-110929-coroutine-conflict-error-ice.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