Rollup merge of #89208 - wesleywiser:rfc_2229_droporder, r=nikomatsakis

[rfc 2229] 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 rust-lang/project-rfc-2229#42

r? `@nikomatsakis`
This commit is contained in:
Jubilee 2021-09-24 11:40:14 -07:00 committed by GitHub
commit 7ade6ed48e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 485 additions and 1 deletions

View File

@ -602,7 +602,78 @@ 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) {
// We do not need to look at the `Projection.ty` fields here because at each
// step of the iteration, the projections will either be the same and therefore
// the types must be as well or the current projection will be different and
// we will return the result of comparing the field indexes.
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);
}
// We should have either a pair of `Deref`s or a pair of `Field`s.
// Anything else is a bug.
(
l @ (ProjectionKind::Deref | ProjectionKind::Field(..)),
r @ (ProjectionKind::Deref | ProjectionKind::Field(..)),
) => bug!(
"ProjectionKinds Deref and Field were mismatched: ({:?}, {:?})",
l,
r
),
(
l
@
(ProjectionKind::Index
| ProjectionKind::Subslice
| ProjectionKind::Deref
| ProjectionKind::Field(..)),
r
@
(ProjectionKind::Index
| ProjectionKind::Subslice
| ProjectionKind::Deref
| ProjectionKind::Field(..)),
) => bug!(
"ProjectionKinds Index or Subslice were unexpected: ({:?}, {:?})",
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);
}

View File

@ -0,0 +1,101 @@
// edition:2021
// Tests that in cases where we individually capture all the fields of a type,
// we still drop them in the order they would have been dropped in the 2018 edition.
// NOTE: It is *critical* that the order of the min capture NOTES in the stderr output
// does *not* change!
#![feature(rustc_attrs)]
#[derive(Debug)]
struct HasDrop;
impl Drop for HasDrop {
fn drop(&mut self) {
println!("dropped");
}
}
fn test_one() {
let a = (HasDrop, HasDrop);
let b = (HasDrop, HasDrop);
let c = #[rustc_capture_analysis]
//~^ ERROR: attributes on expressions are experimental
//~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
|| {
//~^ ERROR: Min Capture analysis includes:
//~| ERROR
println!("{:?}", a.0);
//~^ NOTE: Min Capture a[(0, 0)] -> ImmBorrow
//~| NOTE
println!("{:?}", a.1);
//~^ NOTE: Min Capture a[(1, 0)] -> ImmBorrow
//~| NOTE
println!("{:?}", b.0);
//~^ NOTE: Min Capture b[(0, 0)] -> ImmBorrow
//~| NOTE
println!("{:?}", b.1);
//~^ NOTE: Min Capture b[(1, 0)] -> ImmBorrow
//~| NOTE
};
}
fn test_two() {
let a = (HasDrop, HasDrop);
let b = (HasDrop, HasDrop);
let c = #[rustc_capture_analysis]
//~^ ERROR: attributes on expressions are experimental
//~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
|| {
//~^ ERROR: Min Capture analysis includes:
//~| ERROR
println!("{:?}", a.1);
//~^ NOTE: Min Capture a[(1, 0)] -> ImmBorrow
//~| NOTE
println!("{:?}", a.0);
//~^ NOTE: Min Capture a[(0, 0)] -> ImmBorrow
//~| NOTE
println!("{:?}", b.1);
//~^ NOTE: Min Capture b[(1, 0)] -> ImmBorrow
//~| NOTE
println!("{:?}", b.0);
//~^ NOTE: Min Capture b[(0, 0)] -> ImmBorrow
//~| NOTE
};
}
fn test_three() {
let a = (HasDrop, HasDrop);
let b = (HasDrop, HasDrop);
let c = #[rustc_capture_analysis]
//~^ ERROR: attributes on expressions are experimental
//~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
|| {
//~^ ERROR: Min Capture analysis includes:
//~| ERROR
println!("{:?}", b.1);
//~^ NOTE: Min Capture b[(1, 0)] -> ImmBorrow
//~| NOTE
println!("{:?}", a.1);
//~^ NOTE: Min Capture a[(1, 0)] -> ImmBorrow
//~| NOTE
println!("{:?}", a.0);
//~^ NOTE: Min Capture a[(0, 0)] -> ImmBorrow
//~| NOTE
println!("{:?}", b.0);
//~^ NOTE: Min Capture b[(0, 0)] -> ImmBorrow
//~| NOTE
};
}
fn main() {
test_one();
test_two();
test_three();
}

