Skip to content

Commit

Permalink
Drop fully captured upvars in the same order as the regular drop code
Browse files Browse the repository at this point in the history
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 rust-lang/project-rfc-2229#42
  • Loading branch information
wesleywiser committed Sep 23, 2021
1 parent 7a3e450 commit ab8aef4
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 18 deletions.
42 changes: 41 additions & 1 deletion compiler/rustc_typeck/src/check/upvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
Dropable("y") Dropable("x")
Dropping y
Dropping x
Dropping y

0 comments on commit ab8aef4

Please sign in to comment.