Skip to content

Commit dac354f

Browse files
pnkfelixpnkfelix
committed
Revert "Simplify unscheduling of drops after moves"
This reverts commit b766abc.
1 parent bed69c6 commit dac354f

File tree

5 files changed

+80
-31
lines changed

5 files changed

+80
-31
lines changed

‎compiler/rustc_mir_build/src/build/scope.rs‎

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ that contains only loops and breakable blocks. It tracks where a `break`,
8383

8484
use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder, CFG};
8585
use crate::thir::{Expr, ExprRef, LintLevel};
86-
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
86+
use rustc_data_structures::fx::FxHashMap;
8787
use rustc_index::vec::IndexVec;
8888
use rustc_middle::middle::region;
8989
use rustc_middle::mir::*;
@@ -120,6 +120,8 @@ struct Scope {
120120
/// end of the vector (top of the stack) first.
121121
drops: Vec<DropData>,
122122

123+
moved_locals: Vec<Local>,
124+
123125
/// The drop index that will drop everything in and below this scope on an
124126
/// unwind path.
125127
cached_unwind_block: Option<DropIdx>,
@@ -403,6 +405,7 @@ impl<'tcx> Scopes<'tcx> {
403405
region_scope: region_scope.0,
404406
region_scope_span: region_scope.1.span,
405407
drops: vec![],
408+
moved_locals: vec![],
406409
cached_unwind_block: None,
407410
cached_generator_drop_block: None,
408411
});
@@ -890,19 +893,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
890893
assert_eq!(scope.region_scope, local_scope, "local scope is not the topmost scope!",);
891894

892895
// look for moves of a local variable, like `MOVE(_X)`
893-
let locals_moved = operands
894-
.iter()
895-
.filter_map(|operand| match operand {
896-
Operand::Copy(_) | Operand::Constant(_) => None,
897-
Operand::Move(place) => place.as_local(),
898-
})
899-
.collect::<FxHashSet<_>>();
896+
let locals_moved = operands.iter().flat_map(|operand| match operand {
897+
Operand::Copy(_) | Operand::Constant(_) => None,
898+
Operand::Move(place) => place.as_local(),
899+
});
900900

901-
// Remove the drops for the moved operands.
902-
scope
903-
.drops
904-
.retain(|drop| drop.kind == DropKind::Storage || !locals_moved.contains(&drop.local));
905-
scope.invalidate_cache();
901+
for local in locals_moved {
902+
// check if we have a Drop for this operand and -- if so
903+
// -- add it to the list of moved operands. Note that this
904+
// local might not have been an operand created for this
905+
// call, it could come from other places too.
906+
if scope.drops.iter().any(|drop| drop.local == local && drop.kind == DropKind::Value) {
907+
scope.moved_locals.push(local);
908+
}
909+
}
906910
}
907911

908912
// Other
@@ -1147,6 +1151,14 @@ fn build_scope_drops<'tcx>(
11471151
debug_assert_eq!(unwind_drops.drops[unwind_to].0.kind, drop_data.kind);
11481152
unwind_to = unwind_drops.drops[unwind_to].1;
11491153

1154+
// If the operand has been moved, and we are not on an unwind
1155+
// path, then don't generate the drop. (We only take this into
1156+
// account for non-unwind paths so as not to disturb the
1157+
// caching mechanism.)
1158+
if scope.moved_locals.iter().any(|&o| o == local) {
1159+
continue;
1160+
}
1161+
11501162
unwind_drops.add_entry(block, unwind_to);
11511163

11521164
let next = cfg.start_new_block();

