Skip to content

Commit

Permalink
Do not create move paths that do not need dropping.
Browse files Browse the repository at this point in the history
  • Loading branch information
@cjgillot
cjgillot committed Oct 1, 2023
1 parent 3194181 commit 525722a
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 73 deletions.
30 changes: 16 additions & 14 deletions compiler/rustc_mir_transform/src/elaborate_drops.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops {

let def_id = body.source.def_id();
let param_env = tcx.param_env_reveal_all_normalized(def_id);
let move_data = MoveData::gather_moves(&body, tcx, param_env, |_| true);
let move_data =
MoveData::gather_moves(&body, tcx, param_env, |ty| ty.needs_drop(tcx, param_env));
let elaborate_patch = {
let env = MoveDataParamEnv { move_data, param_env };

Expand Down Expand Up @@ -340,7 +341,16 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {
let terminator = data.terminator();

match terminator.kind {
TerminatorKind::Drop { place, target, unwind, replace } => {
TerminatorKind::Drop { place, target, unwind, replace: _ } => {
if !place
.ty(&self.body.local_decls, self.tcx)
.ty
.needs_drop(self.tcx, self.env.param_env)
{
self.patch.patch_terminator(bb, TerminatorKind::Goto { target });
continue;
}

self.init_data.seek_before(loc);
match self.move_data().rev_lookup.find(place.as_ref()) {
LookupResult::Exact(path) => {
Expand Down Expand Up @@ -373,18 +383,10 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {
bb,
)
}
LookupResult::Parent(..) => {
if !replace {
self.tcx.sess.delay_span_bug(
terminator.source_info.span,
format!("drop of untracked value {bb:?}"),
);
}
// A drop and replace behind a pointer/array/whatever.
// The borrow checker requires that these locations are initialized before the assignment,
// so we just leave an unconditional drop.
assert!(!data.is_cleanup);
}
// A drop and replace behind a pointer/array/whatever.
// The borrow checker requires that these locations are initialized before the assignment,
// so we just leave an unconditional drop.
LookupResult::Parent(..) => {}
}
}
_ => continue,
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_mir_transform/src/remove_uninit_drops.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ pub struct RemoveUninitDrops;
impl<'tcx> MirPass<'tcx> for RemoveUninitDrops {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let param_env = tcx.param_env(body.source.def_id());
let move_data = MoveData::gather_moves(&body, tcx, param_env, |_| true);
let move_data =
MoveData::gather_moves(&body, tcx, param_env, |ty| ty.needs_drop(tcx, param_env));

let mdpe = MoveDataParamEnv { move_data, param_env };
let mut maybe_inits = MaybeInitializedPlaces::new(tcx, body, &mdpe)
Expand Down
24 changes: 8 additions & 16 deletions tests/mir-opt/inline/unsized_argument.caller.Inline.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,18 @@
let mut _0: ();
let _2: ();
let mut _3: std::boxed::Box<[i32]>;
let mut _4: &mut std::boxed::Box<[i32]>;
let mut _5: ();
let mut _6: &mut std::boxed::Box<[i32]>;
let mut _7: ();
let mut _8: &mut std::boxed::Box<[i32]>;
let mut _9: ();
let mut _10: *const [i32];
let mut _4: *const [i32];

bb0: {
StorageLive(_2);
StorageLive(_3);
_3 = move _1;
_10 = (((_3.0: std::ptr::Unique<[i32]>).0: std::ptr::NonNull<[i32]>).0: *const [i32]);
_2 = callee(move (*_10)) -> [return: bb3, unwind: bb4];
_4 = (((_3.0: std::ptr::Unique<[i32]>).0: std::ptr::NonNull<[i32]>).0: *const [i32]);
_2 = callee(move (*_4)) -> [return: bb1, unwind: bb3];
}

bb1 (cleanup): {
resume;
bb1: {
drop(_3) -> [return: bb2, unwind: bb4];
}

bb2: {
Expand All @@ -33,14 +27,12 @@
return;
}

bb3: {
_4 = &mut _3;
_5 = <Box<[i32]> as Drop>::drop(move _4) -> [return: bb2, unwind: bb1];
bb3 (cleanup): {
drop(_3) -> [return: bb4, unwind terminate(cleanup)];
}

bb4 (cleanup): {
_8 = &mut _3;
_9 = <Box<[i32]> as Drop>::drop(move _8) -> [return: bb1, unwind terminate(cleanup)];
resume;
}
}

40 changes: 19 additions & 21 deletions tests/mir-opt/issue_41888.main.ElaborateDrops.panic-abort.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@
let mut _5: isize;
+ let mut _7: bool;
+ let mut _8: bool;
+ let mut _9: bool;
+ let mut _9: isize;
+ let mut _10: isize;
+ let mut _11: isize;
scope 1 {
debug e => _1;
scope 2 {
Expand All @@ -24,7 +23,6 @@
bb0: {
+ _7 = const false;
+ _8 = const false;
+ _9 = const false;
StorageLive(_1);
StorageLive(_2);
_2 = cond() -> [return: bb1, unwind: bb11];
Expand All @@ -47,7 +45,6 @@
bb3: {
+ _7 = const true;
+ _8 = const true;
+ _9 = const true;
_1 = move _3;
- drop(_3) -> [return: bb5, unwind: bb11];
+ goto -> bb5;
Expand All @@ -56,7 +53,6 @@
bb4 (cleanup): {
+ _7 = const true;
+ _8 = const true;
+ _9 = const true;
_1 = move _3;
- drop(_3) -> [return: bb11, unwind terminate(cleanup)];
+ goto -> bb11;
Expand All @@ -70,7 +66,6 @@

bb6: {
StorageLive(_6);
+ _9 = const false;
_6 = move ((_1 as F).0: K);
_0 = const ();
StorageDead(_6);
Expand All @@ -90,13 +85,12 @@
bb9: {
StorageDead(_2);
- drop(_1) -> [return: bb10, unwind: bb12];
+ goto -> bb18;
+ goto -> bb19;
}

bb10: {
+ _7 = const false;
+ _8 = const false;
+ _9 = const false;
StorageDead(_1);
return;
}
Expand All @@ -116,33 +110,37 @@
+ }
+
+ bb14 (cleanup): {
+ goto -> bb12;
+ drop(((_1 as F).0: K)) -> [return: bb12, unwind terminate(cleanup)];
+ }
+
+ bb15: {
+ drop(_1) -> [return: bb13, unwind: bb12];
+ bb15 (cleanup): {
+ switchInt(_7) -> [0: bb12, otherwise: bb14];
+ }
+
+ bb16 (cleanup): {
+ drop(_1) -> [return: bb12, unwind terminate(cleanup)];
+ bb16: {
+ drop(_1) -> [return: bb13, unwind: bb12];
+ }
+
+ bb17: {
+ _10 = discriminant(_1);
+ switchInt(move _10) -> [0: bb13, otherwise: bb15];
+ bb17 (cleanup): {
+ drop(_1) -> [return: bb12, unwind terminate(cleanup)];
+ }
+
+ bb18: {
+ switchInt(_7) -> [0: bb13, otherwise: bb17];
+ _9 = discriminant(_1);
+ switchInt(move _9) -> [0: bb13, otherwise: bb16];
+ }
+
+ bb19 (cleanup): {
+ _11 = discriminant(_1);
+ switchInt(move _11) -> [0: bb14, otherwise: bb16];
+ bb19: {
+ switchInt(_7) -> [0: bb13, otherwise: bb18];
+ }
+
+ bb20 (cleanup): {
+ switchInt(_7) -> [0: bb12, otherwise: bb19];
+ _10 = discriminant(_1);
+ switchInt(move _10) -> [0: bb15, otherwise: bb17];
+ }
+
+ bb21 (cleanup): {
+ switchInt(_7) -> [0: bb12, otherwise: bb20];
}
}

40 changes: 19 additions & 21 deletions tests/mir-opt/issue_41888.main.ElaborateDrops.panic-unwind.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@
let mut _5: isize;
+ let mut _7: bool;
+ let mut _8: bool;
+ let mut _9: bool;
+ let mut _9: isize;
+ let mut _10: isize;
+ let mut _11: isize;
scope 1 {
debug e => _1;
scope 2 {
Expand All @@ -24,7 +23,6 @@
bb0: {
+ _7 = const false;
+ _8 = const false;
+ _9 = const false;
StorageLive(_1);
StorageLive(_2);
_2 = cond() -> [return: bb1, unwind: bb11];
Expand All @@ -47,7 +45,6 @@
bb3: {
+ _7 = const true;
+ _8 = const true;
+ _9 = const true;
_1 = move _3;
- drop(_3) -> [return: bb5, unwind: bb11];
+ goto -> bb5;
Expand All @@ -56,7 +53,6 @@
bb4 (cleanup): {
+ _7 = const true;
+ _8 = const true;
+ _9 = const true;
_1 = move _3;
- drop(_3) -> [return: bb11, unwind terminate(cleanup)];
+ goto -> bb11;
Expand All @@ -70,7 +66,6 @@

bb6: {
StorageLive(_6);
+ _9 = const false;
_6 = move ((_1 as F).0: K);
_0 = const ();
StorageDead(_6);
Expand All @@ -90,13 +85,12 @@
bb9: {
StorageDead(_2);
- drop(_1) -> [return: bb10, unwind continue];
+ goto -> bb18;
+ goto -> bb19;
}

bb10: {
+ _7 = const false;
+ _8 = const false;
+ _9 = const false;
StorageDead(_1);
return;
}
Expand All @@ -116,33 +110,37 @@
+ }
+
+ bb14 (cleanup): {
+ goto -> bb12;
+ drop(((_1 as F).0: K)) -> [return: bb12, unwind terminate(cleanup)];
+ }
+
+ bb15: {
+ drop(_1) -> [return: bb13, unwind: bb12];
+ bb15 (cleanup): {
+ switchInt(_7) -> [0: bb12, otherwise: bb14];
+ }
+
+ bb16 (cleanup): {
+ drop(_1) -> [return: bb12, unwind terminate(cleanup)];
+ bb16: {
+ drop(_1) -> [return: bb13, unwind: bb12];
+ }
+
+ bb17: {
+ _10 = discriminant(_1);
+ switchInt(move _10) -> [0: bb13, otherwise: bb15];
+ bb17 (cleanup): {
+ drop(_1) -> [return: bb12, unwind terminate(cleanup)];
+ }
+
+ bb18: {
+ switchInt(_7) -> [0: bb13, otherwise: bb17];
+ _9 = discriminant(_1);
+ switchInt(move _9) -> [0: bb13, otherwise: bb16];
+ }
+
+ bb19 (cleanup): {
+ _11 = discriminant(_1);
+ switchInt(move _11) -> [0: bb14, otherwise: bb16];
+ bb19: {
+ switchInt(_7) -> [0: bb13, otherwise: bb18];
+ }
+
+ bb20 (cleanup): {
+ switchInt(_7) -> [0: bb12, otherwise: bb19];
+ _10 = discriminant(_1);
+ switchInt(move _10) -> [0: bb15, otherwise: bb17];
+ }
+
+ bb21 (cleanup): {
+ switchInt(_7) -> [0: bb12, otherwise: bb20];
}
}

0 comments on commit 525722a

Please sign in to comment.