View File

@ -0,0 +1,228 @@
error[E0658]: attributes on expressions are experimental
--> $DIR/preserve_field_drop_order.rs:23:13
|
LL | let c = #[rustc_capture_analysis]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information
= help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable
error[E0658]: attributes on expressions are experimental
--> $DIR/preserve_field_drop_order.rs:49:13
|
LL | let c = #[rustc_capture_analysis]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information
= help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable
error[E0658]: attributes on expressions are experimental
--> $DIR/preserve_field_drop_order.rs:75:13
|
LL | let c = #[rustc_capture_analysis]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information
= help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable
error: First Pass analysis includes:
--> $DIR/preserve_field_drop_order.rs:26:5
|
LL | / || {
LL | |
LL | |
LL | | println!("{:?}", a.0);
... |
LL | |
LL | | };
| |_____^
|
note: Capturing a[(0, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:29:26
|
LL | println!("{:?}", a.0);
| ^^^
note: Capturing a[(1, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:32:26
|
LL | println!("{:?}", a.1);
| ^^^
note: Capturing b[(0, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:36:26
|
LL | println!("{:?}", b.0);
| ^^^
note: Capturing b[(1, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:39:26
|
LL | println!("{:?}", b.1);
| ^^^
error: Min Capture analysis includes:
--> $DIR/preserve_field_drop_order.rs:26:5
|
LL | / || {
LL | |
LL | |
LL | | println!("{:?}", a.0);
... |
LL | |
LL | | };
| |_____^
|
note: Min Capture a[(0, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:29:26
|
LL | println!("{:?}", a.0);
| ^^^
note: Min Capture a[(1, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:32:26
|
LL | println!("{:?}", a.1);
| ^^^
note: Min Capture b[(0, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:36:26
|
LL | println!("{:?}", b.0);
| ^^^
note: Min Capture b[(1, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:39:26
|
LL | println!("{:?}", b.1);
| ^^^
error: First Pass analysis includes:
--> $DIR/preserve_field_drop_order.rs:52:5
|
LL | / || {
LL | |
LL | |
LL | | println!("{:?}", a.1);
... |
LL | |
LL | | };
| |_____^
|
note: Capturing a[(1, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:55:26
|
LL | println!("{:?}", a.1);
| ^^^
note: Capturing a[(0, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:58:26
|
LL | println!("{:?}", a.0);
| ^^^
note: Capturing b[(1, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:62:26
|
LL | println!("{:?}", b.1);
| ^^^
note: Capturing b[(0, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:65:26
|
LL | println!("{:?}", b.0);
| ^^^
error: Min Capture analysis includes:
--> $DIR/preserve_field_drop_order.rs:52:5
|
LL | / || {
LL | |
LL | |
LL | | println!("{:?}", a.1);
... |
LL | |
LL | | };
| |_____^
|
note: Min Capture a[(0, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:58:26
|
LL | println!("{:?}", a.0);
| ^^^
note: Min Capture a[(1, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:55:26
|
LL | println!("{:?}", a.1);
| ^^^
note: Min Capture b[(0, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:65:26
|
LL | println!("{:?}", b.0);
| ^^^
note: Min Capture b[(1, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:62:26
|
LL | println!("{:?}", b.1);
| ^^^
error: First Pass analysis includes:
--> $DIR/preserve_field_drop_order.rs:78:5
|
LL | / || {
LL | |
LL | |
LL | | println!("{:?}", b.1);
... |
LL | |
LL | | };
| |_____^
|
note: Capturing b[(1, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:81:26
|
LL | println!("{:?}", b.1);
| ^^^
note: Capturing a[(1, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:84:26
|
LL | println!("{:?}", a.1);
| ^^^
note: Capturing a[(0, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:87:26
|
LL | println!("{:?}", a.0);
| ^^^
note: Capturing b[(0, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:91:26
|
LL | println!("{:?}", b.0);
| ^^^
error: Min Capture analysis includes:
--> $DIR/preserve_field_drop_order.rs:78:5
|
LL | / || {
LL | |
LL | |
LL | | println!("{:?}", b.1);
... |
LL | |
LL | | };
| |_____^
|
note: Min Capture b[(0, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:91:26
|
LL | println!("{:?}", b.0);
| ^^^
note: Min Capture b[(1, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:81:26
|
LL | println!("{:?}", b.1);
| ^^^
note: Min Capture a[(0, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:87:26
|
LL | println!("{:?}", a.0);
| ^^^
note: Min Capture a[(1, 0)] -> ImmBorrow
--> $DIR/preserve_field_drop_order.rs:84:26
|
LL | println!("{:?}", a.1);
| ^^^
error: aborting due to 9 previous errors
For more information about this error, try `rustc --explain E0658`.

View File

@ -0,0 +1,58 @@
// run-pass
// check-run-results
// revisions: twenty_eighteen twenty_twentyone
// [twenty_eighteen]compile-flags: --edition 2018
// [twenty_twentyone]compile-flags: --edition 2021
#[derive(Debug)]
struct Dropable(&'static str);
impl Drop for Dropable {
fn drop(&mut self) {
println!("Dropping {}", self.0)
}
}
#[derive(Debug)]
struct A {
x: Dropable,
y: Dropable,
}
#[derive(Debug)]
struct B {
c: A,
d: A,
}
#[derive(Debug)]
struct R<'a> {
c: &'a A,
d: &'a A,
}
fn main() {
let a = A { x: Dropable("x"), y: Dropable("y") };
let c = move || println!("{:?} {:?}", a.y, a.x);
c();
let b = B {
c: A { x: Dropable("b.c.x"), y: Dropable("b.c.y") },
d: A { x: Dropable("b.d.x"), y: Dropable("b.d.y") },
};
let d = move || println!("{:?} {:?} {:?} {:?}", b.d.y, b.d.x, b.c.y, b.c.x);
d();
let r = R {
c: &A { x: Dropable("r.c.x"), y: Dropable("r.c.y") },
d: &A { x: Dropable("r.d.x"), y: Dropable("r.d.y") },
};
let e = move || println!("{:?} {:?} {:?} {:?}", r.d.y, r.d.x, r.c.y, r.c.x);
e();
}

View File

@ -0,0 +1,13 @@
Dropable("y") Dropable("x")
Dropable("b.d.y") Dropable("b.d.x") Dropable("b.c.y") Dropable("b.c.x")
Dropable("r.d.y") Dropable("r.d.x") Dropable("r.c.y") Dropable("r.c.x")
Dropping r.d.x
Dropping r.d.y
Dropping r.c.x
Dropping r.c.y
Dropping b.c.x
Dropping b.c.y
Dropping b.d.x
Dropping b.d.y
Dropping x
Dropping y

View File

@ -0,0 +1,13 @@
Dropable("y") Dropable("x")
Dropable("b.d.y") Dropable("b.d.x") Dropable("b.c.y") Dropable("b.c.x")
Dropable("r.d.y") Dropable("r.d.x") Dropable("r.c.y") Dropable("r.c.x")
Dropping r.d.x
Dropping r.d.y
Dropping r.c.x
Dropping r.c.y
Dropping b.c.x
Dropping b.c.y
Dropping b.d.x
Dropping b.d.y
Dropping x
Dropping y