‎src/test/mir-opt/box_expr.main.ElaborateDrops.before.mir‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ fn main() -> () {
1414
StorageLive(_1); // scope 0 at $DIR/box_expr.rs:7:9: 7:10
1515
StorageLive(_2); // scope 0 at $DIR/box_expr.rs:7:13: 7:25
1616
_2 = Box(S); // scope 0 at $DIR/box_expr.rs:7:13: 7:25
17-
(*_2) = S::new() -> [return: bb1, unwind: bb6]; // scope 0 at $DIR/box_expr.rs:7:17: 7:25
17+
(*_2) = S::new() -> [return: bb1, unwind: bb7]; // scope 0 at $DIR/box_expr.rs:7:17: 7:25
1818
// mir::Constant
1919
// + span: $DIR/box_expr.rs:7:17: 7:23
2020
// + literal: Const { ty: fn() -> S {S::new}, val: Value(Scalar(<ZST>)) }
@@ -49,14 +49,18 @@ fn main() -> () {
4949
}
5050

5151
bb5 (cleanup): {
52-
drop(_1) -> bb7; // scope 0 at $DIR/box_expr.rs:9:1: 9:2
52+
drop(_4) -> bb6; // scope 1 at $DIR/box_expr.rs:8:11: 8:12
5353
}
5454

5555
bb6 (cleanup): {
56-
drop(_2) -> bb7; // scope 0 at $DIR/box_expr.rs:7:24: 7:25
56+
drop(_1) -> bb8; // scope 0 at $DIR/box_expr.rs:9:1: 9:2
5757
}
5858

5959
bb7 (cleanup): {
60+
drop(_2) -> bb8; // scope 0 at $DIR/box_expr.rs:7:24: 7:25
61+
}
62+
63+
bb8 (cleanup): {
6064
resume; // scope 0 at $DIR/box_expr.rs:6:1: 9:2
6165
}
6266
}

‎src/test/mir-opt/issue_41110.main.ElaborateDrops.after.mir‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,44 +6,65 @@ fn main() -> () {
66
let mut _2: S; // in scope 0 at $DIR/issue-41110.rs:8:13: 8:14
77
let mut _3: S; // in scope 0 at $DIR/issue-41110.rs:8:21: 8:27
88
let mut _4: S; // in scope 0 at $DIR/issue-41110.rs:8:21: 8:22
9+
let mut _5: bool; // in scope 0 at $DIR/issue-41110.rs:8:27: 8:28
910
scope 1 {
1011
debug x => _1; // in scope 1 at $DIR/issue-41110.rs:8:9: 8:10
1112
}
1213

1314
bb0: {
15+
_5 = const false; // scope 0 at $DIR/issue-41110.rs:8:9: 8:10
1416
StorageLive(_1); // scope 0 at $DIR/issue-41110.rs:8:9: 8:10
1517
StorageLive(_2); // scope 0 at $DIR/issue-41110.rs:8:13: 8:14
18+
_5 = const true; // scope 0 at $DIR/issue-41110.rs:8:13: 8:14
1619
_2 = S; // scope 0 at $DIR/issue-41110.rs:8:13: 8:14
1720
StorageLive(_3); // scope 0 at $DIR/issue-41110.rs:8:21: 8:27
1821
StorageLive(_4); // scope 0 at $DIR/issue-41110.rs:8:21: 8:22
1922
_4 = S; // scope 0 at $DIR/issue-41110.rs:8:21: 8:22
20-
_3 = S::id(move _4) -> [return: bb1, unwind: bb3]; // scope 0 at $DIR/issue-41110.rs:8:21: 8:27
23+
_3 = S::id(move _4) -> [return: bb1, unwind: bb4]; // scope 0 at $DIR/issue-41110.rs:8:21: 8:27
2124
// mir::Constant
2225
// + span: $DIR/issue-41110.rs:8:23: 8:25
2326
// + literal: Const { ty: fn(S) -> S {S::id}, val: Value(Scalar(<ZST>)) }
2427
}
2528

2629
bb1: {
2730
StorageDead(_4); // scope 0 at $DIR/issue-41110.rs:8:26: 8:27
28-
_1 = S::other(move _2, move _3) -> bb2; // scope 0 at $DIR/issue-41110.rs:8:13: 8:28
31+
_5 = const false; // scope 0 at $DIR/issue-41110.rs:8:13: 8:28
32+
_1 = S::other(move _2, move _3) -> [return: bb2, unwind: bb3]; // scope 0 at $DIR/issue-41110.rs:8:13: 8:28
2933
// mir::Constant
3034
// + span: $DIR/issue-41110.rs:8:15: 8:20
3135
// + literal: Const { ty: fn(S, S) {S::other}, val: Value(Scalar(<ZST>)) }
3236
}
3337

3438
bb2: {
3539
StorageDead(_3); // scope 0 at $DIR/issue-41110.rs:8:27: 8:28
40+
_5 = const false; // scope 0 at $DIR/issue-41110.rs:8:27: 8:28
3641
StorageDead(_2); // scope 0 at $DIR/issue-41110.rs:8:27: 8:28
3742
_0 = const (); // scope 0 at $DIR/issue-41110.rs:7:11: 9:2
3843
StorageDead(_1); // scope 0 at $DIR/issue-41110.rs:9:1: 9:2
3944
return; // scope 0 at $DIR/issue-41110.rs:9:2: 9:2
4045
}
4146

4247
bb3 (cleanup): {
43-
drop(_2) -> bb4; // scope 0 at $DIR/issue-41110.rs:8:27: 8:28
48+
goto -> bb5; // scope 0 at $DIR/issue-41110.rs:8:27: 8:28
4449
}
4550

4651
bb4 (cleanup): {
52+
goto -> bb5; // scope 0 at $DIR/issue-41110.rs:8:26: 8:27
53+
}
54+
55+
bb5 (cleanup): {
56+
goto -> bb8; // scope 0 at $DIR/issue-41110.rs:8:27: 8:28
57+
}
58+
59+
bb6 (cleanup): {
4760
resume; // scope 0 at $DIR/issue-41110.rs:7:1: 9:2
4861
}
62+
63+
bb7 (cleanup): {
64+
drop(_2) -> bb6; // scope 0 at $DIR/issue-41110.rs:8:27: 8:28
65+
}
66+
67+
bb8 (cleanup): {
68+
switchInt(_5) -> [false: bb6, otherwise: bb7]; // scope 0 at $DIR/issue-41110.rs:8:27: 8:28
69+
}
4970
}

