Skip to content

Commit 1d315cf

Browse files
committed
Add EndRegion statement kind to MIR.
* Emit `EndRegion` for every code-extent for which we observe a borrow. To do this, we needed to thread source info back through to `fn in_scope`, which makes this commit a bit more painful than one might have expected. * There is `end_region` emission in `Builder::pop_scope` and in `Builder::exit_scope`; the first handles falling out of a scope normally, the second handles e.g. `break`. * Remove `EndRegion` statements during the erase_regions mir transformation. * Preallocate the terminator block, and throw an `Unreachable` marker on it from the outset. Then overwrite that Terminator as necessary on demand. * Instead of marking the scope as needs_cleanup after seeing a borrow, just treat every scope in the chain as being part of the diverge_block (after any *one* of them has separately signalled that it needs cleanup, e.g. due to having a destructor to run). * Allow for resume terminators to be patched when looking up drop flags. (In particular, `MirPatch::new` has an explicit code path, presumably previously unreachable, that patches up such resume terminators.) * Make `Scope` implement `Debug` trait. * Expanded a stray comment: we do not emit StorageDead on diverging paths, but that end behavior might not be desirable.
1 parent 7c0c4cd commit 1d315cf

File tree

19 files changed

+94
-10
lines changed

19 files changed

+94
-10
lines changed

src/librustc/ich/impls_mir.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,9 @@ for mir::StatementKind<'tcx> {
226226
mir::StatementKind::StorageDead(ref lvalue) => {
227227
lvalue.hash_stable(hcx, hasher);
228228
}
229+
mir::StatementKind::EndRegion(ref extents) => {
230+
extents.hash_stable(hcx, hasher);
231+
}
229232
mir::StatementKind::Nop => {}
230233
mir::StatementKind::InlineAsm { ref asm, ref outputs, ref inputs } => {
231234
asm.hash_stable(hcx, hasher);

src/librustc/mir/mod.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
1313
use graphviz::IntoCow;
1414
use middle::const_val::ConstVal;
15+
use middle::region::CodeExtent;
1516
use rustc_const_math::{ConstUsize, ConstInt, ConstMathErr};
1617
use rustc_data_structures::indexed_vec::{IndexVec, Idx};
1718
use rustc_data_structures::control_flow_graph::dominators::{Dominators, dominators};
@@ -804,6 +805,10 @@ pub enum StatementKind<'tcx> {
804805
inputs: Vec<Operand<'tcx>>
805806
},
806807

808+
/// Mark one terminating point of an extent (i.e. static region).
809+
/// (The starting point(s) arise implicitly from borrows.)
810+
EndRegion(CodeExtent),
811+
807812
/// No-op. Useful for deleting instructions without affecting statement indices.
808813
Nop,
809814
}
@@ -813,6 +818,8 @@ impl<'tcx> Debug for Statement<'tcx> {
813818
use self::StatementKind::*;
814819
match self.kind {
815820
Assign(ref lv, ref rv) => write!(fmt, "{:?} = {:?}", lv, rv),
821+
// (reuse lifetime rendering policy from ppaux.)
822+
EndRegion(ref ce) => write!(fmt, "EndRegion({})", ty::ReScope(*ce)),
816823
StorageLive(ref lv) => write!(fmt, "StorageLive({:?})", lv),
817824
StorageDead(ref lv) => write!(fmt, "StorageDead({:?})", lv),
818825
SetDiscriminant{lvalue: ref lv, variant_index: index} => {
@@ -1472,6 +1479,13 @@ impl<'tcx> TypeFoldable<'tcx> for Statement<'tcx> {
14721479
outputs: outputs.fold_with(folder),
14731480
inputs: inputs.fold_with(folder)
14741481
},
1482+
1483+
// Note for future: If we want to expose the extents
1484+
// during the fold, we need to either generalize EndRegion
1485+
// to carry `[ty::Region]`, or extend the `TypeFolder`
1486+
// trait with a `fn fold_extent`.
1487+
EndRegion(ref extent) => EndRegion(extent.clone()),
1488+
14751489
Nop => Nop,
14761490
};
14771491
Statement {
@@ -1490,6 +1504,13 @@ impl<'tcx> TypeFoldable<'tcx> for Statement<'tcx> {
14901504
StorageDead(ref lvalue) => lvalue.visit_with(visitor),
14911505
InlineAsm { ref outputs, ref inputs, .. } =>
14921506
outputs.visit_with(visitor) || inputs.visit_with(visitor),
1507+
1508+
// Note for future: If we want to expose the extents
1509+
// during the visit, we need to either generalize EndRegion
1510+
// to carry `[ty::Region]`, or extend the `TypeVisitor`
1511+
// trait with a `fn visit_extent`.
1512+
EndRegion(ref _extent) => false,
1513+
14931514
Nop => false,
14941515
}
14951516
}

