@@ -319,39 +319,65 @@ pub fn dtorck_constraint_for_ty_inner<'tcx>(
319319 }
320320
321321 ty:: Coroutine ( def_id, args) => {
322- // rust-lang/rust#49918: types can be constructed, stored
323- // in the interior, and sit idle when coroutine yields
324- // (and is subsequently dropped).
322+ // rust-lang/rust#49918: Locals can be stored across await points in the coroutine,
323+ // called interior/witness types. Since we do not compute these witnesses until after
324+ // building MIR, we consider all coroutines to unconditionally require a drop during
325+ // MIR building. However, considering the coroutine to unconditionally require a drop
326+ // here may unnecessarily require its upvars' regions to be live when they don't need
327+ // to be, leading to borrowck errors: <https://github.com/rust-lang/rust/issues/116242>.
325328 //
326- // It would be nice to descend into interior of a
327- // coroutine to determine what effects dropping it might
328- // have (by looking at any drop effects associated with
329- // its interior).
329+ // Here, we implement a more precise approximation for the coroutine's dtorck constraint
330+ // by considering whether any of the interior types needs drop. Note that this is still
331+ // an approximation because the coroutine interior has its regions erased, so we must add
332+ // *all* of the upvars to live types set if we find that *any* interior type needs drop.
333+ // This is because any of the regions captured in the upvars may be stored in the interior,
334+ // which then has its regions replaced by a binder (conceptually erasing the regions),
335+ // so there's no way to enforce that the precise region in the interior type is live
336+ // since we've lost that information by this point.
330337 //
331- // However, the interior's representation uses things like
332- // CoroutineWitness that explicitly assume they are not
333- // traversed in such a manner. So instead, we will
334- // simplify things for now by treating all coroutines as
335- // if they were like trait objects, where its upvars must
336- // all be alive for the coroutine's (potential)
337- // destructor.
338+ // Note also that this check requires that the coroutine's upvars are use-live, since
339+ // a region from a type that does not have a destructor that was captured in an upvar
340+ // may flow into an interior type with a destructor. This is stronger than requiring
341+ // the upvars are drop-live.
338342 //
339- // In particular, skipping over `_interior` is safe
340- // because any side-effects from dropping `_interior` can
341- // only take place through references with lifetimes
342- // derived from lifetimes attached to the upvars and resume
343- // argument, and we *do* incorporate those here .
343+ // For example, if we capture two upvar references `&'1 (), &'2 ()` and have some type
344+ // in the interior, `for<'r> { NeedsDrop<'r> }`, we have no way to tell whether the
345+ // region `'r` came from the `'1` or `'2` region, so we require both are live. This
346+ // could even be unnecessary if `'r` was actually a `'static` region or some region
347+ // local to the coroutine! That's why it's an approximation .
344348 let args = args. as_coroutine ( ) ;
345349
346- // While we conservatively assume that all coroutines require drop
347- // to avoid query cycles during MIR building, we can check the actual
348- // witness during borrowck to avoid unnecessary liveness constraints .
350+ // Note that we don't care about whether the resume type has any drops since this is
351+ // redundant; there is no storage for the resume type, so if it is actually stored
352+ // in the interior, we'll already detect the need for a drop by checking the interior .
349353 let typing_env = tcx. erase_regions ( typing_env) ;
350- if tcx. mir_coroutine_witnesses ( def_id) . is_some_and ( |witness| {
354+ let needs_drop = tcx. mir_coroutine_witnesses ( def_id) . is_some_and ( |witness| {
351355 witness. field_tys . iter ( ) . any ( |field| field. ty . needs_drop ( tcx, typing_env) )
352- } ) {
356+ } ) ;
357+ if needs_drop {
358+ // Pushing types directly to `constraints.outlives` is equivalent
359+ // to requiring them to be use-live, since if we were instead to
360+ // recurse on them like we do below, we only end up collecting the
361+ // types that are relevant for drop-liveness.
353362 constraints. outlives . extend ( args. upvar_tys ( ) . iter ( ) . map ( ty:: GenericArg :: from) ) ;
354363 constraints. outlives . push ( args. resume_ty ( ) . into ( ) ) ;
364+ } else {
365+ // Even if a witness type doesn't need a drop, we still require that
366+ // the upvars are drop-live. This is only needed if we aren't already
367+ // counting *all* of the upvars as use-live above, since use-liveness
368+ // is a *stronger requirement* than drop-liveness. Recursing here
369+ // unconditionally would just be collecting duplicated types for no
370+ // reason.
371+ for ty in args. upvar_tys ( ) {
372+ dtorck_constraint_for_ty_inner (
373+ tcx,
374+ typing_env,
375+ span,
376+ depth + 1 ,
377+ ty,
378+ constraints,
379+ ) ;
380+ }
355381 }
356382 }
357383
0 commit comments