‎src/test/mir-opt/issue_41110.test.ElaborateDrops.after.mir‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ fn test() -> () {
3737
StorageLive(_5); // scope 2 at $DIR/issue-41110.rs:18:9: 18:10
3838
_6 = const false; // scope 2 at $DIR/issue-41110.rs:18:9: 18:10
3939
_5 = move _1; // scope 2 at $DIR/issue-41110.rs:18:9: 18:10
40-
goto -> bb11; // scope 2 at $DIR/issue-41110.rs:18:5: 18:6
40+
goto -> bb12; // scope 2 at $DIR/issue-41110.rs:18:5: 18:6
4141
}
4242

4343
bb2: {
@@ -47,7 +47,7 @@ fn test() -> () {
4747
bb3: {
4848
StorageDead(_5); // scope 2 at $DIR/issue-41110.rs:18:9: 18:10
4949
_0 = const (); // scope 0 at $DIR/issue-41110.rs:14:15: 19:2
50-
drop(_2) -> [return: bb4, unwind: bb8]; // scope 1 at $DIR/issue-41110.rs:19:1: 19:2
50+
drop(_2) -> [return: bb4, unwind: bb9]; // scope 1 at $DIR/issue-41110.rs:19:1: 19:2
5151
}
5252

5353
bb4: {
@@ -62,36 +62,40 @@ fn test() -> () {
6262
}
6363

6464
bb6 (cleanup): {
65-
goto -> bb7; // scope 2 at $DIR/issue-41110.rs:18:9: 18:10
65+
goto -> bb8; // scope 2 at $DIR/issue-41110.rs:18:9: 18:10
6666
}
6767

6868
bb7 (cleanup): {
69-
goto -> bb8; // scope 1 at $DIR/issue-41110.rs:19:1: 19:2
69+
goto -> bb8; // scope 2 at $DIR/issue-41110.rs:17:11: 17:12
7070
}
7171

7272
bb8 (cleanup): {
73-
goto -> bb13; // scope 0 at $DIR/issue-41110.rs:19:1: 19:2
73+
goto -> bb9; // scope 1 at $DIR/issue-41110.rs:19:1: 19:2
7474
}
7575

7676
bb9 (cleanup): {
77-
resume; // scope 0 at $DIR/issue-41110.rs:14:1: 19:2
77+
goto -> bb14; // scope 0 at $DIR/issue-41110.rs:19:1: 19:2
7878
}
7979

8080
bb10 (cleanup): {
81+
resume; // scope 0 at $DIR/issue-41110.rs:14:1: 19:2
82+
}
83+
84+
bb11 (cleanup): {
8185
_2 = move _5; // scope 2 at $DIR/issue-41110.rs:18:5: 18:6
8286
goto -> bb6; // scope 2 at $DIR/issue-41110.rs:18:5: 18:6
8387
}
8488

85-
bb11: {
89+
bb12: {
8690
_2 = move _5; // scope 2 at $DIR/issue-41110.rs:18:5: 18:6
8791
goto -> bb2; // scope 2 at $DIR/issue-41110.rs:18:5: 18:6
8892
}
8993

90-
bb12 (cleanup): {
91-
drop(_1) -> bb9; // scope 0 at $DIR/issue-41110.rs:19:1: 19:2
94+
bb13 (cleanup): {
95+
drop(_1) -> bb10; // scope 0 at $DIR/issue-41110.rs:19:1: 19:2
9296
}
9397

94-
bb13 (cleanup): {
95-
switchInt(_6) -> [false: bb9, otherwise: bb12]; // scope 0 at $DIR/issue-41110.rs:19:1: 19:2
98+
bb14 (cleanup): {
99+
switchInt(_6) -> [false: bb10, otherwise: bb13]; // scope 0 at $DIR/issue-41110.rs:19:1: 19:2
96100
}
97101
}

‎src/test/mir-opt/no_spurious_drop_after_call.main.ElaborateDrops.before.mir‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ fn main() -> () {
2828

2929
bb1: {
3030
StorageDead(_3); // scope 0 at $DIR/no-spurious-drop-after-call.rs:9:33: 9:34
31-
_1 = std::mem::drop::<String>(move _2) -> bb2; // scope 0 at $DIR/no-spurious-drop-after-call.rs:9:5: 9:35
31+
_1 = std::mem::drop::<String>(move _2) -> [return: bb2, unwind: bb3]; // scope 0 at $DIR/no-spurious-drop-after-call.rs:9:5: 9:35
3232
// mir::Constant
3333
// + span: $DIR/no-spurious-drop-after-call.rs:9:5: 9:19
3434
// + literal: Const { ty: fn(std::string::String) {std::mem::drop::<std::string::String>}, val: Value(Scalar(<ZST>)) }
@@ -41,4 +41,12 @@ fn main() -> () {
4141
_0 = const (); // scope 0 at $DIR/no-spurious-drop-after-call.rs:8:11: 10:2
4242
return; // scope 0 at $DIR/no-spurious-drop-after-call.rs:10:2: 10:2
4343
}
44+
45+
bb3 (cleanup): {
46+
drop(_2) -> bb4; // scope 0 at $DIR/no-spurious-drop-after-call.rs:9:34: 9:35
47+
}
48+
49+
bb4 (cleanup): {
50+
resume; // scope 0 at $DIR/no-spurious-drop-after-call.rs:8:1: 10:2
51+
}
4452
}

0 commit comments

Comments
 (0)