src/librustc/mir/visit.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,7 @@ macro_rules! make_mir_visitor {
325325
ref $($mutability)* rvalue) => {
326326
self.visit_assign(block, lvalue, rvalue, location);
327327
}
328+
StatementKind::EndRegion(_) => {}
328329
StatementKind::SetDiscriminant{ ref $($mutability)* lvalue, .. } => {
329330
self.visit_lvalue(lvalue, LvalueContext::Store, location);
330331
}

src/librustc_borrowck/borrowck/mir/dataflow/impls.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,7 @@ impl<'a, 'tcx> BitDenotation for MovingOutStatements<'a, 'tcx> {
474474
mir::StatementKind::StorageLive(_) |
475475
mir::StatementKind::StorageDead(_) |
476476
mir::StatementKind::InlineAsm { .. } |
477+
mir::StatementKind::EndRegion(_) |
477478
mir::StatementKind::Nop => {}
478479
}
479480
}

src/librustc_borrowck/borrowck/mir/dataflow/sanity_check.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ fn each_block<'a, 'tcx, O>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
105105
mir::StatementKind::StorageLive(_) |
106106
mir::StatementKind::StorageDead(_) |
107107
mir::StatementKind::InlineAsm { .. } |
108+
mir::StatementKind::EndRegion(_) |
108109
mir::StatementKind::Nop => continue,
109110
mir::StatementKind::SetDiscriminant{ .. } =>
110111
span_bug!(stmt.source_info.span,

src/librustc_borrowck/borrowck/mir/elaborate_drops.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,11 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {
585585
// drop elaboration should handle that by itself
586586
continue
587587
}
588+
TerminatorKind::Resume => {
589+
// We can replace resumes with gotos
590+
// jumping to a canonical resume.
591+
continue
592+
}
588593
TerminatorKind::DropAndReplace { .. } => {
589594
// this contains the move of the source and
590595
// the initialization of the destination. We

src/librustc_borrowck/borrowck/mir/gather_moves.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,7 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
413413
"SetDiscriminant should not exist during borrowck");
414414
}
415415
StatementKind::InlineAsm { .. } |
416+
StatementKind::EndRegion(_) |
416417
StatementKind::Nop => {}
417418
}
418419
}

