Skip to content

Commit b86e675

Browse files
matthewjaspermatthewjasper
committed
Add StorageDead statements for while conditions
1 parent be23bd4 commit b86e675

File tree

8 files changed

+172
-128
lines changed

8 files changed

+172
-128
lines changed

‎src/librustc_mir/build/expr/into.rs‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -179,19 +179,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
179179
// conduct the test, if necessary
180180
let body_block;
181181
if let Some(cond_expr) = opt_cond_expr {
182-
let loop_block_end;
183-
let cond = unpack!(
184-
loop_block_end = this.as_local_operand(loop_block, cond_expr)
185-
);
186-
body_block = this.cfg.start_new_block();
187-
let false_block = this.cfg.start_new_block();
188-
let term = TerminatorKind::if_(
189-
this.hir.tcx(),
190-
cond,
191-
body_block,
192-
false_block,
193-
);
194-
this.cfg.terminate(loop_block_end, source_info, term);
182+
let cond_expr = this.hir.mirror(cond_expr);
183+
let (true_block, false_block)
184+
= this.test_bool(loop_block, cond_expr, source_info);
185+
body_block = true_block;
195186

196187
// if the test is false, there's no `break` to assign `destination`, so
197188
// we have to do it

‎src/librustc_mir/build/matches/mod.rs‎

Lines changed: 11 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1490,15 +1490,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
14901490
};
14911491
let source_info = self.source_info(guard.span);
14921492
let guard_end = self.source_info(tcx.sess.source_map().end_point(guard.span));
1493-
let cond = unpack!(block = self.as_local_operand(block, guard));
1493+
let (post_guard_block, otherwise_post_guard_block)
1494+
= self.test_bool(block, guard, source_info);
14941495
let guard_frame = self.guard_context.pop().unwrap();
14951496
debug!(
14961497
"Exiting guard building context with locals: {:?}",
14971498
guard_frame
14981499
);
14991500

