From ab8aef40b1e3a7151daebe7acea2c8b0117a8c9f Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Thu, 23 Sep 2021 16:36:49 -0400 Subject: [PATCH] Drop fully captured upvars in the same order as the regular drop code Currently, with the new 2021 edition, if a closure captures all of the fields of an upvar, we'll drop those fields in the order they are used within the closure instead of the normal drop order (the definition order of the fields in the type). This changes that so we sort the captured fields by the definition order which causes them to drop in that same order as well. Fixes https://github.com/rust-lang/project-rfc-2229/issues/42 --- compiler/rustc_typeck/src/check/upvar.rs | 42 ++++++++++++++++++- .../preserve_field_drop_order.stderr | 32 +++++++------- ...ld_drop_order2.twenty_twentyone.run.stdout | 2 +- 3 files changed, 58 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 53b99a14f379d..036e103738364 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -602,7 +602,47 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - debug!("For closure={:?}, min_captures={:#?}", closure_def_id, root_var_min_capture_list); + debug!( + "For closure={:?}, min_captures before sorting={:?}", + closure_def_id, root_var_min_capture_list + ); + + // Now that we have the minimized list of captures, sort the captures by field id. + // This causes the closure to capture the upvars in the same order as the fields are + // declared which is also the drop order. Thus, in situations where we capture all the + // fields of some type, the obserable drop order will remain the same as it previously + // was even though we're dropping each capture individually. + // See https://github.com/rust-lang/project-rfc-2229/issues/42 and + // `src/test/ui/closures/2229_closure_analysis/preserve_field_drop_order.rs`. + for (_, captures) in &mut root_var_min_capture_list { + captures.sort_by(|capture1, capture2| { + for (p1, p2) in capture1.place.projections.iter().zip(&capture2.place.projections) { + match (p1.kind, p2.kind) { + // Paths are the same, continue to next loop. + (ProjectionKind::Deref, ProjectionKind::Deref) => {} + (ProjectionKind::Field(i1, _), ProjectionKind::Field(i2, _)) + if i1 == i2 => {} + + // Fields are different, compare them. + (ProjectionKind::Field(i1, _), ProjectionKind::Field(i2, _)) => { + return i1.cmp(&i2); + } + + (l, r) => bug!("ProjectionKinds were different: ({:?}, {:?})", l, r), + } + } + + unreachable!( + "we captured two identical projections: capture1 = {:?}, capture2 = {:?}", + capture1, capture2 + ); + }); + } + + debug!( + "For closure={:?}, min_captures after sorting={:#?}", + closure_def_id, root_var_min_capture_list + ); typeck_results.closure_min_captures.insert(closure_def_id, root_var_min_capture_list); } diff --git a/src/test/ui/closures/2229_closure_analysis/preserve_field_drop_order.stderr b/src/test/ui/closures/2229_closure_analysis/preserve_field_drop_order.stderr index 7fdeab6a74d71..559580ec05995 100644 --- a/src/test/ui/closures/2229_closure_analysis/preserve_field_drop_order.stderr +++ b/src/test/ui/closures/2229_closure_analysis/preserve_field_drop_order.stderr @@ -136,26 +136,26 @@ LL | | LL | | }; | |_____^ | -note: Min Capture a[(1, 0)] -> ImmBorrow - --> $DIR/preserve_field_drop_order.rs:52:26 - | -LL | println!("{:?}", a.1); - | ^^^ note: Min Capture a[(0, 0)] -> ImmBorrow --> $DIR/preserve_field_drop_order.rs:55:26 | LL | println!("{:?}", a.0); | ^^^ -note: Min Capture b[(1, 0)] -> ImmBorrow - --> $DIR/preserve_field_drop_order.rs:59:26 +note: Min Capture a[(1, 0)] -> ImmBorrow + --> $DIR/preserve_field_drop_order.rs:52:26 | -LL | println!("{:?}", b.1); +LL | println!("{:?}", a.1); | ^^^ note: Min Capture b[(0, 0)] -> ImmBorrow --> $DIR/preserve_field_drop_order.rs:62:26 | LL | println!("{:?}", b.0); | ^^^ +note: Min Capture b[(1, 0)] -> ImmBorrow + --> $DIR/preserve_field_drop_order.rs:59:26 + | +LL | println!("{:?}", b.1); + | ^^^ error: First Pass analysis includes: --> $DIR/preserve_field_drop_order.rs:75:5 @@ -202,26 +202,26 @@ LL | | LL | | }; | |_____^ | -note: Min Capture b[(1, 0)] -> ImmBorrow - --> $DIR/preserve_field_drop_order.rs:78:26 - | -LL | println!("{:?}", b.1); - | ^^^ note: Min Capture b[(0, 0)] -> ImmBorrow --> $DIR/preserve_field_drop_order.rs:88:26 | LL | println!("{:?}", b.0); | ^^^ -note: Min Capture a[(1, 0)] -> ImmBorrow - --> $DIR/preserve_field_drop_order.rs:81:26 +note: Min Capture b[(1, 0)] -> ImmBorrow + --> $DIR/preserve_field_drop_order.rs:78:26 | -LL | println!("{:?}", a.1); +LL | println!("{:?}", b.1); | ^^^ note: Min Capture a[(0, 0)] -> ImmBorrow --> $DIR/preserve_field_drop_order.rs:84:26 | LL | println!("{:?}", a.0); | ^^^ +note: Min Capture a[(1, 0)] -> ImmBorrow + --> $DIR/preserve_field_drop_order.rs:81:26 + | +LL | println!("{:?}", a.1); + | ^^^ error: aborting due to 9 previous errors diff --git a/src/test/ui/closures/2229_closure_analysis/preserve_field_drop_order2.twenty_twentyone.run.stdout b/src/test/ui/closures/2229_closure_analysis/preserve_field_drop_order2.twenty_twentyone.run.stdout index 90b1200fd08d1..e393169651817 100644 --- a/src/test/ui/closures/2229_closure_analysis/preserve_field_drop_order2.twenty_twentyone.run.stdout +++ b/src/test/ui/closures/2229_closure_analysis/preserve_field_drop_order2.twenty_twentyone.run.stdout @@ -1,3 +1,3 @@ Dropable("y") Dropable("x") -Dropping y Dropping x +Dropping y