src/librustc_borrowck/borrowck/mir/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,7 @@ fn drop_flag_effects_for_location<'a, 'tcx, F>(
394394
mir::StatementKind::StorageLive(_) |
395395
mir::StatementKind::StorageDead(_) |
396396
mir::StatementKind::InlineAsm { .. } |
397+
mir::StatementKind::EndRegion(_) |
397398
mir::StatementKind::Nop => {}
398399
},
399400
None => {

src/librustc_mir/build/block.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
7171
let outer_visibility_scope = this.visibility_scope;
7272
let source_info = this.source_info(span);
7373
for stmt in stmts {
74-
let Stmt { span: _, kind, opt_destruction_extent } = this.hir.mirror(stmt);
74+
let Stmt { span, kind, opt_destruction_extent } = this.hir.mirror(stmt);
7575
match kind {
7676
StmtKind::Expr { scope, expr } => {
7777
unpack!(block = this.in_opt_scope(
@@ -122,7 +122,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
122122
if let Some(expr) = expr {
123123
unpack!(block = this.into(destination, block, expr));
124124
} else {
125-
let source_info = this.source_info(span);
126125
this.cfg.push_assign_unit(block, source_info, destination);
127126
}
128127
// Finally, we pop all the let scopes before exiting out from the scope of block

src/librustc_mir/build/cfg.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
//! Routines for manipulating the control-flow graph.
1515
1616
use build::CFG;
17+
use rustc::middle::region::CodeExtent;
1718
use rustc::mir::*;
1819

1920
impl<'tcx> CFG<'tcx> {
@@ -43,6 +44,16 @@ impl<'tcx> CFG<'tcx> {
4344
self.block_data_mut(block).statements.push(statement);
4445
}
4546

47+
pub fn push_end_region(&mut self,
48+
block: BasicBlock,
49+
source_info: SourceInfo,
50+
extent: CodeExtent) {
51+
self.push(block, Statement {
52+
source_info: source_info,
53+
kind: StatementKind::EndRegion(extent),
54+
});
55+
}
56+
4657
pub fn push_assign(&mut self,
4758
block: BasicBlock,
4859
source_info: SourceInfo,

src/librustc_mir/build/expr/as_temp.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
4949

5050
let expr_ty = expr.ty.clone();
5151
let temp = this.temp(expr_ty.clone(), expr_span);
52-
let source_info = this.source_info(expr_span);
5352

5453
if !expr_ty.is_never() && temp_lifetime.is_some() {
5554
this.cfg.push(block, Statement {

src/librustc_mir/build/scope.rs

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,11 @@ use rustc::ty::subst::{Kind, Subst};
9494
use rustc::ty::{Ty, TyCtxt};
9595
use rustc::mir::*;
9696
use rustc::mir::transform::MirSource;
97-
use syntax_pos::Span;
97+
use syntax_pos::{Span};
9898
use rustc_data_structures::indexed_vec::Idx;
9999
use rustc_data_structures::fx::FxHashMap;
100100

101+
#[derive(Debug)]
101102
pub struct Scope<'tcx> {
102103
/// The visibility scope this scope was created in.
103104
visibility_scope: VisibilityScope,
@@ -114,7 +115,7 @@ pub struct Scope<'tcx> {
114115
/// * pollutting the cleanup MIR with StorageDead creates
115116
/// landing pads even though there's no actual destructors
116117
/// * freeing up stack space has no effect during unwinding
117-
needs_cleanup: bool,
118+
pub(super) needs_cleanup: bool,
118119

119120
/// set of lvalues to drop when exiting this scope. This starts
120121
/// out empty but grows as variables are declared during the
@@ -141,6 +142,7 @@ pub struct Scope<'tcx> {
141142
cached_exits: FxHashMap<(BasicBlock, CodeExtent), BasicBlock>,
142143
}
143144

145+
#[derive(Debug)]
144146
struct DropData<'tcx> {
145147
/// span where drop obligation was incurred (typically where lvalue was declared)
146148
span: Span,
@@ -152,6 +154,7 @@ struct DropData<'tcx> {
152154
kind: DropKind
153155
}
154156

157+
#[derive(Debug)]
155158
enum DropKind {
156159
Value {
157160
/// The cached block for the cleanups-on-diverge path. This block
@@ -163,6 +166,7 @@ enum DropKind {
163166
Storage
164167
}
165168

169+
#[derive(Debug)]
166170
struct FreeData<'tcx> {
167171
/// span where free obligation was incurred
168172
span: Span,
@@ -338,6 +342,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
338342
&self.scopes,
339343
block,
340344
self.arg_count));
345+
346+
self.cfg.push_end_region(block, extent.1, scope.extent);
341347
block.unit()
342348
}
343349

@@ -379,6 +385,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
379385
rest,
380386
block,
381387
self.arg_count));
388+
389+
// End all regions for scopes out of which we are breaking.
390+
self.cfg.push_end_region(block, extent.1, scope.extent);
391+
382392
if let Some(ref free_data) = scope.free {
383393
let next = self.cfg.start_new_block();
384394
let free = build_free(self.hir.tcx(), &tmp, free_data, next);
@@ -640,7 +650,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
640650
resumeblk
641651
};
642652

643-
for scope in scopes.iter_mut().filter(|s| s.needs_cleanup) {
653+
for scope in scopes.iter_mut() {
644654
target = build_diverge_scope(hir.tcx(), cfg, &unit_temp, span, scope, target);
645655
}
646656
Some(target)
@@ -775,9 +785,9 @@ fn build_diverge_scope<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
775785
// Build up the drops in **reverse** order. The end result will
776786
// look like:
777787
//
778-
// [drops[n]] -...-> [drops[0]] -> [Free] -> [target]
779-
// | |
780-
// +------------------------------------+
788+
// [EndRegion Block] -> [drops[n]] -...-> [drops[0]] -> [Free] -> [target]
789+
// | |
790+
// +---------------------------------------------------------+
781791
// code for scope
782792
//
783793
// The code in this function reads from right to left. At each
@@ -807,9 +817,16 @@ fn build_diverge_scope<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
807817
// Next, build up the drops. Here we iterate the vector in
808818
// *forward* order, so that we generate drops[0] first (right to
809819
// left in diagram above).
810-
for drop_data in &mut scope.drops {
820+
for (j, drop_data) in scope.drops.iter_mut().enumerate() {
821+
debug!("build_diverge_scope drop_data[{}]: {:?}", j, drop_data);
811822
// Only full value drops are emitted in the diverging path,
812823
// not StorageDead.
824+
//
825+
// Note: This may not actually be what we desire (are we
826+
// "freeing" stack storage as we unwind, or merely observing a
827+
// frozen stack)? In particular, the intent may have been to
828+
// match the behavior of clang, but on inspection eddyb says
829+
// this is not what clang does.
813830
let cached_block = match drop_data.kind {
814831
DropKind::Value { ref mut cached_block } => cached_block,
815832
DropKind::Storage => continue
@@ -829,6 +846,15 @@ fn build_diverge_scope<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
829846
};
830847
}
831848

849+
// Finally, push the EndRegion block, used by mir-borrowck. (Block
850+
// becomes trivial goto after pass that removes all EndRegions.)
851+
{
852+
let block = cfg.start_new_cleanup_block();
853+
cfg.push_end_region(block, source_info(span), scope.extent);
854+
cfg.terminate(block, source_info(span), TerminatorKind::Goto { target: target });
855+
target = block
856+
}
857+
832858
target
833859
}
834860

src/librustc_mir/transform/erase_regions.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,15 @@ impl<'a, 'tcx> MutVisitor<'tcx> for EraseRegionsVisitor<'a, 'tcx> {
6565
substs: &mut ClosureSubsts<'tcx>) {
6666
*substs = self.tcx.erase_regions(substs);
6767
}
68+
69+
fn visit_statement(&mut self,
70+
_block: BasicBlock,
71+
statement: &mut Statement<'tcx>,
72+
_location: Location) {
73+
if let StatementKind::EndRegion(_) = statement.kind {
74+
statement.kind = StatementKind::Nop;
75+
}
76+
}
6877
}
6978

7079
pub struct EraseRegions;

src/librustc_mir/transform/qualify_consts.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -907,6 +907,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
907907
StatementKind::StorageLive(_) |
908908
StatementKind::StorageDead(_) |
909909
StatementKind::InlineAsm {..} |
910+
StatementKind::EndRegion(_) |
910911
StatementKind::Nop => {}
911912
}
912913
});

src/librustc_mir/transform/type_check.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
413413
}
414414
}
415415
StatementKind::InlineAsm { .. } |
416+
StatementKind::EndRegion(_) |
416417
StatementKind::Nop => {}
417418
}
418419
}