15001501
for &(_, temp) in fake_borrows {
1501-
self.cfg.push(block, Statement {
1502+
self.cfg.push(post_guard_block, Statement {
15021503
source_info: guard_end,
15031504
kind: StatementKind::FakeRead(
15041505
FakeReadCause::ForMatchGuard,
@@ -1507,6 +1508,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
15071508
});
15081509
}
15091510

1511+
self.exit_scope(
1512+
source_info.span,
1513+
region_scope,
1514+
otherwise_post_guard_block,
1515+
candidate.otherwise_block.unwrap(),
1516+
);
1517+
15101518
// We want to ensure that the matched candidates are bound
15111519
// after we have confirmed this candidate *and* any
15121520
// associated guard; Binding them on `block` is too soon,
@@ -1533,41 +1541,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
15331541
// ```
15341542
//
15351543
// and that is clearly not correct.
1536-
let post_guard_block = self.cfg.start_new_block();
1537-
let otherwise_post_guard_block = self.cfg.start_new_block();
1538-
self.cfg.terminate(
1539-
block,
1540-
source_info,
1541-
TerminatorKind::if_(
1542-
self.hir.tcx(),
1543-
cond.clone(),
1544-
post_guard_block,
1545-
otherwise_post_guard_block,
1546-
),
1547-
);
1548-
1549-
self.exit_scope(
1550-
source_info.span,
1551-
region_scope,
1552-
otherwise_post_guard_block,
1553-
candidate.otherwise_block.unwrap(),
1554-
);
1555-
1556-
if let Operand::Copy(cond_place) | Operand::Move(cond_place) = cond {
1557-
if let Place::Base(PlaceBase::Local(cond_temp)) = cond_place {
1558-
// We will call `clear_top_scope` if there's another guard. So
1559-
// we have to drop this variable now or it will be "storage
1560-
// leaked".
1561-
self.pop_variable(
1562-
post_guard_block,
1563-
region_scope.0,
1564-
cond_temp
1565-
);
1566-
} else {
1567-
bug!("Expected as_local_operand to produce a temporary");
1568-
}
1569-
}
1570-
15711544
let by_value_bindings = candidate.bindings.iter().filter(|binding| {
15721545
if let BindingMode::ByValue = binding.binding_mode { true } else { false }
15731546
});
@@ -1577,7 +1550,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
15771550
let local_id = self.var_local_id(binding.var_id, RefWithinGuard);
15781551
let place = Place::from(local_id);
15791552
self.cfg.push(
1580-
block,
1553+
post_guard_block,
15811554
Statement {
15821555
source_info: guard_end,
15831556
kind: StatementKind::FakeRead(FakeReadCause::ForGuardBinding, place),

‎src/librustc_mir/build/scope.rs‎

Lines changed: 73 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ should go to.
8383
*/
8484

8585
use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder, CFG};
86-
use crate::hair::{ExprRef, LintLevel};
86+
use crate::hair::{Expr, ExprRef, LintLevel};
8787
use rustc::middle::region;
8888
use rustc::ty::Ty;
8989
use rustc::hir;
@@ -829,6 +829,78 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
829829

830830
// Other
831831
// =====
832+
/// Branch based on a boolean condition.
833+
///
834+
/// This is a special case because the temporary for the condition needs to
835+
/// be dropped on both the true and the false arm.
836+
pub fn test_bool(
837+
&mut self,
838+
mut block: BasicBlock,
839+
condition: Expr<'tcx>,
840+
source_info: SourceInfo,
841+
) -> (BasicBlock, BasicBlock) {
842+
let cond = unpack!(block = self.as_local_operand(block, condition));
843+
let true_block = self.cfg.start_new_block();
844+
let false_block = self.cfg.start_new_block();
845+
let term = TerminatorKind::if_(
846+
self.hir.tcx(),
847+
cond.clone(),
848+
true_block,
849+
false_block,
850+
);
851+
self.cfg.terminate(block, source_info, term);
852+
853+
match cond {
854+
// Don't try to drop a constant
855+
Operand::Constant(_) => (),
856+
// If constants and statics, we don't generate StorageLive for this
857+
// temporary, so don't try to generate StorageDead for it either.
858+
_ if self.local_scope().is_none() => (),
859+
Operand::Copy(Place::Base(PlaceBase::Local(cond_temp)))
860+
| Operand::Move(Place::Base(PlaceBase::Local(cond_temp))) => {
861+
// Manually drop the condition on both branches.
862+
let top_scope = self.scopes.scopes.last_mut().unwrap();
863+
let top_drop_data = top_scope.drops.pop().unwrap();
864+
865+
match top_drop_data.kind {
866+
DropKind::Value { .. } => {
867+
bug!("Drop scheduled on top of condition variable")
868+
}
869+
DropKind::Storage => {
870+
// Drop the storage for both value and storage drops.
871+
// Only temps and vars need their storage dead.
872+
match top_drop_data.location {
873+
Place::Base(PlaceBase::Local(index)) => {
874+
let source_info = top_scope.source_info(top_drop_data.span);
875+
assert_eq!(index, cond_temp, "Drop scheduled on top of condition");
876+
self.cfg.push(
877+
true_block,
878+
Statement {
879+
source_info,
880+
kind: StatementKind::StorageDead(index)
881+
},
882+
);
883+
self.cfg.push(
884+
false_block,
885+
Statement {
886+
source_info,
887+
kind: StatementKind::StorageDead(index)
888+
},
889+
);
890+
}
891+
_ => unreachable!(),
892+
}
893+
}
894+
}
895+
896+
top_scope.invalidate_cache(true, self.is_generator, true);
897+
}
898+
_ => bug!("Expected as_local_operand to produce a temporary"),
899+
}
900+
901+
(true_block, false_block)
902+
}
903+
832904
/// Creates a path that performs all required cleanup for unwinding.
833905
///
834906
/// This path terminates in Resume. Returns the start of the path.
@@ -942,57 +1014,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
9421014
top_scope.drops.clear();
9431015
top_scope.invalidate_cache(false, self.is_generator, true);
9441016
}
945-
946-
/// Drops the single variable provided
947-
///
948-
/// * The scope must be the top scope.
949-
/// * The variable must be in that scope.
950-
/// * The variable must be at the top of that scope: it's the next thing
951-
/// scheduled to drop.
952-
/// * The drop must be of `DropKind::Storage`.
953-
///
954-
/// This is used for the boolean holding the result of the match guard. We
955-
/// do this because:
956-
///
957-
/// * The boolean is different for each pattern
958-
/// * There is only one exit for the arm scope
959-
/// * The guard expression scope is too short, it ends just before the
960-
/// boolean is tested.
961-
pub(crate) fn pop_variable(
962-
&mut self,
963-
block: BasicBlock,
964-
region_scope: region::Scope,
965-
variable: Local,
966-
) {
967-
let top_scope = self.scopes.scopes.last_mut().unwrap();
968-
969-
assert_eq!(top_scope.region_scope, region_scope);
970-
971-
let top_drop_data = top_scope.drops.pop().unwrap();
972-
973-
match top_drop_data.kind {
974-
DropKind::Value { .. } => {
975-
bug!("Should not be calling pop_top_variable on non-copy type!")
976-
}
977-
DropKind::Storage => {
978-
// Drop the storage for both value and storage drops.
979-
// Only temps and vars need their storage dead.
980-
match top_drop_data.location {
981-
Place::Base(PlaceBase::Local(index)) => {
982-
let source_info = top_scope.source_info(top_drop_data.span);
983-
assert_eq!(index, variable);
984-
self.cfg.push(block, Statement {
985-
source_info,
986-
kind: StatementKind::StorageDead(index)
987-
});
988-
}
989-
_ => unreachable!(),
990-
}
991-
}
992-
}
993-
994-
top_scope.invalidate_cache(true, self.is_generator, true);
995-
}
9961017
}
9971018

9981019
/// Builds drops for pop_scope and exit_scope.

‎src/test/mir-opt/match-arm-scopes.rs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,6 @@ fn main() {
103103
// bb10: { // `else` block - first time
104104
// _9 = (*_6);
105105
// StorageDead(_10);
106-
// FakeRead(ForMatchGuard, _3);
107-
// FakeRead(ForMatchGuard, _4);
108-
// FakeRead(ForGuardBinding, _6);
109-
// FakeRead(ForGuardBinding, _8);
110106
// switchInt(move _9) -> [false: bb16, otherwise: bb15];
111107
// }
112108
// bb11: { // `return 3` - first time
@@ -128,6 +124,10 @@ fn main() {
128124
// }
129125
// bb15: {
130126
// StorageDead(_9);
127+
// FakeRead(ForMatchGuard, _3);
128+
// FakeRead(ForMatchGuard, _4);
129+
// FakeRead(ForGuardBinding, _6);
130+
// FakeRead(ForGuardBinding, _8);
131131
// StorageLive(_5);
132132
// _5 = (_2.1: bool);
133133
// StorageLive(_7);
@@ -159,10 +159,6 @@ fn main() {
159159
// bb19: { // `else` block - second time
160160
// _12 = (*_6);
161161
// StorageDead(_13);
162-
// FakeRead(ForMatchGuard, _3);
163-
// FakeRead(ForMatchGuard, _4);
164-
// FakeRead(ForGuardBinding, _6);
165-
// FakeRead(ForGuardBinding, _8);
166162
// switchInt(move _12) -> [false: bb22, otherwise: bb21];
167163
// }
168164
// bb20: {
@@ -175,6 +171,10 @@ fn main() {
175171
// }
176172
// bb21: { // bindings for arm 1
177173
// StorageDead(_12);
174+
// FakeRead(ForMatchGuard, _3);
175+
// FakeRead(ForMatchGuard, _4);
176+
// FakeRead(ForGuardBinding, _6);
177+
// FakeRead(ForGuardBinding, _8);
178178
// StorageLive(_5);
179179
// _5 = (_2.0: bool);
180180
// StorageLive(_7);

‎src/test/mir-opt/match_false_edges.rs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,12 +71,12 @@ fn main() {
7171
// _7 = const guard() -> [return: bb7, unwind: bb1];
7272
// }
7373
// bb7: { // end of guard
74-
// FakeRead(ForMatchGuard, _4);
75-
// FakeRead(ForGuardBinding, _6);
7674
// switchInt(move _7) -> [false: bb9, otherwise: bb8];
7775
// }
7876
// bb8: { // arm1
7977
// StorageDead(_7);
78+
// FakeRead(ForMatchGuard, _4);
79+
// FakeRead(ForGuardBinding, _6);
8080
// StorageLive(_5);
8181
// _5 = ((_2 as Some).0: i32);
8282
// StorageLive(_8);
@@ -138,12 +138,12 @@ fn main() {
138138
// _7 = const guard() -> [return: bb6, unwind: bb1];
139139
// }
140140
// bb6: { // end of guard
141-
// FakeRead(ForMatchGuard, _4);
142-
// FakeRead(ForGuardBinding, _6);
143141
// switchInt(move _7) -> [false: bb8, otherwise: bb7];
144142
// }
145143
// bb7: {
146144
// StorageDead(_7);
145+
// FakeRead(ForMatchGuard, _4);
146+
// FakeRead(ForGuardBinding, _6);
147147
// StorageLive(_5);
148148
// _5 = ((_2 as Some).0: i32);
149149
// StorageLive(_8);
@@ -209,12 +209,12 @@ fn main() {
209209
// _8 = const guard() -> [return: bb6, unwind: bb1];
210210
// }
211211
// bb6: { //end of guard1
212-
// FakeRead(ForMatchGuard, _5);
213-
// FakeRead(ForGuardBinding, _7);
214212
// switchInt(move _8) -> [false: bb8, otherwise: bb7];
215213
// }
216214
// bb7: {
217215
// StorageDead(_8);
216+
// FakeRead(ForMatchGuard, _5);
217+
// FakeRead(ForGuardBinding, _7);
218218
// StorageLive(_6);
219219
// _6 = ((_2 as Some).0: i32);
220220
// _1 = const 1i32;
@@ -245,12 +245,12 @@ fn main() {
245245
// }
246246
// bb11: { // end of guard2
247247
// StorageDead(_13);
248-
// FakeRead(ForMatchGuard, _5);
249-
// FakeRead(ForGuardBinding, _11);
250248
// switchInt(move _12) -> [false: bb13, otherwise: bb12];
251249
// }
252250
// bb12: { // binding4 & arm4
253251
// StorageDead(_12);
252+
// FakeRead(ForMatchGuard, _5);
253+
// FakeRead(ForGuardBinding, _11);
254254
// StorageLive(_10);
255255
// _10 = ((_2 as Some).0: i32);
256256
// _1 = const 3i32;

‎src/test/mir-opt/match_test.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,11 @@ fn main() {
5454
// _8 = &shallow _1;
5555
// StorageLive(_9);
5656
// _9 = _2;
57-
// FakeRead(ForMatchGuard, _8);
5857
// switchInt(move _9) -> [false: bb11, otherwise: bb10];
5958
// }
6059
// bb10: {
6160
// StorageDead(_9);
61+
// FakeRead(ForMatchGuard, _8);
6262
// _3 = const 0i32;
6363
// goto -> bb14;
6464
// }

0 commit comments

Comments
 (0)