Skip to content

Commit

Permalink
Auto merge of #39409 - pnkfelix:mir-borrowck2, r=nikomatsakis
Browse files Browse the repository at this point in the history
MIR EndRegion Statements (was MIR dataflow for Borrows)

This PR adds an `EndRegion` statement to MIR (where the `EndRegion` statement is what terminates a borrow).

An earlier version of the PR implemented a dataflow analysis on borrow expressions, but I am now factoring that into a follow-up PR so that reviewing this one is easier. (And also because there are some revisions I want to make to that dataflow code, but I want this PR to get out of WIP status...)

This is a baby step towards MIR borrowck. I just want to get the review process going while I independently work on the remaining steps.
  • Loading branch information
bors committed Jun 19, 2017
2 parents 5ce5126 + 11f4968 commit 0414594
Show file tree
Hide file tree
Showing 45 changed files with 1,047 additions and 73 deletions.
3 changes: 3 additions & 0 deletions src/librustc/ich/impls_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,9 @@ for mir::StatementKind<'tcx> {
mir::StatementKind::StorageDead(ref lvalue) => {
lvalue.hash_stable(hcx, hasher);
}
mir::StatementKind::EndRegion(ref extents) => {
extents.hash_stable(hcx, hasher);
}
mir::StatementKind::Nop => {}
mir::StatementKind::InlineAsm { ref asm, ref outputs, ref inputs } => {
asm.hash_stable(hcx, hasher);
Expand Down
41 changes: 38 additions & 3 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

use graphviz::IntoCow;
use middle::const_val::ConstVal;
use middle::region::CodeExtent;
use rustc_const_math::{ConstUsize, ConstInt, ConstMathErr};
use rustc_data_structures::indexed_vec::{IndexVec, Idx};
use rustc_data_structures::control_flow_graph::dominators::{Dominators, dominators};
Expand Down Expand Up @@ -804,6 +805,10 @@ pub enum StatementKind<'tcx> {
inputs: Vec<Operand<'tcx>>
},

/// Mark one terminating point of an extent (i.e. static region).
/// (The starting point(s) arise implicitly from borrows.)
EndRegion(CodeExtent),