src/librustc_mir/util/patch.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ impl<'tcx> MirPatch<'tcx> {
4646
for (bb, block) in mir.basic_blocks().iter_enumerated() {
4747
if let TerminatorKind::Resume = block.terminator().kind {
4848
if block.statements.len() > 0 {
49+
assert!(resume_stmt_block.is_none());
4950
resume_stmt_block = Some(bb);
5051
} else {
5152
resume_block = Some(bb);

src/librustc_passes/mir_stats.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ impl<'a, 'tcx> mir_visit::Visitor<'tcx> for StatCollector<'a, 'tcx> {
125125
self.record("Statement", statement);
126126
self.record(match statement.kind {
127127
StatementKind::Assign(..) => "StatementKind::Assign",
128+
StatementKind::EndRegion(..) => "StatementKind::EndRegion",
128129
StatementKind::SetDiscriminant { .. } => "StatementKind::SetDiscriminant",
129130
StatementKind::StorageLive(..) => "StatementKind::StorageLive",
130131
StatementKind::StorageDead(..) => "StatementKind::StorageDead",

src/librustc_trans/mir/constant.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,7 @@ impl<'a, 'tcx> MirConstContext<'a, 'tcx> {
284284
}
285285
mir::StatementKind::StorageLive(_) |
286286
mir::StatementKind::StorageDead(_) |
287+
mir::StatementKind::EndRegion(_) |
287288
mir::StatementKind::Nop => {}
288289
mir::StatementKind::InlineAsm { .. } |
289290
mir::StatementKind::SetDiscriminant{ .. } => {

src/librustc_trans/mir/statement.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> {
8686
asm::trans_inline_asm(&bcx, asm, outputs, input_vals);
8787
bcx
8888
}
89+
mir::StatementKind::EndRegion(_) |
8990
mir::StatementKind::Nop => bcx,
9091
}
9192
}

0 commit comments

Comments
 (0)