/// No-op. Useful for deleting instructions without affecting statement indices.
Nop,
}
Expand All @@ -813,6 +818,8 @@ impl<'tcx> Debug for Statement<'tcx> {
use self::StatementKind::*;
match self.kind {
Assign(ref lv, ref rv) => write!(fmt, "{:?} = {:?}", lv, rv),
// (reuse lifetime rendering policy from ppaux.)
EndRegion(ref ce) => write!(fmt, "EndRegion({})", ty::ReScope(*ce)),
StorageLive(ref lv) => write!(fmt, "StorageLive({:?})", lv),
StorageDead(ref lv) => write!(fmt, "StorageDead({:?})", lv),
SetDiscriminant{lvalue: ref lv, variant_index: index} => {
Expand Down Expand Up @@ -1176,12 +1183,22 @@ impl<'tcx> Debug for Rvalue<'tcx> {
UnaryOp(ref op, ref a) => write!(fmt, "{:?}({:?})", op, a),
Discriminant(ref lval) => write!(fmt, "discriminant({:?})", lval),
NullaryOp(ref op, ref t) => write!(fmt, "{:?}({:?})", op, t),
Ref(_, borrow_kind, ref lv) => {
Ref(region, borrow_kind, ref lv) => {
let kind_str = match borrow_kind {
BorrowKind::Shared => "",
BorrowKind::Mut | BorrowKind::Unique => "mut ",
};
write!(fmt, "&{}{:?}", kind_str, lv)

// When identifying regions, add trailing space if
// necessary.
let region = if ppaux::identify_regions() {
let mut region = format!("{}", region);
if region.len() > 0 { region.push(' '); }
region
} else {
"".to_owned()
};
write!(fmt, "&{}{}{:?}", region, kind_str, lv)
}

Aggregate(ref kind, ref lvs) => {
Expand Down Expand Up @@ -1224,7 +1241,11 @@ impl<'tcx> Debug for Rvalue<'tcx> {

AggregateKind::Closure(def_id, _) => ty::tls::with(|tcx| {
if let Some(node_id) = tcx.hir.as_local_node_id(def_id) {
let name = format!("[closure@{:?}]", tcx.hir.span(node_id));
let name = if tcx.sess.opts.debugging_opts.span_free_formats {
format!("[closure@{:?}]", node_id)
} else {
format!("[closure@{:?}]", tcx.hir.span(node_id))
};
let mut struct_fmt = fmt.debug_struct(&name);

tcx.with_freevars(node_id, |freevars| {
Expand Down Expand Up @@ -1458,6 +1479,13 @@ impl<'tcx> TypeFoldable<'tcx> for Statement<'tcx> {
outputs: outputs.fold_with(folder),
inputs: inputs.fold_with(folder)
},

// Note for future: If we want to expose the extents
// during the fold, we need to either generalize EndRegion
// to carry `[ty::Region]`, or extend the `TypeFolder`
// trait with a `fn fold_extent`.
EndRegion(ref extent) => EndRegion(extent.clone()),

Nop => Nop,
};
Statement {
Expand All @@ -1476,6 +1504,13 @@ impl<'tcx> TypeFoldable<'tcx> for Statement<'tcx> {
StorageDead(ref lvalue) => lvalue.visit_with(visitor),
InlineAsm { ref outputs, ref inputs, .. } =>
outputs.visit_with(visitor) || inputs.visit_with(visitor),

// Note for future: If we want to expose the extents
// during the visit, we need to either generalize EndRegion
// to carry `[ty::Region]`, or extend the `TypeVisitor`
// trait with a `fn visit_extent`.
EndRegion(ref _extent) => false,

Nop => false,
}
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,7 @@ macro_rules! make_mir_visitor {
ref $($mutability)* rvalue) => {
self.visit_assign(block, lvalue, rvalue, location);
}
StatementKind::EndRegion(_) => {}
StatementKind::SetDiscriminant{ ref $($mutability)* lvalue, .. } => {
self.visit_lvalue(lvalue, LvalueContext::Store, location);
}
Expand Down
4 changes: 4 additions & 0 deletions src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,10 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
DB_OPTIONS, db_type_desc, dbsetters,
verbose: bool = (false, parse_bool, [UNTRACKED],
"in general, enable more debug printouts"),
span_free_formats: bool = (false, parse_bool, [UNTRACKED],
"when debug-printing compiler state, do not include spans"), // o/w tests have closure@path
identify_regions: bool = (false, parse_bool, [UNTRACKED],
"make unnamed regions display as '# (where # is some non-ident unique id)"),
time_passes: bool = (false, parse_bool, [UNTRACKED],
"measure time of each rustc pass"),
count_llvm_insns: bool = (false, parse_bool,
Expand Down
29 changes: 28 additions & 1 deletion src/librustc/util/ppaux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use hir::BodyId;
use hir::def_id::DefId;
use hir::map::definitions::DefPathData;
use middle::region::{CodeExtent, BlockRemainder};
use ty::subst::{self, Subst};
use ty::{BrAnon, BrEnv, BrFresh, BrNamed};
use ty::{TyBool, TyChar, TyAdt};
Expand All @@ -32,6 +34,10 @@ pub fn verbose() -> bool {
ty::tls::with(|tcx| tcx.sess.verbose())
}

pub fn identify_regions() -> bool {
ty::tls::with(|tcx| tcx.sess.opts.debugging_opts.identify_regions)
}

fn fn_sig(f: &mut fmt::Formatter,
inputs: &[Ty],
variadic: bool,
Expand Down Expand Up @@ -519,6 +525,23 @@ impl fmt::Display for ty::RegionKind {
ty::ReSkolemized(_, br) => {
write!(f, "{}", br)
}
ty::ReScope(code_extent) if identify_regions() => {
match code_extent {
CodeExtent::Misc(node_id) =>
write!(f, "'{}mce", node_id.as_u32()),
CodeExtent::CallSiteScope(BodyId { node_id }) =>
write!(f, "'{}cce", node_id.as_u32()),
CodeExtent::ParameterScope(BodyId { node_id }) =>
write!(f, "'{}pce", node_id.as_u32()),
CodeExtent::DestructionScope(node_id) =>
write!(f, "'{}dce", node_id.as_u32()),
CodeExtent::Remainder(BlockRemainder { block, first_statement_index }) =>
write!(f, "'{}_{}rce", block, first_statement_index),
}
}
ty::ReVar(region_vid) if identify_regions() => {
write!(f, "'{}rv", region_vid.index)
}
ty::ReScope(_) |
ty::ReVar(_) |
ty::ReErased => Ok(()),
Expand Down Expand Up @@ -789,7 +812,11 @@ impl<'tcx> fmt::Display for ty::TypeVariants<'tcx> {
write!(f, "[closure")?;

if let Some(node_id) = tcx.hir.as_local_node_id(did) {
write!(f, "@{:?}", tcx.hir.span(node_id))?;
if tcx.sess.opts.debugging_opts.span_free_formats {
write!(f, "@{:?}", node_id)?;
} else {
write!(f, "@{:?}", tcx.hir.span(node_id))?;
}
let mut sep = " ";
tcx.with_freevars(node_id, |freevars| {
for (freevar, upvar_ty) in freevars.iter().zip(upvar_tys) {
Expand Down
1 change: 1 addition & 0 deletions src/librustc_borrowck/borrowck/mir/dataflow/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,7 @@ impl<'a, 'tcx> BitDenotation for MovingOutStatements<'a, 'tcx> {
mir::StatementKind::StorageLive(_) |
mir::StatementKind::StorageDead(_) |
mir::StatementKind::InlineAsm { .. } |
mir::StatementKind::EndRegion(_) |
mir::StatementKind::Nop => {}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ fn each_block<'a, 'tcx, O>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
mir::StatementKind::StorageLive(_) |
mir::StatementKind::StorageDead(_) |
mir::StatementKind::InlineAsm { .. } |
mir::StatementKind::EndRegion(_) |
mir::StatementKind::Nop => continue,
mir::StatementKind::SetDiscriminant{ .. } =>
span_bug!(stmt.source_info.span,
Expand Down
5 changes: 5 additions & 0 deletions src/librustc_borrowck/borrowck/mir/elaborate_drops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,11 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> {
assert!(self.patch.is_patched(bb));
allow_initializations = false;
}
TerminatorKind::Resume => {
// It is possible for `Resume` to be patched
// (in particular it can be patched to be replaced with
// a Goto; see `MirPatch::new`).
}
_ => {
assert!(!self.patch.is_patched(bb));
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_borrowck/borrowck/mir/gather_moves.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,7 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
"SetDiscriminant should not exist during borrowck");
}
StatementKind::InlineAsm { .. } |
StatementKind::EndRegion(_) |
StatementKind::Nop => {}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_borrowck/borrowck/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,7 @@ fn drop_flag_effects_for_location<'a, 'tcx, F>(
mir::StatementKind::StorageLive(_) |
mir::StatementKind::StorageDead(_) |
mir::StatementKind::InlineAsm { .. } |
mir::StatementKind::EndRegion(_) |
mir::StatementKind::Nop => {}
},
None => {
Expand Down
3 changes: 3 additions & 0 deletions src/librustc_driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -914,6 +914,9 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
let mut passes = Passes::new();
passes.push_hook(mir::transform::dump_mir::DumpMir);

// Remove all `EndRegion` statements that are not involved in borrows.
passes.push_pass(MIR_CONST, mir::transform::clean_end_regions::CleanEndRegions);

// What we need to do constant evaluation.
passes.push_pass(MIR_CONST, mir::transform::simplify::SimplifyCfg::new("initial"));
passes.push_pass(MIR_CONST, mir::transform::type_check::TypeckMir);
Expand Down
59 changes: 34 additions & 25 deletions src/librustc_mir/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,24 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
ast_block: &'tcx hir::Block,
source_info: SourceInfo)
-> BlockAnd<()> {
let Block { extent, span, stmts, expr, targeted_by_break } = self.hir.mirror(ast_block);
self.in_scope(extent, block, move |this| {
if targeted_by_break {
// This is a `break`-able block (currently only `catch { ... }`)
let exit_block = this.cfg.start_new_block();
let block_exit = this.in_breakable_scope(None, exit_block,
destination.clone(), |this| {
let Block { extent, opt_destruction_extent, span, stmts, expr, targeted_by_break } =
self.hir.mirror(ast_block);
self.in_opt_scope(opt_destruction_extent.map(|de|(de, source_info)), block, move |this| {
this.in_scope((extent, source_info), block, move |this| {
if targeted_by_break {
// This is a `break`-able block (currently only `catch { ... }`)
let exit_block = this.cfg.start_new_block();
let block_exit = this.in_breakable_scope(
None, exit_block, destination.clone(), |this| {
this.ast_block_stmts(destination, block, span, stmts, expr)
});
this.cfg.terminate(unpack!(block_exit), source_info,
TerminatorKind::Goto { target: exit_block });
exit_block.unit()
} else {
this.ast_block_stmts(destination, block, span, stmts, expr)
});
this.cfg.terminate(unpack!(block_exit), source_info,
TerminatorKind::Goto { target: exit_block });
exit_block.unit()
} else {
this.ast_block_stmts(destination, block, span, stmts, expr)
}
}
})
})
}

Expand Down Expand Up @@ -66,14 +69,18 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
// First we build all the statements in the block.
let mut let_extent_stack = Vec::with_capacity(8);
let outer_visibility_scope = this.visibility_scope;
let source_info = this.source_info(span);
for stmt in stmts {
let Stmt { span: _, kind } = this.hir.mirror(stmt);
let Stmt { span, kind, opt_destruction_extent } = this.hir.mirror(stmt);
match kind {
StmtKind::Expr { scope, expr } => {
unpack!(block = this.in_scope(scope, block, |this| {
let expr = this.hir.mirror(expr);
this.stmt_expr(block, expr)
}));
unpack!(block = this.in_opt_scope(
opt_destruction_extent.map(|de|(de, source_info)), block, |this| {
this.in_scope((scope, source_info), block, |this| {
let expr = this.hir.mirror(expr);
this.stmt_expr(block, expr)
})
}));
}
StmtKind::Let { remainder_scope, init_scope, pattern, initializer } => {
let tcx = this.hir.tcx();
Expand All @@ -89,10 +96,13 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {

// Evaluate the initializer, if present.
if let Some(init) = initializer {
unpack!(block = this.in_scope(init_scope, block, move |this| {
// FIXME #30046 ^~~~
this.expr_into_pattern(block, pattern, init)
}));
unpack!(block = this.in_opt_scope(
opt_destruction_extent.map(|de|(de, source_info)), block, move |this| {
this.in_scope((init_scope, source_info), block, move |this| {
// FIXME #30046 ^~~~
this.expr_into_pattern(block, pattern, init)
})
}));
} else {
this.visit_bindings(&pattern, &mut |this, _, _, node, span, _| {
this.storage_live_binding(block, node, span);
Expand All @@ -112,13 +122,12 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
if let Some(expr) = expr {
unpack!(block = this.into(destination, block, expr));
} else {
let source_info = this.source_info(span);
this.cfg.push_assign_unit(block, source_info, destination);
}
// Finally, we pop all the let scopes before exiting out from the scope of block
// itself.
for extent in let_extent_stack.into_iter().rev() {
unpack!(block = this.pop_scope(extent, block));
unpack!(block = this.pop_scope((extent, source_info), block));
}
// Restore the original visibility scope.
this.visibility_scope = outer_visibility_scope;
Expand Down
11 changes: 11 additions & 0 deletions src/librustc_mir/build/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
//! Routines for manipulating the control-flow graph.

use build::CFG;
use rustc::middle::region::CodeExtent;
use rustc::mir::*;

impl<'tcx> CFG<'tcx> {
Expand Down Expand Up @@ -43,6 +44,16 @@ impl<'tcx> CFG<'tcx> {
self.block_data_mut(block).statements.push(statement);
}

pub fn push_end_region(&mut self,
block: BasicBlock,
source_info: SourceInfo,
extent: CodeExtent) {
self.push(block, Statement {
source_info: source_info,
kind: StatementKind::EndRegion(extent),
});
}

pub fn push_assign(&mut self,
block: BasicBlock,
source_info: SourceInfo,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/build/expr/as_lvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
let source_info = this.source_info(expr_span);
match expr.kind {
ExprKind::Scope { extent, value } => {
this.in_scope(extent, block, |this| this.as_lvalue(block, value))
this.in_scope((extent, source_info), block, |this| this.as_lvalue(block, value))
}
ExprKind::Field { lhs, name } => {
let lvalue = unpack!(block = this.as_lvalue(block, lhs));
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_mir/build/expr/as_operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
let this = self;

if let ExprKind::Scope { extent, value } = expr.kind {
let source_info = this.source_info(expr.span);
let extent = (extent, source_info);
return this.in_scope(extent, block, |this| {
this.as_operand(block, scope, value)
});
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/build/expr/as_rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {

match expr.kind {
ExprKind::Scope { extent, value } => {
let extent = (extent, source_info);
this.in_scope(extent, block, |this| this.as_rvalue(block, scope, value))
}
ExprKind::Repeat { value, count } => {
Expand Down Expand Up @@ -99,7 +100,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
// to start, malloc some memory of suitable type (thus far, uninitialized):
let box_ = Rvalue::NullaryOp(NullOp::Box, value.ty);
this.cfg.push_assign(block, source_info, &result, box_);
this.in_scope(value_extents, block, |this| {
this.in_scope((value_extents, source_info), block, |this| {
// schedule a shallow free of that memory, lest we unwind:
this.schedule_box_free(expr_span, value_extents, &result, value.ty);
// initialize the box contents:
Expand Down
Loading

0 comments on commit 0414594

Please sign in to comment.