From be9c2d13815eb9aa1b6213a46542aa4262127643 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 20 May 2014 18:49:19 +0200 Subject: [PATCH 1/9] Bug fixes for flowgraph construction. 1. After recursively processing an ExprWhile, need to pop loop_scopes the same way we do for ExprLoop. 2. Proposed fix for flowgraph handling of ExprInlineAsm: we need to represent the flow into the subexpressions of an `asm!` block. --- src/librustc/middle/cfg/construct.rs | 38 ++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/src/librustc/middle/cfg/construct.rs b/src/librustc/middle/cfg/construct.rs index f855ca37597a3..0d8729071ef6b 100644 --- a/src/librustc/middle/cfg/construct.rs +++ b/src/librustc/middle/cfg/construct.rs @@ -254,6 +254,7 @@ impl<'a> CFGBuilder<'a> { }); let body_exit = self.block(&**body, cond_exit); // 4 self.add_contained_edge(body_exit, loopback); // 5 + self.loop_scopes.pop(); expr_exit } @@ -427,8 +428,22 @@ impl<'a> CFGBuilder<'a> { self.straightline(expr, pred, [e]) } + ast::ExprInlineAsm(ref inline_asm) => { + let inputs = inline_asm.inputs.iter(); + let outputs = inline_asm.outputs.iter(); + fn extract_expr(&(_, expr): &(A, Gc)) -> Gc { expr } + let post_inputs = self.exprs(inputs.map(|a| { + debug!("cfg::construct InlineAsm id:{} input:{:?}", expr.id, a); + extract_expr(a) + }), pred); + let post_outputs = self.exprs(outputs.map(|a| { + debug!("cfg::construct InlineAsm id:{} output:{:?}", expr.id, a); + extract_expr(a) + }), post_inputs); + self.add_node(expr.id, [post_outputs]) + } + ast::ExprMac(..) | - ast::ExprInlineAsm(..) | ast::ExprFnBlock(..) | ast::ExprProc(..) | ast::ExprLit(..) | @@ -444,15 +459,22 @@ impl<'a> CFGBuilder<'a> { func_or_rcvr: Gc, args: &[Gc]) -> CFGIndex { let func_or_rcvr_exit = self.expr(func_or_rcvr, pred); - self.straightline(call_expr, func_or_rcvr_exit, args) + let ret = self.straightline(call_expr, func_or_rcvr_exit, args); + + let return_ty = ty::node_id_to_type(self.tcx, call_expr.id); + let fails = ty::type_is_bot(return_ty); + if fails { + self.add_node(ast::DUMMY_NODE_ID, []) + } else { + ret + } } - fn exprs(&mut self, - exprs: &[Gc], - pred: CFGIndex) -> CFGIndex { + fn exprs>>(&mut self, + mut exprs: I, + pred: CFGIndex) -> CFGIndex { //! Constructs graph for `exprs` evaluated in order - - exprs.iter().fold(pred, |p, &e| self.expr(e, p)) + exprs.fold(pred, |p, e| self.expr(e, p)) } fn opt_expr(&mut self, @@ -469,7 +491,7 @@ impl<'a> CFGBuilder<'a> { subexprs: &[Gc]) -> CFGIndex { //! Handles case of an expression that evaluates `subexprs` in order - let subexprs_exit = self.exprs(subexprs, pred); + let subexprs_exit = self.exprs(subexprs.iter().map(|&e|e), pred); self.add_node(expr.id, [subexprs_exit]) } From fef63e2f237ea016b367c97dca50f35ab68c5164 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 21 May 2014 14:48:03 +0200 Subject: [PATCH 2/9] NodeIndex should derive `Show`. --- src/librustc/middle/graph.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/middle/graph.rs b/src/librustc/middle/graph.rs index e3a89fe2525d2..3d5e4b167a330 100644 --- a/src/librustc/middle/graph.rs +++ b/src/librustc/middle/graph.rs @@ -55,7 +55,7 @@ pub struct Edge { pub data: E, } -#[deriving(PartialEq)] +#[deriving(PartialEq, Show)] pub struct NodeIndex(pub uint); pub static InvalidNodeIndex: NodeIndex = NodeIndex(uint::MAX); From 75340f41763c4166172af24c8db676c1da97910d Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 21 May 2014 14:49:16 +0200 Subject: [PATCH 3/9] Revise dataflow to do a cfg-driven walk. Fix #6298. This is instead of the prior approach of emulating cfg traversal privately by traversing AST in same way). Of special note, this removes a special case handling of `ExprParen` that was actually injecting a bug (since it was acting like an expression like `(*func)()` was consuming `*func` *twice*: once from `(*func)` and again from `*func`). nikomatsakis was the first one to point out that it might suffice to simply have the outer `ExprParen` do the consumption of the contents (alone). (This version has been updated to incorporate feedback from Niko's review of PR 14873.) --- src/librustc/middle/borrowck/mod.rs | 24 +- src/librustc/middle/borrowck/move_data.rs | 31 +- src/librustc/middle/dataflow.rs | 872 ++++++++-------------- src/librustc/middle/expr_use_visitor.rs | 19 +- src/librustc/middle/graph.rs | 2 +- 5 files changed, 340 insertions(+), 608 deletions(-) diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs index 18cd0b1326d96..98edc6365cf32 100644 --- a/src/librustc/middle/borrowck/mod.rs +++ b/src/librustc/middle/borrowck/mod.rs @@ -12,7 +12,9 @@ #![allow(non_camel_case_types)] +use middle::cfg; use middle::dataflow::DataFlowContext; +use middle::dataflow::BitwiseOperator; use middle::dataflow::DataFlowOperator; use middle::def; use euv = middle::expr_use_visitor; @@ -126,8 +128,13 @@ fn borrowck_fn(this: &mut BorrowckCtxt, let id_range = ast_util::compute_id_range_for_fn_body(fk, decl, body, sp, id); let (all_loans, move_data) = gather_loans::gather_loans_in_fn(this, decl, body); + let cfg = cfg::CFG::new(this.tcx, body); + let mut loan_dfcx = DataFlowContext::new(this.tcx, + "borrowck", + Some(decl), + &cfg, LoanDataFlowOperator, id_range, all_loans.len()); @@ -135,11 +142,14 @@ fn borrowck_fn(this: &mut BorrowckCtxt, loan_dfcx.add_gen(loan.gen_scope, loan_idx); loan_dfcx.add_kill(loan.kill_scope, loan_idx); } - loan_dfcx.propagate(body); + loan_dfcx.add_kills_from_flow_exits(&cfg); + loan_dfcx.propagate(&cfg, body); let flowed_moves = move_data::FlowedMoveData::new(move_data, this.tcx, + &cfg, id_range, + decl, body); check_loans::check_loans(this, &loan_dfcx, flowed_moves, @@ -753,15 +763,17 @@ fn is_statement_scope(tcx: &ty::ctxt, region: ty::Region) -> bool { } } -impl DataFlowOperator for LoanDataFlowOperator { +impl BitwiseOperator for LoanDataFlowOperator { #[inline] - fn initial_value(&self) -> bool { - false // no loans in scope by default + fn join(&self, succ: uint, pred: uint) -> uint { + succ | pred // loans from both preds are in scope } +} +impl DataFlowOperator for LoanDataFlowOperator { #[inline] - fn join(&self, succ: uint, pred: uint) -> uint { - succ | pred // loans from both preds are in scope + fn initial_value(&self) -> bool { + false // no loans in scope by default } } diff --git a/src/librustc/middle/borrowck/move_data.rs b/src/librustc/middle/borrowck/move_data.rs index f7c2629233484..3e2f763a84bc2 100644 --- a/src/librustc/middle/borrowck/move_data.rs +++ b/src/librustc/middle/borrowck/move_data.rs @@ -20,7 +20,9 @@ use std::rc::Rc; use std::uint; use std::collections::{HashMap, HashSet}; use middle::borrowck::*; +use middle::cfg; use middle::dataflow::DataFlowContext; +use middle::dataflow::BitwiseOperator; use middle::dataflow::DataFlowOperator; use euv = middle::expr_use_visitor; use middle::ty; @@ -499,22 +501,33 @@ impl MoveData { impl<'a> FlowedMoveData<'a> { pub fn new(move_data: MoveData, tcx: &'a ty::ctxt, + cfg: &'a cfg::CFG, id_range: ast_util::IdRange, + decl: &ast::FnDecl, body: &ast::Block) -> FlowedMoveData<'a> { let mut dfcx_moves = DataFlowContext::new(tcx, + "flowed_move_data_moves", + Some(decl), + cfg, MoveDataFlowOperator, id_range, move_data.moves.borrow().len()); let mut dfcx_assign = DataFlowContext::new(tcx, + "flowed_move_data_assigns", + Some(decl), + cfg, AssignDataFlowOperator, id_range, move_data.var_assignments.borrow().len()); move_data.add_gen_kills(tcx, &mut dfcx_moves, &mut dfcx_assign); - dfcx_moves.propagate(body); - dfcx_assign.propagate(body); + dfcx_moves.add_kills_from_flow_exits(cfg); + dfcx_assign.add_kills_from_flow_exits(cfg); + dfcx_moves.propagate(cfg, body); + dfcx_assign.propagate(cfg, body); + FlowedMoveData { move_data: move_data, dfcx_moves: dfcx_moves, @@ -659,12 +672,21 @@ impl<'a> FlowedMoveData<'a> { } } +impl BitwiseOperator for MoveDataFlowOperator { + #[inline] + fn join(&self, succ: uint, pred: uint) -> uint { + succ | pred // moves from both preds are in scope + } +} + impl DataFlowOperator for MoveDataFlowOperator { #[inline] fn initial_value(&self) -> bool { false // no loans in scope by default } +} +impl BitwiseOperator for AssignDataFlowOperator { #[inline] fn join(&self, succ: uint, pred: uint) -> uint { succ | pred // moves from both preds are in scope @@ -676,9 +698,4 @@ impl DataFlowOperator for AssignDataFlowOperator { fn initial_value(&self) -> bool { false // no assignments in scope by default } - - #[inline] - fn join(&self, succ: uint, pred: uint) -> uint { - succ | pred // moves from both preds are in scope - } } diff --git a/src/librustc/middle/dataflow.rs b/src/librustc/middle/dataflow.rs index bc083dac6ac75..872a0878e3792 100644 --- a/src/librustc/middle/dataflow.rs +++ b/src/librustc/middle/dataflow.rs @@ -17,23 +17,24 @@ */ -use middle::def; +use middle::cfg; +use middle::cfg::CFGIndex; use middle::ty; -use middle::typeck; use std::io; -use std::gc::Gc; use std::uint; use syntax::ast; -use syntax::ast_util; use syntax::ast_util::IdRange; +use syntax::visit; use syntax::print::{pp, pprust}; -use util::ppaux::Repr; use util::nodemap::NodeMap; #[deriving(Clone)] pub struct DataFlowContext<'a, O> { tcx: &'a ty::ctxt, + /// a name for the analysis using this dataflow instance + analysis_name: &'static str, + /// the data flow operator oper: O, @@ -44,33 +45,39 @@ pub struct DataFlowContext<'a, O> { /// equal to bits_per_id/uint::BITS rounded up. words_per_id: uint, - // mapping from node to bitset index. - nodeid_to_bitset: NodeMap, + // mapping from cfg node index to bitset index. + index_to_bitset: Vec>, + + // mapping from node to cfg node index + // FIXME (#6298): Shouldn't this go with CFG? + nodeid_to_index: NodeMap, - // Bit sets per id. The following three fields (`gens`, `kills`, + // Bit sets per cfg node. The following three fields (`gens`, `kills`, // and `on_entry`) all have the same structure. For each id in // `id_range`, there is a range of words equal to `words_per_id`. // So, to access the bits for any given id, you take a slice of // the full vector (see the method `compute_id_range()`). - /// bits generated as we exit the scope `id`. Updated by `add_gen()`. + /// bits generated as we exit the cfg node. Updated by `add_gen()`. gens: Vec, - /// bits killed as we exit the scope `id`. Updated by `add_kill()`. + /// bits killed as we exit the cfg node. Updated by `add_kill()`. kills: Vec, - /// bits that are valid on entry to the scope `id`. Updated by + /// bits that are valid on entry to the cfg node. Updated by /// `propagate()`. on_entry: Vec, } +pub trait BitwiseOperator { + /// Joins two predecessor bits together, typically either `|` or `&` + fn join(&self, succ: uint, pred: uint) -> uint; +} + /// Parameterization for the precise form of data flow that is used. -pub trait DataFlowOperator { +pub trait DataFlowOperator : BitwiseOperator { /// Specifies the initial value for each bit in the `on_entry` set fn initial_value(&self) -> bool; - - /// Joins two predecessor bits together, typically either `|` or `&` - fn join(&self, succ: uint, pred: uint) -> uint; } struct PropagationContext<'a, 'b, O> { @@ -78,9 +85,68 @@ struct PropagationContext<'a, 'b, O> { changed: bool } -struct LoopScope<'a> { - loop_id: ast::NodeId, - break_bits: Vec +fn to_cfgidx_or_die(id: ast::NodeId, index: &NodeMap) -> CFGIndex { + let opt_cfgindex = index.find(&id).map(|&i|i); + opt_cfgindex.unwrap_or_else(|| { + fail!("nodeid_to_index does not have entry for NodeId {}", id); + }) +} + +impl<'a, O:DataFlowOperator> DataFlowContext<'a, O> { + fn has_bitset(&self, n: ast::NodeId) -> bool { + assert!(n != ast::DUMMY_NODE_ID); + match self.nodeid_to_index.find(&n) { + None => false, + Some(&cfgidx) => { + let node_id = cfgidx.node_id(); + node_id < self.index_to_bitset.len() && + self.index_to_bitset.get(node_id).is_some() + } + } + } + fn get_bitset_index(&self, cfgidx: CFGIndex) -> uint { + let node_id = cfgidx.node_id(); + self.index_to_bitset.get(node_id).unwrap() + } + fn get_or_create_bitset_index(&mut self, cfgidx: CFGIndex) -> uint { + assert!(self.words_per_id > 0); + let len = self.gens.len() / self.words_per_id; + let expanded; + let n; + if self.index_to_bitset.len() <= cfgidx.node_id() { + self.index_to_bitset.grow_set(cfgidx.node_id(), &None, Some(len)); + expanded = true; + n = len; + } else { + let entry = self.index_to_bitset.get_mut(cfgidx.node_id()); + match *entry { + None => { + *entry = Some(len); + expanded = true; + n = len; + } + Some(bitidx) => { + expanded = false; + n = bitidx; + } + } + } + if expanded { + let entry = if self.oper.initial_value() { uint::MAX } else {0}; + for _ in range(0, self.words_per_id) { + self.gens.push(0); + self.kills.push(0); + self.on_entry.push(entry); + } + } + + let start = n * self.words_per_id; + let end = start + self.words_per_id; + let len = self.gens.len(); + assert!(start < len); + assert!(end <= len); + n + } } impl<'a, O:DataFlowOperator> pprust::PpAnn for DataFlowContext<'a, O> { @@ -94,8 +160,9 @@ impl<'a, O:DataFlowOperator> pprust::PpAnn for DataFlowContext<'a, O> { pprust::NodePat(pat) => pat.id }; - if self.nodeid_to_bitset.contains_key(&id) { - let (start, end) = self.compute_id_range_frozen(id); + if self.has_bitset(id) { + let cfgidx = to_cfgidx_or_die(id, &self.nodeid_to_index); + let (start, end) = self.compute_id_range_frozen(cfgidx); let on_entry = self.on_entry.slice(start, end); let entry_str = bits_to_str(on_entry); @@ -121,24 +188,74 @@ impl<'a, O:DataFlowOperator> pprust::PpAnn for DataFlowContext<'a, O> { } } +fn build_nodeid_to_index(decl: Option<&ast::FnDecl>, + cfg: &cfg::CFG) -> NodeMap { + let mut index = NodeMap::new(); + + // FIXME (#6298): Would it be better to fold formals from decl + // into cfg itself? i.e. introduce a fn-based flow-graph in + // addition to the current block-based flow-graph, rather than + // have to put traversals like this here? + match decl { + None => {} + Some(decl) => add_entries_from_fn_decl(&mut index, decl, cfg.entry) + } + + cfg.graph.each_node(|node_idx, node| { + if node.data.id != ast::DUMMY_NODE_ID { + index.insert(node.data.id, node_idx); + } + true + }); + + return index; + + fn add_entries_from_fn_decl(index: &mut NodeMap, + decl: &ast::FnDecl, + entry: CFGIndex) { + //! add mappings from the ast nodes for the formal bindings to + //! the entry-node in the graph. + struct Formals<'a> { + entry: CFGIndex, + index: &'a mut NodeMap, + } + let mut formals = Formals { entry: entry, index: index }; + visit::walk_fn_decl(&mut formals, decl, ()); + impl<'a> visit::Visitor<()> for Formals<'a> { + fn visit_pat(&mut self, p: &ast::Pat, e: ()) { + self.index.insert(p.id, self.entry); + visit::walk_pat(self, p, e) + } + } + } +} + impl<'a, O:DataFlowOperator> DataFlowContext<'a, O> { pub fn new(tcx: &'a ty::ctxt, + analysis_name: &'static str, + decl: Option<&ast::FnDecl>, + cfg: &cfg::CFG, oper: O, id_range: IdRange, bits_per_id: uint) -> DataFlowContext<'a, O> { let words_per_id = (bits_per_id + uint::BITS - 1) / uint::BITS; - debug!("DataFlowContext::new(id_range={:?}, bits_per_id={:?}, words_per_id={:?})", - id_range, bits_per_id, words_per_id); + debug!("DataFlowContext::new(analysis_name: {:s}, id_range={:?}, \ + bits_per_id={:?}, words_per_id={:?})", + analysis_name, id_range, bits_per_id, words_per_id); let gens = Vec::new(); let kills = Vec::new(); let on_entry = Vec::new(); + let nodeid_to_index = build_nodeid_to_index(decl, cfg); + DataFlowContext { tcx: tcx, + analysis_name: analysis_name, words_per_id: words_per_id, - nodeid_to_bitset: NodeMap::new(), + index_to_bitset: Vec::new(), + nodeid_to_index: nodeid_to_index, bits_per_id: bits_per_id, oper: oper, gens: gens, @@ -149,74 +266,60 @@ impl<'a, O:DataFlowOperator> DataFlowContext<'a, O> { pub fn add_gen(&mut self, id: ast::NodeId, bit: uint) { //! Indicates that `id` generates `bit` - - debug!("add_gen(id={:?}, bit={:?})", id, bit); - let (start, end) = self.compute_id_range(id); - { - let gens = self.gens.mut_slice(start, end); - set_bit(gens, bit); + if self.nodeid_to_index.contains_key(&id) { + debug!("add_gen(id={:?}, bit={:?})", id, bit); + let cfgidx = to_cfgidx_or_die(id, &self.nodeid_to_index); + let (start, end) = self.compute_id_range(cfgidx); + { + let gens = self.gens.mut_slice(start, end); + set_bit(gens, bit); + } + } else { + debug!("{:s} add_gen skip (id={:?}, bit={:?}); id not in current fn", + self.analysis_name, id, bit); } } pub fn add_kill(&mut self, id: ast::NodeId, bit: uint) { //! Indicates that `id` kills `bit` - - debug!("add_kill(id={:?}, bit={:?})", id, bit); - let (start, end) = self.compute_id_range(id); - { - let kills = self.kills.mut_slice(start, end); - set_bit(kills, bit); + if self.nodeid_to_index.contains_key(&id) { + debug!("add_kill(id={:?}, bit={:?})", id, bit); + let cfgidx = to_cfgidx_or_die(id, &self.nodeid_to_index); + let (start, end) = self.compute_id_range(cfgidx); + { + let kills = self.kills.mut_slice(start, end); + set_bit(kills, bit); + } + } else { + debug!("{:s} add_kill skip (id={:?}, bit={:?}); id not in current fn", + self.analysis_name, id, bit); } } - fn apply_gen_kill(&mut self, id: ast::NodeId, bits: &mut [uint]) { + fn apply_gen_kill(&mut self, cfgidx: CFGIndex, bits: &mut [uint]) { //! Applies the gen and kill sets for `id` to `bits` - - debug!("apply_gen_kill(id={:?}, bits={}) [before]", - id, mut_bits_to_str(bits)); - let (start, end) = self.compute_id_range(id); + debug!("{:s} apply_gen_kill(cfgidx={}, bits={}) [before]", + self.analysis_name, cfgidx, mut_bits_to_str(bits)); + let (start, end) = self.compute_id_range(cfgidx); let gens = self.gens.slice(start, end); - bitwise(bits, gens, |a, b| a | b); + bitwise(bits, gens, &Union); let kills = self.kills.slice(start, end); - bitwise(bits, kills, |a, b| a & !b); - - debug!("apply_gen_kill(id={:?}, bits={}) [after]", - id, mut_bits_to_str(bits)); - } + bitwise(bits, kills, &Subtract); - fn apply_kill(&mut self, id: ast::NodeId, bits: &mut [uint]) { - debug!("apply_kill(id={:?}, bits={}) [before]", - id, mut_bits_to_str(bits)); - let (start, end) = self.compute_id_range(id); - let kills = self.kills.slice(start, end); - bitwise(bits, kills, |a, b| a & !b); - debug!("apply_kill(id={:?}, bits={}) [after]", - id, mut_bits_to_str(bits)); + debug!("{:s} apply_gen_kill(cfgidx={}, bits={}) [after]", + self.analysis_name, cfgidx, mut_bits_to_str(bits)); } - fn compute_id_range_frozen(&self, id: ast::NodeId) -> (uint, uint) { - let n = *self.nodeid_to_bitset.get(&id); + fn compute_id_range_frozen(&self, cfgidx: CFGIndex) -> (uint, uint) { + let n = self.get_bitset_index(cfgidx); let start = n * self.words_per_id; let end = start + self.words_per_id; (start, end) } - fn compute_id_range(&mut self, id: ast::NodeId) -> (uint, uint) { - let mut expanded = false; - let len = self.nodeid_to_bitset.len(); - let n = self.nodeid_to_bitset.find_or_insert_with(id, |_| { - expanded = true; - len - }); - if expanded { - let entry = if self.oper.initial_value() { uint::MAX } else {0}; - for _ in range(0, self.words_per_id) { - self.gens.push(0); - self.kills.push(0); - self.on_entry.push(entry); - } - } - let start = *n * self.words_per_id; + fn compute_id_range(&mut self, cfgidx: CFGIndex) -> (uint, uint) { + let n = self.get_or_create_bitset_index(cfgidx); + let start = n * self.words_per_id; let end = start + self.words_per_id; assert!(start < self.gens.len()); @@ -234,26 +337,28 @@ impl<'a, O:DataFlowOperator> DataFlowContext<'a, O> { -> bool { //! Iterates through each bit that is set on entry to `id`. //! Only useful after `propagate()` has been called. - if !self.nodeid_to_bitset.contains_key(&id) { + if !self.has_bitset(id) { return true; } - let (start, end) = self.compute_id_range_frozen(id); + let cfgidx = to_cfgidx_or_die(id, &self.nodeid_to_index); + let (start, end) = self.compute_id_range_frozen(cfgidx); let on_entry = self.on_entry.slice(start, end); - debug!("each_bit_on_entry_frozen(id={:?}, on_entry={})", - id, bits_to_str(on_entry)); + debug!("{:s} each_bit_on_entry_frozen(id={:?}, on_entry={})", + self.analysis_name, id, bits_to_str(on_entry)); self.each_bit(on_entry, f) } pub fn each_gen_bit_frozen(&self, id: ast::NodeId, f: |uint| -> bool) -> bool { //! Iterates through each bit in the gen set for `id`. - if !self.nodeid_to_bitset.contains_key(&id) { + if !self.has_bitset(id) { return true; } - let (start, end) = self.compute_id_range_frozen(id); + let cfgidx = to_cfgidx_or_die(id, &self.nodeid_to_index); + let (start, end) = self.compute_id_range_frozen(cfgidx); let gens = self.gens.slice(start, end); - debug!("each_gen_bit(id={:?}, gens={})", - id, bits_to_str(gens)); + debug!("{:s} each_gen_bit(id={:?}, gens={})", + self.analysis_name, id, bits_to_str(gens)); self.each_bit(gens, f) } @@ -287,11 +392,63 @@ impl<'a, O:DataFlowOperator> DataFlowContext<'a, O> { } return true; } + + pub fn add_kills_from_flow_exits(&mut self, cfg: &cfg::CFG) { + //! Whenever you have a `break` or `continue` statement, flow + //! exits through any number of enclosing scopes on its way to + //! the new destination. This function infers the kill bits of + //! those control operators based on the kill bits associated + //! with those scopes. + //! + //! This is usually called (if it is called at all), after + //! all add_gen and add_kill calls, but before propagate. + + debug!("{:s} add_kills_from_flow_exits", self.analysis_name); + if self.bits_per_id == 0 { + // Skip the surprisingly common degenerate case. (Note + // compute_id_range requires self.words_per_id > 0.) + return; + } + cfg.graph.each_edge(|_edge_index, edge| { + let flow_exit = edge.source(); + let (start, end) = self.compute_id_range(flow_exit); + let mut orig_kills = self.kills.slice(start, end).to_owned(); + + let mut changed = false; + for &node_id in edge.data.exiting_scopes.iter() { + let opt_cfg_idx = self.nodeid_to_index.find(&node_id).map(|&i|i); + match opt_cfg_idx { + Some(cfg_idx) => { + let (start, end) = self.compute_id_range(cfg_idx); + let kills = self.kills.slice(start, end); + if bitwise(orig_kills.as_mut_slice(), kills, &Union) { + changed = true; + } + } + None => { + debug!("{:s} add_kills_from_flow_exits flow_exit={} \ + no cfg_idx for exiting_scope={:?}", + self.analysis_name, flow_exit, node_id); + } + } + } + + if changed { + let bits = self.kills.mut_slice(start, end); + debug!("{:s} add_kills_from_flow_exits flow_exit={} bits={} [before]", + self.analysis_name, flow_exit, mut_bits_to_str(bits)); + bits.copy_from(orig_kills.as_slice()); + debug!("{:s} add_kills_from_flow_exits flow_exit={} bits={} [after]", + self.analysis_name, flow_exit, mut_bits_to_str(bits)); + } + true + }); + } } impl<'a, O:DataFlowOperator+Clone+'static> DataFlowContext<'a, O> { // ^^^^^^^^^^^^^ only needed for pretty printing - pub fn propagate(&mut self, blk: &ast::Block) { + pub fn propagate(&mut self, cfg: &cfg::CFG, blk: &ast::Block) { //! Performs the data flow analysis. if self.bits_per_id == 0 { @@ -307,16 +464,14 @@ impl<'a, O:DataFlowOperator+Clone+'static> DataFlowContext<'a, O> { }; let mut temp = Vec::from_elem(words_per_id, 0u); - let mut loop_scopes = Vec::new(); - while propcx.changed { propcx.changed = false; propcx.reset(temp.as_mut_slice()); - propcx.walk_block(blk, temp.as_mut_slice(), &mut loop_scopes); + propcx.walk_cfg(cfg, temp.as_mut_slice()); } } - debug!("Dataflow result:"); + debug!("Dataflow result for {:s}:", self.analysis_name); debug!("{}", { self.pretty_print_to(box io::stderr(), blk).unwrap(); "" @@ -334,462 +489,30 @@ impl<'a, O:DataFlowOperator+Clone+'static> DataFlowContext<'a, O> { } impl<'a, 'b, O:DataFlowOperator> PropagationContext<'a, 'b, O> { - fn tcx(&self) -> &'b ty::ctxt { - self.dfcx.tcx - } - - fn walk_block(&mut self, - blk: &ast::Block, - in_out: &mut [uint], - loop_scopes: &mut Vec ) { - debug!("DataFlowContext::walk_block(blk.id={}, in_out={})", - blk.id, bits_to_str(in_out)); - - self.merge_with_entry_set(blk.id, in_out); - - for stmt in blk.stmts.iter() { - self.walk_stmt(stmt.clone(), in_out, loop_scopes); - } - - self.walk_opt_expr(blk.expr, in_out, loop_scopes); - - self.dfcx.apply_gen_kill(blk.id, in_out); - } - - fn walk_stmt(&mut self, - stmt: Gc, - in_out: &mut [uint], - loop_scopes: &mut Vec ) { - match stmt.node { - ast::StmtDecl(ref decl, _) => { - self.walk_decl(decl.clone(), in_out, loop_scopes); - } - - ast::StmtExpr(ref expr, _) | ast::StmtSemi(ref expr, _) => { - self.walk_expr(&**expr, in_out, loop_scopes); - } - - ast::StmtMac(..) => { - self.tcx().sess.span_bug(stmt.span, "unexpanded macro"); - } - } - } - - fn walk_decl(&mut self, - decl: Gc, - in_out: &mut [uint], - loop_scopes: &mut Vec ) { - match decl.node { - ast::DeclLocal(ref local) => { - self.walk_opt_expr(local.init, in_out, loop_scopes); - self.walk_pat(local.pat, in_out, loop_scopes); - } - - ast::DeclItem(_) => {} - } - } - - fn walk_expr(&mut self, - expr: &ast::Expr, - in_out: &mut [uint], - loop_scopes: &mut Vec ) { - debug!("DataFlowContext::walk_expr(expr={}, in_out={})", - expr.repr(self.dfcx.tcx), bits_to_str(in_out)); - - self.merge_with_entry_set(expr.id, in_out); - - match expr.node { - ast::ExprFnBlock(..) | ast::ExprProc(..) => { - } - - ast::ExprIf(cond, then, els) => { - // - // (cond) - // | - // v - // ( ) - // / \ - // | | - // v v - // (then)(els) - // | | - // v v - // ( succ ) - // - self.walk_expr(&*cond, in_out, loop_scopes); - - let mut then_bits = in_out.to_owned(); - self.walk_block(&*then, then_bits.as_mut_slice(), loop_scopes); - - self.walk_opt_expr(els, in_out, loop_scopes); - join_bits(&self.dfcx.oper, then_bits.as_slice(), in_out); - } - - ast::ExprWhile(cond, blk) => { - // - // (expr) <--+ - // | | - // v | - // +--(cond) | - // | | | - // | v | - // v (blk) ----+ - // | - // <--+ (break) - // - - self.walk_expr(&*cond, in_out, loop_scopes); - - let mut body_bits = in_out.to_owned(); - loop_scopes.push(LoopScope { - loop_id: expr.id, - break_bits: Vec::from_slice(in_out) - }); - self.walk_block(&*blk, body_bits.as_mut_slice(), loop_scopes); - self.add_to_entry_set(expr.id, body_bits.as_slice()); - let new_loop_scope = loop_scopes.pop().unwrap(); - copy_bits(new_loop_scope.break_bits.as_slice(), in_out); - } - - ast::ExprForLoop(..) => fail!("non-desugared expr_for_loop"), - - ast::ExprLoop(ref blk, _) => { - // - // (expr) <--+ - // | | - // v | - // (blk) ----+ - // | - // <--+ (break) - // - - let mut body_bits = in_out.to_owned(); - self.reset(in_out); - loop_scopes.push(LoopScope { - loop_id: expr.id, - break_bits: Vec::from_slice(in_out) - }); - self.walk_block(&**blk, body_bits.as_mut_slice(), loop_scopes); - self.add_to_entry_set(expr.id, body_bits.as_slice()); - - let new_loop_scope = loop_scopes.pop().unwrap(); - assert_eq!(new_loop_scope.loop_id, expr.id); - copy_bits(new_loop_scope.break_bits.as_slice(), in_out); - } - - ast::ExprMatch(ref discr, ref arms) => { - // - // (discr) - // / | \ - // | | | - // v v v - // (..arms..) - // | | | - // v v v - // ( succ ) - // - // - self.walk_expr(&**discr, in_out, loop_scopes); - - let mut guards = in_out.to_owned(); - - // We know that exactly one arm will be taken, so we - // can start out with a blank slate and just union - // together the bits from each arm: - self.reset(in_out); - - for arm in arms.iter() { - // in_out reflects the discr and all guards to date - self.walk_opt_expr(arm.guard, guards.as_mut_slice(), - loop_scopes); - - // determine the bits for the body and then union - // them into `in_out`, which reflects all bodies to date - let mut body = guards.to_owned(); - self.walk_pat_alternatives(arm.pats.as_slice(), - body.as_mut_slice(), - loop_scopes); - self.walk_expr(&*arm.body, body.as_mut_slice(), loop_scopes); - join_bits(&self.dfcx.oper, body.as_slice(), in_out); - } - } - - ast::ExprRet(o_e) => { - self.walk_opt_expr(o_e, in_out, loop_scopes); - self.reset(in_out); - } - - ast::ExprBreak(label) => { - let scope = self.find_scope(expr, label, loop_scopes); - self.break_from_to(expr, scope, in_out); - self.reset(in_out); - } - - ast::ExprAgain(label) => { - let scope = self.find_scope(expr, label, loop_scopes); - self.pop_scopes(expr, scope, in_out); - self.add_to_entry_set(scope.loop_id, in_out); - self.reset(in_out); - } - - ast::ExprAssign(ref l, ref r) | - ast::ExprAssignOp(_, ref l, ref r) => { - self.walk_expr(&**r, in_out, loop_scopes); - self.walk_expr(&**l, in_out, loop_scopes); - } - - ast::ExprVec(ref exprs) => { - self.walk_exprs(exprs.as_slice(), in_out, loop_scopes) - } - - ast::ExprRepeat(ref l, ref r) => { - self.walk_expr(&**l, in_out, loop_scopes); - self.walk_expr(&**r, in_out, loop_scopes); - } - - ast::ExprStruct(_, ref fields, with_expr) => { - for field in fields.iter() { - self.walk_expr(&*field.expr, in_out, loop_scopes); - } - self.walk_opt_expr(with_expr, in_out, loop_scopes); - } - - ast::ExprCall(ref f, ref args) => { - self.walk_expr(&**f, in_out, loop_scopes); - self.walk_call(expr.id, args.as_slice(), in_out, loop_scopes); - } - - ast::ExprMethodCall(_, _, ref args) => { - self.walk_call(expr.id, args.as_slice(), in_out, loop_scopes); - } - - ast::ExprIndex(l, r) | - ast::ExprBinary(_, l, r) if self.is_method_call(expr) => { - self.walk_call(expr.id, [l, r], in_out, loop_scopes); - } - - ast::ExprUnary(_, e) if self.is_method_call(expr) => { - self.walk_call(expr.id, [e], in_out, loop_scopes); - } - - ast::ExprTup(ref exprs) => { - self.walk_exprs(exprs.as_slice(), in_out, loop_scopes); - } - - ast::ExprBinary(op, ref l, ref r) if ast_util::lazy_binop(op) => { - self.walk_expr(&**l, in_out, loop_scopes); - let temp = in_out.to_owned(); - self.walk_expr(&**r, in_out, loop_scopes); - join_bits(&self.dfcx.oper, temp.as_slice(), in_out); - } - - ast::ExprIndex(l, r) | - ast::ExprBinary(_, l, r) => { - self.walk_exprs([l, r], in_out, loop_scopes); - } - - ast::ExprLit(..) | - ast::ExprPath(..) => {} - - ast::ExprAddrOf(_, ref e) | - ast::ExprCast(ref e, _) | - ast::ExprUnary(_, ref e) | - ast::ExprParen(ref e) | - ast::ExprVstore(ref e, _) | - ast::ExprField(ref e, _, _) => { - self.walk_expr(&**e, in_out, loop_scopes); - } - - ast::ExprBox(ref s, ref e) => { - self.walk_expr(&**s, in_out, loop_scopes); - self.walk_expr(&**e, in_out, loop_scopes); - } - - ast::ExprInlineAsm(ref inline_asm) => { - for &(_, ref expr) in inline_asm.inputs.iter() { - self.walk_expr(&**expr, in_out, loop_scopes); - } - for &(_, ref expr) in inline_asm.outputs.iter() { - self.walk_expr(&**expr, in_out, loop_scopes); - } - } - - ast::ExprBlock(ref blk) => { - self.walk_block(&**blk, in_out, loop_scopes); - } - - ast::ExprMac(..) => { - self.tcx().sess.span_bug(expr.span, "unexpanded macro"); - } - } - - self.dfcx.apply_gen_kill(expr.id, in_out); - } - - fn pop_scopes(&mut self, - from_expr: &ast::Expr, - to_scope: &mut LoopScope, - in_out: &mut [uint]) { - //! Whenever you have a `break` or a `loop` statement, flow - //! exits through any number of enclosing scopes on its - //! way to the new destination. This function applies the kill - //! sets of those enclosing scopes to `in_out` (those kill sets - //! concern items that are going out of scope). - - let tcx = self.tcx(); - - debug!("pop_scopes(from_expr={}, to_scope={:?}, in_out={})", - from_expr.repr(tcx), to_scope.loop_id, - bits_to_str(in_out)); - - let mut id = from_expr.id; - while id != to_scope.loop_id { - self.dfcx.apply_kill(id, in_out); - - match tcx.region_maps.opt_encl_scope(id) { - Some(i) => { id = i; } - None => { - tcx.sess.span_bug( - from_expr.span, - format!("pop_scopes(from_expr={}, to_scope={:?}) \ - to_scope does not enclose from_expr", - from_expr.repr(tcx), - to_scope.loop_id).as_slice()); - } - } - } - } - - fn break_from_to(&mut self, - from_expr: &ast::Expr, - to_scope: &mut LoopScope, - in_out: &mut [uint]) { - self.pop_scopes(from_expr, to_scope, in_out); - self.dfcx.apply_kill(from_expr.id, in_out); - join_bits(&self.dfcx.oper, - in_out, - to_scope.break_bits.as_mut_slice()); - debug!("break_from_to(from_expr={}, to_scope={}) final break_bits={}", - from_expr.repr(self.tcx()), - to_scope.loop_id, - bits_to_str(in_out)); - } - - fn walk_exprs(&mut self, - exprs: &[Gc], - in_out: &mut [uint], - loop_scopes: &mut Vec ) { - for expr in exprs.iter() { - self.walk_expr(&**expr, in_out, loop_scopes); - } - } - - fn walk_opt_expr(&mut self, - opt_expr: Option>, - in_out: &mut [uint], - loop_scopes: &mut Vec ) { - for expr in opt_expr.iter() { - self.walk_expr(&**expr, in_out, loop_scopes); - } - } - - fn walk_call(&mut self, - call_id: ast::NodeId, - args: &[Gc], - in_out: &mut [uint], - loop_scopes: &mut Vec ) { - self.walk_exprs(args, in_out, loop_scopes); - - // FIXME(#6268) nested method calls - // self.merge_with_entry_set(in_out); - // self.dfcx.apply_gen_kill(in_out); - - let return_ty = ty::node_id_to_type(self.tcx(), call_id); - let fails = ty::type_is_bot(return_ty); - if fails { - self.reset(in_out); - } - } - - fn walk_pat(&mut self, - pat: Gc, - in_out: &mut [uint], - _loop_scopes: &mut Vec ) { - debug!("DataFlowContext::walk_pat(pat={}, in_out={})", - pat.repr(self.dfcx.tcx), bits_to_str(in_out)); - - ast_util::walk_pat(&*pat, |p| { - debug!(" p.id={} in_out={}", p.id, bits_to_str(in_out)); - self.merge_with_entry_set(p.id, in_out); - self.dfcx.apply_gen_kill(p.id, in_out); - true + fn walk_cfg(&mut self, + cfg: &cfg::CFG, + in_out: &mut [uint]) { + debug!("DataFlowContext::walk_cfg(in_out={}) {:s}", + bits_to_str(in_out), self.dfcx.analysis_name); + cfg.graph.each_node(|node_index, node| { + debug!("DataFlowContext::walk_cfg idx={} id={} begin in_out={}", + node_index, node.data.id, bits_to_str(in_out)); + + let (start, end) = self.dfcx.compute_id_range(node_index); + + // Initialize local bitvector with state on-entry. + in_out.copy_from(self.dfcx.on_entry.slice(start, end)); + + // Compute state on-exit by applying transfer function to + // state on-entry. + self.dfcx.apply_gen_kill(node_index, in_out); + + // Propagate state on-exit from node into its successors. + self.propagate_bits_into_graph_successors_of(in_out, cfg, node_index); + true // continue to next node }); } - fn walk_pat_alternatives(&mut self, - pats: &[Gc], - in_out: &mut [uint], - loop_scopes: &mut Vec ) { - if pats.len() == 1 { - // Common special case: - return self.walk_pat(pats[0], in_out, loop_scopes); - } - - // In the general case, the patterns in `pats` are - // alternatives, so we must treat this like an N-way select - // statement. - let initial_state = in_out.to_owned(); - for &pat in pats.iter() { - let mut temp = initial_state.clone(); - self.walk_pat(pat, temp.as_mut_slice(), loop_scopes); - join_bits(&self.dfcx.oper, temp.as_slice(), in_out); - } - } - - fn find_scope<'a,'b>( - &self, - expr: &ast::Expr, - label: Option, - loop_scopes: &'a mut Vec>) - -> &'a mut LoopScope<'b> { - let index = match label { - None => { - let len = loop_scopes.len(); - len - 1 - } - - Some(_) => { - match self.tcx().def_map.borrow().find(&expr.id) { - Some(&def::DefLabel(loop_id)) => { - match loop_scopes.iter().position(|l| l.loop_id == loop_id) { - Some(i) => i, - None => { - self.tcx().sess.span_bug( - expr.span, - format!("no loop scope for id {:?}", - loop_id).as_slice()); - } - } - } - - r => { - self.tcx().sess.span_bug( - expr.span, - format!("bad entry `{:?}` in def_map for label", - r).as_slice()); - } - } - } - }; - - loop_scopes.get_mut(index) - } - - fn is_method_call(&self, expr: &ast::Expr) -> bool { - let method_call = typeck::MethodCall::expr(expr.id); - self.dfcx.tcx.method_map.borrow().contains_key(&method_call) - } - fn reset(&mut self, bits: &mut [uint]) { let e = if self.dfcx.oper.initial_value() {uint::MAX} else {0}; for b in bits.mut_iter() { @@ -797,36 +520,33 @@ impl<'a, 'b, O:DataFlowOperator> PropagationContext<'a, 'b, O> { } } - fn add_to_entry_set(&mut self, id: ast::NodeId, pred_bits: &[uint]) { - debug!("add_to_entry_set(id={:?}, pred_bits={})", - id, bits_to_str(pred_bits)); - let (start, end) = self.dfcx.compute_id_range(id); - let changed = { // FIXME(#5074) awkward construction - let on_entry = self.dfcx.on_entry.mut_slice(start, end); - join_bits(&self.dfcx.oper, pred_bits, on_entry) - }; - if changed { - debug!("changed entry set for {:?} to {}", - id, bits_to_str(self.dfcx.on_entry.slice(start, end))); - self.changed = true; - } + fn propagate_bits_into_graph_successors_of(&mut self, + pred_bits: &[uint], + cfg: &cfg::CFG, + cfgidx: CFGIndex) { + cfg.graph.each_outgoing_edge(cfgidx, |_e_idx, edge| { + self.propagate_bits_into_entry_set_for(pred_bits, edge); + true + }); } - fn merge_with_entry_set(&mut self, - id: ast::NodeId, - pred_bits: &mut [uint]) { - debug!("merge_with_entry_set(id={:?}, pred_bits={})", - id, mut_bits_to_str(pred_bits)); - let (start, end) = self.dfcx.compute_id_range(id); - let changed = { // FIXME(#5074) awkward construction + fn propagate_bits_into_entry_set_for(&mut self, + pred_bits: &[uint], + edge: &cfg::CFGEdge) { + let source = edge.source(); + let cfgidx = edge.target(); + debug!("{:s} propagate_bits_into_entry_set_for(pred_bits={}, {} to {})", + self.dfcx.analysis_name, bits_to_str(pred_bits), source, cfgidx); + let (start, end) = self.dfcx.compute_id_range(cfgidx); + let changed = { + // (scoping mutable borrow of self.dfcx.on_entry) let on_entry = self.dfcx.on_entry.mut_slice(start, end); - let changed = join_bits(&self.dfcx.oper, pred_bits, on_entry); - copy_bits(on_entry, pred_bits); - changed + bitwise(on_entry, pred_bits, &self.dfcx.oper) }; if changed { - debug!("changed entry set for {:?} to {}", - id, bits_to_str(self.dfcx.on_entry.slice(start, end))); + debug!("{:s} changed entry set for {:?} to {}", + self.dfcx.analysis_name, cfgidx, + bits_to_str(self.dfcx.on_entry.slice(start, end))); self.changed = true; } } @@ -855,24 +575,15 @@ fn bits_to_str(words: &[uint]) -> String { return result } -fn copy_bits(in_vec: &[uint], out_vec: &mut [uint]) -> bool { - bitwise(out_vec, in_vec, |_, b| b) -} - -fn join_bits(oper: &O, - in_vec: &[uint], - out_vec: &mut [uint]) -> bool { - bitwise(out_vec, in_vec, |a, b| oper.join(a, b)) -} - #[inline] -fn bitwise(out_vec: &mut [uint], in_vec: &[uint], op: |uint, uint| -> uint) - -> bool { +fn bitwise(out_vec: &mut [uint], + in_vec: &[uint], + op: &Op) -> bool { assert_eq!(out_vec.len(), in_vec.len()); let mut changed = false; for (out_elt, in_elt) in out_vec.mut_iter().zip(in_vec.iter()) { let old_val = *out_elt; - let new_val = op(old_val, *in_elt); + let new_val = op.join(old_val, *in_elt); *out_elt = new_val; changed |= old_val != new_val; } @@ -897,3 +608,12 @@ fn bit_str(bit: uint) -> String { let lobits = 1 << (bit & 0xFF); format!("[{}:{}-{:02x}]", bit, byte, lobits) } + +struct Union; +impl BitwiseOperator for Union { + fn join(&self, a: uint, b: uint) -> uint { a | b } +} +struct Subtract; +impl BitwiseOperator for Subtract { + fn join(&self, a: uint, b: uint) -> uint { a & !b } +} diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index 86cd3c53804de..ad058ab6b5f1d 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -186,24 +186,7 @@ impl<'d,'t,TYPER:mc::Typer> ExprUseVisitor<'d,'t,TYPER> { let cmt = return_if_err!(self.mc.cat_expr(expr)); self.delegate_consume(expr.id, expr.span, cmt); - - match expr.node { - ast::ExprParen(ref subexpr) => { - // Argh but is ExprParen horrible. So, if we consume - // `(x)`, that generally is also consuming `x`, UNLESS - // there are adjustments on the `(x)` expression - // (e.g., autoderefs and autorefs). - if self.typer.adjustments().borrow().contains_key(&expr.id) { - self.walk_expr(expr); - } else { - self.consume_expr(&**subexpr); - } - } - - _ => { - self.walk_expr(expr) - } - } + self.walk_expr(expr); } fn mutate_expr(&mut self, diff --git a/src/librustc/middle/graph.rs b/src/librustc/middle/graph.rs index 3d5e4b167a330..b1f9b0bff9fd2 100644 --- a/src/librustc/middle/graph.rs +++ b/src/librustc/middle/graph.rs @@ -55,7 +55,7 @@ pub struct Edge { pub data: E, } -#[deriving(PartialEq, Show)] +#[deriving(Clone, PartialEq, Show)] pub struct NodeIndex(pub uint); pub static InvalidNodeIndex: NodeIndex = NodeIndex(uint::MAX); From 4d82456f69d858bb8ebd7bfc508365cf983eea54 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 21 May 2014 14:48:33 +0200 Subject: [PATCH 4/9] middle::cfg code cleanup. Namely: 1. Now that cfg mod is used for dataflow, we do not need to turn on the `allow(deadcode)` to placate the linter. 2. remove dead struct defn. --- src/librustc/middle/cfg/mod.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/librustc/middle/cfg/mod.rs b/src/librustc/middle/cfg/mod.rs index f0b912fb87bbf..bb758ec7c38b7 100644 --- a/src/librustc/middle/cfg/mod.rs +++ b/src/librustc/middle/cfg/mod.rs @@ -15,8 +15,6 @@ Uses `Graph` as the underlying representation. */ -#![allow(dead_code)] // still a WIP, #6298 - use middle::graph; use middle::ty; use syntax::ast; @@ -48,11 +46,6 @@ pub type CFGNode = graph::Node; pub type CFGEdge = graph::Edge; -pub struct CFGIndices { - entry: CFGIndex, - exit: CFGIndex, -} - impl CFG { pub fn new(tcx: &ty::ctxt, blk: &ast::Block) -> CFG { From 263a433f1901592f91e6433ccc79539e619cd95f Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Fri, 13 Jun 2014 17:19:29 +0200 Subject: [PATCH 5/9] Ensure dataflow of a proc never looks at blocks from closed-over context. Details: in a program like: ``` type T = proc(int) -> int; /* 4 */ pub fn outer(captured /* pat 16 */: T) -> T { (proc(x /* pat 23 */) { ((captured /* 29 */).foo((x /* 30 */)) /* 28 */) } /* block 27 */ /* 20 */) } /* block 19 */ /* 12 */ ``` the `captured` arg is moved from the outer fn into the inner proc (id=20). The old dataflow analysis for flowed_move_data_moves, when looking at the inner proc, would attempt to add a kill bit for `captured` at the end of its scope; the problem is that it thought the end of the `captured` arg's scope was the outer fn (id=12), even though at that point in the analysis, the `captured` arg's scope should now be restricted to the proc itself (id=20). This patch fixes handling of upvars so that dataflow of a fn/proc should never attempts to add a gen or kill bit to any NodeId outside of the current fn/proc. It accomplishes this by adding an `LpUpvar` variant to `borrowck::LoanPath`, so for cases like `captured` above will carry both their original `var_id`, as before, as well as the `NodeId` for the closure that is capturing them. As a drive-by fix to another occurrence of a similar bug that nikomatsakis pointed out to me earlier, this also fixes `gather_loans::compute_kill_scope` so that it computes the kill scope of the `captured` arg to be block 27; that is, the block for the proc itself (id=20). (This is an updated version that generalizes the new loan path variant to cover all upvars, and thus renamed the variant from `LpCopiedUpvar` to just `LpUpvar`.) --- src/librustc/middle/borrowck/check_loans.rs | 4 +- .../middle/borrowck/gather_loans/mod.rs | 5 ++- .../borrowck/gather_loans/restrictions.rs | 17 ++++++-- src/librustc/middle/borrowck/mod.rs | 42 +++++++++++++++---- src/librustc/middle/borrowck/move_data.rs | 13 +++++- src/librustc/middle/dataflow.rs | 38 +++++++---------- src/librustc/middle/mem_categorization.rs | 5 ++- 7 files changed, 82 insertions(+), 42 deletions(-) diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 166d069880f9f..df208b9cdc133 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -242,7 +242,7 @@ impl<'a> CheckLoanCtxt<'a> { let mut loan_path = loan_path; loop { match *loan_path { - LpVar(_) => { + LpVar(_) | LpUpvar(_) => { break; } LpExtend(ref lp_base, _, _) => { @@ -632,7 +632,7 @@ impl<'a> CheckLoanCtxt<'a> { */ match **lp { - LpVar(_) => { + LpVar(_) | LpUpvar(_) => { // assigning to `x` does not require that `x` is initialized } LpExtend(ref lp_base, _, LpInterior(_)) => { diff --git a/src/librustc/middle/borrowck/gather_loans/mod.rs b/src/librustc/middle/borrowck/gather_loans/mod.rs index 89f304513ffb3..454c3dcd5d3ca 100644 --- a/src/librustc/middle/borrowck/gather_loans/mod.rs +++ b/src/librustc/middle/borrowck/gather_loans/mod.rs @@ -395,7 +395,8 @@ impl<'a> GatherLoanCtxt<'a> { //! from a local variable, mark the mutability decl as necessary. match *loan_path { - LpVar(local_id) => { + LpVar(local_id) | + LpUpvar(ty::UpvarId{ var_id: local_id, closure_expr_id: _ }) => { self.tcx().used_mut_nodes.borrow_mut().insert(local_id); } LpExtend(ref base, mc::McInherited, _) => { @@ -445,8 +446,8 @@ impl<'a> GatherLoanCtxt<'a> { //! with immutable `&` pointers, because borrows of such pointers //! do not require restrictions and hence do not cause a loan. + let lexical_scope = lp.kill_scope(self.bccx.tcx); let rm = &self.bccx.tcx.region_maps; - let lexical_scope = rm.var_scope(lp.node_id()); if rm.is_subscope_of(lexical_scope, loan_scope) { lexical_scope } else { diff --git a/src/librustc/middle/borrowck/gather_loans/restrictions.rs b/src/librustc/middle/borrowck/gather_loans/restrictions.rs index 5b3e1ac0b2c74..d131b6f7eda29 100644 --- a/src/librustc/middle/borrowck/gather_loans/restrictions.rs +++ b/src/librustc/middle/borrowck/gather_loans/restrictions.rs @@ -67,13 +67,23 @@ impl<'a> RestrictionsContext<'a> { } mc::cat_local(local_id) | - mc::cat_arg(local_id) | - mc::cat_upvar(ty::UpvarId {var_id: local_id, ..}, _) => { - // R-Variable + mc::cat_arg(local_id) => { + // R-Variable, locally declared let lp = Rc::new(LpVar(local_id)); SafeIf(lp.clone(), vec!(lp)) } + mc::cat_upvar(upvar_id, _) => { + // R-Variable, captured into closure + let lp = Rc::new(LpUpvar(upvar_id)); + SafeIf(lp.clone(), vec!(lp)) + } + + mc::cat_copied_upvar(..) => { + // FIXME(#2152) allow mutation of upvars + Safe + } + mc::cat_downcast(cmt_base) => { // When we borrow the interior of an enum, we have to // ensure the enum itself is not mutated, because that @@ -107,7 +117,6 @@ impl<'a> RestrictionsContext<'a> { self.extend(result, cmt.mutbl, LpDeref(pk)) } - mc::cat_copied_upvar(..) | // FIXME(#2152) allow mutation of upvars mc::cat_static_item(..) => { Safe } diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs index 98edc6365cf32..0c77e63779074 100644 --- a/src/librustc/middle/borrowck/mod.rs +++ b/src/librustc/middle/borrowck/mod.rs @@ -201,6 +201,7 @@ pub struct Loan { #[deriving(PartialEq, Eq, Hash)] pub enum LoanPath { LpVar(ast::NodeId), // `x` in doc.rs + LpUpvar(ty::UpvarId), // `x` captured by-value into closure LpExtend(Rc, mc::MutabilityCategory, LoanPathElem) } @@ -210,11 +211,25 @@ pub enum LoanPathElem { LpInterior(mc::InteriorKind) // `LV.f` in doc.rs } +pub fn closure_to_block(closure_id: ast::NodeId, + tcx: &ty::ctxt) -> ast::NodeId { + match tcx.map.get(closure_id) { + ast_map::NodeExpr(expr) => match expr.node { + ast::ExprProc(_decl, block) | + ast::ExprFnBlock(_decl, block) => { block.id } + _ => fail!("encountered non-closure id: {}", closure_id) + }, + _ => fail!("encountered non-expr id: {}", closure_id) + } +} + impl LoanPath { - pub fn node_id(&self) -> ast::NodeId { + pub fn kill_scope(&self, tcx: &ty::ctxt) -> ast::NodeId { match *self { - LpVar(local_id) => local_id, - LpExtend(ref base, _, _) => base.node_id() + LpVar(local_id) => tcx.region_maps.var_scope(local_id), + LpUpvar(upvar_id) => + closure_to_block(upvar_id.closure_expr_id, tcx), + LpExtend(ref base, _, _) => base.kill_scope(tcx), } } } @@ -234,12 +249,18 @@ pub fn opt_loan_path(cmt: &mc::cmt) -> Option> { } mc::cat_local(id) | - mc::cat_arg(id) | - mc::cat_copied_upvar(mc::CopiedUpvar { upvar_id: id, .. }) | - mc::cat_upvar(ty::UpvarId {var_id: id, ..}, _) => { + mc::cat_arg(id) => { Some(Rc::new(LpVar(id))) } + mc::cat_upvar(ty::UpvarId {var_id: id, closure_expr_id: proc_id}, _) | + mc::cat_copied_upvar(mc::CopiedUpvar { upvar_id: id, + onceness: _, + capturing_proc: proc_id }) => { + let upvar_id = ty::UpvarId{ var_id: id, closure_expr_id: proc_id }; + Some(Rc::new(LpUpvar(upvar_id))) + } + mc::cat_deref(ref cmt_base, _, pk) => { opt_loan_path(cmt_base).map(|lp| { Rc::new(LpExtend(lp, cmt.mutbl, LpDeref(pk))) @@ -693,6 +714,7 @@ impl<'a> BorrowckCtxt<'a> { loan_path: &LoanPath, out: &mut String) { match *loan_path { + LpUpvar(ty::UpvarId{ var_id: id, closure_expr_id: _ }) | LpVar(id) => { out.push_str(ty::local_var_name_str(self.tcx, id).get()); } @@ -734,7 +756,7 @@ impl<'a> BorrowckCtxt<'a> { self.append_autoderefd_loan_path_to_str(&**lp_base, out) } - LpVar(..) | LpExtend(_, _, LpInterior(..)) => { + LpVar(..) | LpUpvar(..) | LpExtend(_, _, LpInterior(..)) => { self.append_loan_path_to_str(loan_path, out) } } @@ -796,6 +818,12 @@ impl Repr for LoanPath { (format!("$({})", tcx.map.node_to_str(id))).to_string() } + &LpUpvar(ty::UpvarId{ var_id, closure_expr_id }) => { + let s = tcx.map.node_to_str(var_id); + let s = format!("$({} captured by id={})", s, closure_expr_id); + s.to_string() + } + &LpExtend(ref lp, _, LpDeref(_)) => { (format!("{}.*", lp.repr(tcx))).to_string() } diff --git a/src/librustc/middle/borrowck/move_data.rs b/src/librustc/middle/borrowck/move_data.rs index 3e2f763a84bc2..bb92043b1ea6a 100644 --- a/src/librustc/middle/borrowck/move_data.rs +++ b/src/librustc/middle/borrowck/move_data.rs @@ -231,7 +231,7 @@ impl MoveData { } let index = match *lp { - LpVar(..) => { + LpVar(..) | LpUpvar(..) => { let index = MovePathIndex(self.paths.borrow().len()); self.paths.borrow_mut().push(MovePath { @@ -302,7 +302,7 @@ impl MoveData { } None => { match **lp { - LpVar(..) => { } + LpVar(..) | LpUpvar(..) => { } LpExtend(ref b, _, _) => { self.add_existing_base_paths(b, result); } @@ -418,6 +418,11 @@ impl MoveData { let path = *self.path_map.borrow().get(&path.loan_path); self.kill_moves(path, kill_id, dfcx_moves); } + LpUpvar(ty::UpvarId { var_id: _, closure_expr_id }) => { + let kill_id = closure_to_block(closure_expr_id, tcx); + let path = *self.path_map.borrow().get(&path.loan_path); + self.kill_moves(path, kill_id, dfcx_moves); + } LpExtend(..) => {} } } @@ -430,6 +435,10 @@ impl MoveData { let kill_id = tcx.region_maps.var_scope(id); dfcx_assign.add_kill(kill_id, assignment_index); } + LpUpvar(ty::UpvarId { var_id: _, closure_expr_id }) => { + let kill_id = closure_to_block(closure_expr_id, tcx); + dfcx_assign.add_kill(kill_id, assignment_index); + } LpExtend(..) => { tcx.sess.bug("var assignment for non var path"); } diff --git a/src/librustc/middle/dataflow.rs b/src/librustc/middle/dataflow.rs index 872a0878e3792..7a26d2104826f 100644 --- a/src/librustc/middle/dataflow.rs +++ b/src/librustc/middle/dataflow.rs @@ -266,34 +266,24 @@ impl<'a, O:DataFlowOperator> DataFlowContext<'a, O> { pub fn add_gen(&mut self, id: ast::NodeId, bit: uint) { //! Indicates that `id` generates `bit` - if self.nodeid_to_index.contains_key(&id) { - debug!("add_gen(id={:?}, bit={:?})", id, bit); - let cfgidx = to_cfgidx_or_die(id, &self.nodeid_to_index); - let (start, end) = self.compute_id_range(cfgidx); - { - let gens = self.gens.mut_slice(start, end); - set_bit(gens, bit); - } - } else { - debug!("{:s} add_gen skip (id={:?}, bit={:?}); id not in current fn", - self.analysis_name, id, bit); - } + debug!("{:s} add_gen(id={:?}, bit={:?})", + self.analysis_name, id, bit); + assert!(self.nodeid_to_index.contains_key(&id)); + let cfgidx = to_cfgidx_or_die(id, &self.nodeid_to_index); + let (start, end) = self.compute_id_range(cfgidx); + let gens = self.gens.mut_slice(start, end); + set_bit(gens, bit); } pub fn add_kill(&mut self, id: ast::NodeId, bit: uint) { //! Indicates that `id` kills `bit` - if self.nodeid_to_index.contains_key(&id) { - debug!("add_kill(id={:?}, bit={:?})", id, bit); - let cfgidx = to_cfgidx_or_die(id, &self.nodeid_to_index); - let (start, end) = self.compute_id_range(cfgidx); - { - let kills = self.kills.mut_slice(start, end); - set_bit(kills, bit); - } - } else { - debug!("{:s} add_kill skip (id={:?}, bit={:?}); id not in current fn", - self.analysis_name, id, bit); - } + debug!("{:s} add_kill(id={:?}, bit={:?})", + self.analysis_name, id, bit); + assert!(self.nodeid_to_index.contains_key(&id)); + let cfgidx = to_cfgidx_or_die(id, &self.nodeid_to_index); + let (start, end) = self.compute_id_range(cfgidx); + let kills = self.kills.mut_slice(start, end); + set_bit(kills, bit); } fn apply_gen_kill(&mut self, cfgidx: CFGIndex, bits: &mut [uint]) { diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index 8224557f86007..03c9c56c78701 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -97,6 +97,7 @@ pub enum categorization { pub struct CopiedUpvar { pub upvar_id: ast::NodeId, pub onceness: ast::Onceness, + pub capturing_proc: ast::NodeId, } // different kinds of pointers: @@ -559,7 +560,9 @@ impl<'t,TYPER:Typer> MemCategorizationContext<'t,TYPER> { span:span, cat:cat_copied_upvar(CopiedUpvar { upvar_id: var_id, - onceness: closure_ty.onceness}), + onceness: closure_ty.onceness, + capturing_proc: fn_node_id, + }), mutbl:McImmutable, ty:expr_ty })) From 95fdbeee48163691cf66de4a53fdc108157babcf Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 10 Jun 2014 14:22:48 +0200 Subject: [PATCH 6/9] fix typo in borrowck doc. --- src/librustc/middle/borrowck/doc.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/middle/borrowck/doc.rs b/src/librustc/middle/borrowck/doc.rs index 2427df463bfe4..9fc291d397191 100644 --- a/src/librustc/middle/borrowck/doc.rs +++ b/src/librustc/middle/borrowck/doc.rs @@ -948,7 +948,7 @@ The borrow checker is also in charge of ensuring that: These are two separate dataflow analyses built on the same framework. Let's look at checking that memory is initialized first; -the checking of immutable local variabe assignments works in a very +the checking of immutable local variable assignments works in a very similar way. To track the initialization of memory, we actually track all the From 373b0fc56905c17d14438446e16712bf714c6f08 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 17 Jun 2014 14:52:11 +0200 Subject: [PATCH 7/9] some extra test cases to cover in the borrow checker. --- .../loop-no-reinit-needed-post-bot.rs | 41 +++++++++++++++++++ src/test/run-pass/struct-partial-move-1.rs | 30 ++++++++++++++ src/test/run-pass/struct-partial-move-2.rs | 37 +++++++++++++++++ 3 files changed, 108 insertions(+) create mode 100644 src/test/run-pass/loop-no-reinit-needed-post-bot.rs create mode 100644 src/test/run-pass/struct-partial-move-1.rs create mode 100644 src/test/run-pass/struct-partial-move-2.rs diff --git a/src/test/run-pass/loop-no-reinit-needed-post-bot.rs b/src/test/run-pass/loop-no-reinit-needed-post-bot.rs new file mode 100644 index 0000000000000..8b77500225022 --- /dev/null +++ b/src/test/run-pass/loop-no-reinit-needed-post-bot.rs @@ -0,0 +1,41 @@ +// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +struct S; +// Ensure S is moved, not copied, on assignment. +impl Drop for S { fn drop(&mut self) { } } + +// user-defined function "returning" bottom (i.e. no return at all). +fn my_fail() -> ! { loop {} } + +pub fn step(f: bool) { + let mut g = S; + let mut i = 0; + loop + { + if i > 10 { break; } else { i += 1; } + + let _g = g; + + if f { + // re-initialize g, but only before restarting loop. + g = S; + continue; + } + + my_fail(); + + // we never get here, so we do not need to re-initialize g. + } +} + +pub fn main() { + step(true); +} diff --git a/src/test/run-pass/struct-partial-move-1.rs b/src/test/run-pass/struct-partial-move-1.rs new file mode 100644 index 0000000000000..7e02d10208182 --- /dev/null +++ b/src/test/run-pass/struct-partial-move-1.rs @@ -0,0 +1,30 @@ +// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[deriving(PartialEq, Show)] +struct Partial { x: T, y: T } + +#[deriving(PartialEq, Show)] +struct S { val: int } +impl S { fn new(v: int) -> S { S { val: v } } } +impl Drop for S { fn drop(&mut self) { } } + +pub fn f((b1, b2): (T, T), f: |T| -> T) -> Partial { + let p = Partial { x: b1, y: b2 }; + + // Move of `p` is legal even though we are also moving `p.y`; the + // `..p` moves all fields *except* `p.y` in this context. + Partial { y: f(p.y), ..p } +} + +pub fn main() { + let p = f((S::new(3), S::new(4)), |S { val: z }| S::new(z+1)); + assert_eq!(p, Partial { x: S::new(3), y: S::new(5) }); +} diff --git a/src/test/run-pass/struct-partial-move-2.rs b/src/test/run-pass/struct-partial-move-2.rs new file mode 100644 index 0000000000000..fe5e1eaaa1af5 --- /dev/null +++ b/src/test/run-pass/struct-partial-move-2.rs @@ -0,0 +1,37 @@ +// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[deriving(PartialEq, Show)] +struct Partial { x: T, y: T } + +#[deriving(PartialEq, Show)] +struct S { val: int } +impl S { fn new(v: int) -> S { S { val: v } } } +impl Drop for S { fn drop(&mut self) { } } + +type Two = (Partial, Partial); + +pub fn f((b1, b2): (T, T), (b3, b4): (T, T), f: |T| -> T) -> Two { + let p = Partial { x: b1, y: b2 }; + let q = Partial { x: b3, y: b4 }; + + // Move of `q` is legal even though we have already moved `q.y`; + // the `..q` moves all fields *except* `q.y` in this context. + // Likewise, the move of `p.x` is legal for similar reasons. + (Partial { x: f(q.y), ..p }, Partial { y: f(p.x), ..q }) +} + +pub fn main() { + let two = f((S::new(1), S::new(3)), + (S::new(5), S::new(7)), + |S { val: z }| S::new(z+1)); + assert_eq!(two, (Partial { x: S::new(8), y: S::new(3) }, + Partial { x: S::new(5), y: S::new(2) })); +} From 3ddb987f45c9126009b1c20cd1cc108b9de9c734 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 17 Jun 2014 15:38:50 +0200 Subject: [PATCH 8/9] Regression tests for flowgraph construction bug on ExprWhile. --- src/test/run-make/graphviz-flowgraph/Makefile | 3 +- .../graphviz-flowgraph/f23.dot-expected.dot | 93 +++++++++++++ src/test/run-make/graphviz-flowgraph/f23.rs | 31 +++++ .../graphviz-flowgraph/f24.dot-expected.dot | 123 ++++++++++++++++++ src/test/run-make/graphviz-flowgraph/f24.rs | 36 +++++ .../graphviz-flowgraph/f25.dot-expected.dot | 123 ++++++++++++++++++ src/test/run-make/graphviz-flowgraph/f25.rs | 36 +++++ 7 files changed, 444 insertions(+), 1 deletion(-) create mode 100644 src/test/run-make/graphviz-flowgraph/f23.dot-expected.dot create mode 100644 src/test/run-make/graphviz-flowgraph/f23.rs create mode 100644 src/test/run-make/graphviz-flowgraph/f24.dot-expected.dot create mode 100644 src/test/run-make/graphviz-flowgraph/f24.rs create mode 100644 src/test/run-make/graphviz-flowgraph/f25.dot-expected.dot create mode 100644 src/test/run-make/graphviz-flowgraph/f25.rs diff --git a/src/test/run-make/graphviz-flowgraph/Makefile b/src/test/run-make/graphviz-flowgraph/Makefile index fedcc89cd429f..09440949177dd 100644 --- a/src/test/run-make/graphviz-flowgraph/Makefile +++ b/src/test/run-make/graphviz-flowgraph/Makefile @@ -2,7 +2,8 @@ FILES=f00.rs f01.rs f02.rs f03.rs f04.rs f05.rs f06.rs f07.rs \ f08.rs f09.rs f10.rs f11.rs f12.rs f13.rs f14.rs f15.rs \ - f16.rs f17.rs f18.rs f19.rs f20.rs f21.rs f22.rs + f16.rs f17.rs f18.rs f19.rs f20.rs f21.rs f22.rs f23.rs \ + f24.rs f25.rs # all: $(patsubst %.rs,$(TMPDIR)/%.dot,$(FILES)) $(patsubst %.rs,$(TMPDIR)/%.pp,$(FILES)) diff --git a/src/test/run-make/graphviz-flowgraph/f23.dot-expected.dot b/src/test/run-make/graphviz-flowgraph/f23.dot-expected.dot new file mode 100644 index 0000000000000..876957a0689d6 --- /dev/null +++ b/src/test/run-make/graphviz-flowgraph/f23.dot-expected.dot @@ -0,0 +1,93 @@ +digraph block { + N0[label="entry"]; + N1[label="exit"]; + N2[label="expr 23"]; + N3[label="local mut x"]; + N4[label="expr 23"]; + N5[label="local mut y"]; + N6[label="expr 23"]; + N7[label="local mut z"]; + N8[label="(dummy_node)"]; + N9[label="expr x"]; + N10[label="expr 0"]; + N11[label="expr x > 0"]; + N12[label="expr while x > 0 {\l x -= 1;\l while y > 0 {\l y -= 1;\l while z > 0 { z -= 1; }\l if x > 10 { return; \"unreachable\"; }\l }\l}\l"]; + N13[label="expr 1"]; + N14[label="expr x"]; + N15[label="expr x -= 1"]; + N16[label="(dummy_node)"]; + N17[label="expr y"]; + N18[label="expr 0"]; + N19[label="expr y > 0"]; + N20[label="expr while y > 0 {\l y -= 1;\l while z > 0 { z -= 1; }\l if x > 10 { return; \"unreachable\"; }\l}\l"]; + N21[label="expr 1"]; + N22[label="expr y"]; + N23[label="expr y -= 1"]; + N24[label="(dummy_node)"]; + N25[label="expr z"]; + N26[label="expr 0"]; + N27[label="expr z > 0"]; + N28[label="expr while z > 0 { z -= 1; }"]; + N29[label="expr 1"]; + N30[label="expr z"]; + N31[label="expr z -= 1"]; + N32[label="block { z -= 1; }"]; + N33[label="expr x"]; + N34[label="expr 10"]; + N35[label="expr x > 10"]; + N36[label="expr return"]; + N37[label="(dummy_node)"]; + N38[label="expr \"unreachable\""]; + N39[label="block { return; \"unreachable\"; }"]; + N40[label="expr if x > 10 { return; \"unreachable\"; }"]; + N41[label="block { y -= 1; while z > 0 { z -= 1; } if x > 10 { return; \"unreachable\"; } }"]; + N42[label="block {\l x -= 1;\l while y > 0 {\l y -= 1;\l while z > 0 { z -= 1; }\l if x > 10 { return; \"unreachable\"; }\l }\l}\l"]; + N43[label="block {\l let mut x = 23;\l let mut y = 23;\l let mut z = 23;\l while x > 0 {\l x -= 1;\l while y > 0 {\l y -= 1;\l while z > 0 { z -= 1; }\l if x > 10 { return; \"unreachable\"; }\l }\l }\l}\l"]; + N0 -> N2; + N2 -> N3; + N3 -> N4; + N4 -> N5; + N5 -> N6; + N6 -> N7; + N7 -> N8; + N8 -> N9; + N9 -> N10; + N10 -> N11; + N11 -> N12; + N11 -> N13; + N13 -> N14; + N14 -> N15; + N15 -> N16; + N16 -> N17; + N17 -> N18; + N18 -> N19; + N19 -> N20; + N19 -> N21; + N21 -> N22; + N22 -> N23; + N23 -> N24; + N24 -> N25; + N25 -> N26; + N26 -> N27; + N27 -> N28; + N27 -> N29; + N29 -> N30; + N30 -> N31; + N31 -> N32; + N32 -> N24; + N28 -> N33; + N33 -> N34; + N34 -> N35; + N35 -> N36; + N36 -> N1[label="exiting scope_0 expr while y > 0 {\l y -= 1;\l while z > 0 { z -= 1; }\l if x > 10 { return; \"unreachable\"; }\l}\l,\lexiting scope_1 expr while x > 0 {\l x -= 1;\l while y > 0 {\l y -= 1;\l while z > 0 { z -= 1; }\l if x > 10 { return; \"unreachable\"; }\l }\l}\l"]; + N37 -> N38; + N38 -> N39; + N35 -> N40; + N39 -> N40; + N40 -> N41; + N41 -> N16; + N20 -> N42; + N42 -> N8; + N12 -> N43; + N43 -> N1; +} diff --git a/src/test/run-make/graphviz-flowgraph/f23.rs b/src/test/run-make/graphviz-flowgraph/f23.rs new file mode 100644 index 0000000000000..52341a3fbd408 --- /dev/null +++ b/src/test/run-make/graphviz-flowgraph/f23.rs @@ -0,0 +1,31 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[allow(unreachable_code)] +pub fn expr_while_23() { + let mut x = 23; + let mut y = 23; + let mut z = 23; + + while x > 0 { + x -= 1; + + while y > 0 { + y -= 1; + + while z > 0 { z -= 1; } + + if x > 10 { + return; + "unreachable"; + } + } + } +} diff --git a/src/test/run-make/graphviz-flowgraph/f24.dot-expected.dot b/src/test/run-make/graphviz-flowgraph/f24.dot-expected.dot new file mode 100644 index 0000000000000..2558998be6e1f --- /dev/null +++ b/src/test/run-make/graphviz-flowgraph/f24.dot-expected.dot @@ -0,0 +1,123 @@ +digraph block { + N0[label="entry"]; + N1[label="exit"]; + N2[label="expr 24"]; + N3[label="local mut x"]; + N4[label="expr 24"]; + N5[label="local mut y"]; + N6[label="expr 24"]; + N7[label="local mut z"]; + N8[label="(dummy_node)"]; + N9[label="expr loop {\l if x == 0 { break ; \"unreachable\"; }\l x -= 1;\l loop {\l if y == 0 { break ; \"unreachable\"; }\l y -= 1;\l loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }\l if x > 10 { return; \"unreachable\"; }\l }\l}\l"]; + N10[label="expr x"]; + N11[label="expr 0"]; + N12[label="expr x == 0"]; + N13[label="expr break"]; + N14[label="(dummy_node)"]; + N15[label="expr \"unreachable\""]; + N16[label="block { break ; \"unreachable\"; }"]; + N17[label="expr if x == 0 { break ; \"unreachable\"; }"]; + N18[label="expr 1"]; + N19[label="expr x"]; + N20[label="expr x -= 1"]; + N21[label="(dummy_node)"]; + N22[label="expr loop {\l if y == 0 { break ; \"unreachable\"; }\l y -= 1;\l loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }\l if x > 10 { return; \"unreachable\"; }\l}\l"]; + N23[label="expr y"]; + N24[label="expr 0"]; + N25[label="expr y == 0"]; + N26[label="expr break"]; + N27[label="(dummy_node)"]; + N28[label="expr \"unreachable\""]; + N29[label="block { break ; \"unreachable\"; }"]; + N30[label="expr if y == 0 { break ; \"unreachable\"; }"]; + N31[label="expr 1"]; + N32[label="expr y"]; + N33[label="expr y -= 1"]; + N34[label="(dummy_node)"]; + N35[label="expr loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }"]; + N36[label="expr z"]; + N37[label="expr 0"]; + N38[label="expr z == 0"]; + N39[label="expr break"]; + N40[label="(dummy_node)"]; + N41[label="expr \"unreachable\""]; + N42[label="block { break ; \"unreachable\"; }"]; + N43[label="expr if z == 0 { break ; \"unreachable\"; }"]; + N44[label="expr 1"]; + N45[label="expr z"]; + N46[label="expr z -= 1"]; + N47[label="block { if z == 0 { break ; \"unreachable\"; } z -= 1; }"]; + N48[label="expr x"]; + N49[label="expr 10"]; + N50[label="expr x > 10"]; + N51[label="expr return"]; + N52[label="(dummy_node)"]; + N53[label="expr \"unreachable\""]; + N54[label="block { return; \"unreachable\"; }"]; + N55[label="expr if x > 10 { return; \"unreachable\"; }"]; + N56[label="block {\l if y == 0 { break ; \"unreachable\"; }\l y -= 1;\l loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }\l if x > 10 { return; \"unreachable\"; }\l}\l"]; + N57[label="block {\l if x == 0 { break ; \"unreachable\"; }\l x -= 1;\l loop {\l if y == 0 { break ; \"unreachable\"; }\l y -= 1;\l loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }\l if x > 10 { return; \"unreachable\"; }\l }\l}\l"]; + N58[label="block {\l let mut x = 24;\l let mut y = 24;\l let mut z = 24;\l loop {\l if x == 0 { break ; \"unreachable\"; }\l x -= 1;\l loop {\l if y == 0 { break ; \"unreachable\"; }\l y -= 1;\l loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }\l if x > 10 { return; \"unreachable\"; }\l }\l }\l}\l"]; + N0 -> N2; + N2 -> N3; + N3 -> N4; + N4 -> N5; + N5 -> N6; + N6 -> N7; + N7 -> N8; + N8 -> N10; + N10 -> N11; + N11 -> N12; + N12 -> N13; + N13 -> N9[label="exiting scope_0 expr break,\lexiting scope_1 stmt break ;,\lexiting scope_2 block { break ; \"unreachable\"; },\lexiting scope_3 expr if x == 0 { break ; \"unreachable\"; },\lexiting scope_4 stmt if x == 0 { break ; \"unreachable\"; },\lexiting scope_5 block {\l if x == 0 { break ; \"unreachable\"; }\l x -= 1;\l loop {\l if y == 0 { break ; \"unreachable\"; }\l y -= 1;\l loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }\l if x > 10 { return; \"unreachable\"; }\l }\l}\l"]; + N14 -> N15; + N15 -> N16; + N12 -> N17; + N16 -> N17; + N17 -> N18; + N18 -> N19; + N19 -> N20; + N20 -> N21; + N21 -> N23; + N23 -> N24; + N24 -> N25; + N25 -> N26; + N26 -> N22[label="exiting scope_0 expr break,\lexiting scope_1 stmt break ;,\lexiting scope_2 block { break ; \"unreachable\"; },\lexiting scope_3 expr if y == 0 { break ; \"unreachable\"; },\lexiting scope_4 stmt if y == 0 { break ; \"unreachable\"; },\lexiting scope_5 block {\l if y == 0 { break ; \"unreachable\"; }\l y -= 1;\l loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }\l if x > 10 { return; \"unreachable\"; }\l}\l"]; + N27 -> N28; + N28 -> N29; + N25 -> N30; + N29 -> N30; + N30 -> N31; + N31 -> N32; + N32 -> N33; + N33 -> N34; + N34 -> N36; + N36 -> N37; + N37 -> N38; + N38 -> N39; + N39 -> N35[label="exiting scope_0 expr break,\lexiting scope_1 stmt break ;,\lexiting scope_2 block { break ; \"unreachable\"; },\lexiting scope_3 expr if z == 0 { break ; \"unreachable\"; },\lexiting scope_4 stmt if z == 0 { break ; \"unreachable\"; },\lexiting scope_5 block { if z == 0 { break ; \"unreachable\"; } z -= 1; }"]; + N40 -> N41; + N41 -> N42; + N38 -> N43; + N42 -> N43; + N43 -> N44; + N44 -> N45; + N45 -> N46; + N46 -> N47; + N47 -> N34; + N35 -> N48; + N48 -> N49; + N49 -> N50; + N50 -> N51; + N51 -> N1[label="exiting scope_0 expr loop {\l if y == 0 { break ; \"unreachable\"; }\l y -= 1;\l loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }\l if x > 10 { return; \"unreachable\"; }\l}\l,\lexiting scope_1 expr loop {\l if x == 0 { break ; \"unreachable\"; }\l x -= 1;\l loop {\l if y == 0 { break ; \"unreachable\"; }\l y -= 1;\l loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }\l if x > 10 { return; \"unreachable\"; }\l }\l}\l"]; + N52 -> N53; + N53 -> N54; + N50 -> N55; + N54 -> N55; + N55 -> N56; + N56 -> N21; + N22 -> N57; + N57 -> N8; + N9 -> N58; + N58 -> N1; +} diff --git a/src/test/run-make/graphviz-flowgraph/f24.rs b/src/test/run-make/graphviz-flowgraph/f24.rs new file mode 100644 index 0000000000000..f796d660a1856 --- /dev/null +++ b/src/test/run-make/graphviz-flowgraph/f24.rs @@ -0,0 +1,36 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[allow(unreachable_code)] +pub fn expr_while_24() { + let mut x = 24; + let mut y = 24; + let mut z = 24; + + loop { + if x == 0 { break; "unreachable"; } + x -= 1; + + loop { + if y == 0 { break; "unreachable"; } + y -= 1; + + loop { + if z == 0 { break; "unreachable"; } + z -= 1; + } + + if x > 10 { + return; + "unreachable"; + } + } + } +} diff --git a/src/test/run-make/graphviz-flowgraph/f25.dot-expected.dot b/src/test/run-make/graphviz-flowgraph/f25.dot-expected.dot new file mode 100644 index 0000000000000..c393b63546c70 --- /dev/null +++ b/src/test/run-make/graphviz-flowgraph/f25.dot-expected.dot @@ -0,0 +1,123 @@ +digraph block { + N0[label="entry"]; + N1[label="exit"]; + N2[label="expr 25"]; + N3[label="local mut x"]; + N4[label="expr 25"]; + N5[label="local mut y"]; + N6[label="expr 25"]; + N7[label="local mut z"]; + N8[label="(dummy_node)"]; + N9[label="expr \'a:\l loop {\l if x == 0 { break ; \"unreachable\"; }\l x -= 1;\l \'a:\l loop {\l if y == 0 { break ; \"unreachable\"; }\l y -= 1;\l \'a: loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }\l if x > 10 { continue \'a ; \"unreachable\"; }\l }\l }\l"]; + N10[label="expr x"]; + N11[label="expr 0"]; + N12[label="expr x == 0"]; + N13[label="expr break"]; + N14[label="(dummy_node)"]; + N15[label="expr \"unreachable\""]; + N16[label="block { break ; \"unreachable\"; }"]; + N17[label="expr if x == 0 { break ; \"unreachable\"; }"]; + N18[label="expr 1"]; + N19[label="expr x"]; + N20[label="expr x -= 1"]; + N21[label="(dummy_node)"]; + N22[label="expr \'a:\l loop {\l if y == 0 { break ; \"unreachable\"; }\l y -= 1;\l \'a: loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }\l if x > 10 { continue \'a ; \"unreachable\"; }\l }\l"]; + N23[label="expr y"]; + N24[label="expr 0"]; + N25[label="expr y == 0"]; + N26[label="expr break"]; + N27[label="(dummy_node)"]; + N28[label="expr \"unreachable\""]; + N29[label="block { break ; \"unreachable\"; }"]; + N30[label="expr if y == 0 { break ; \"unreachable\"; }"]; + N31[label="expr 1"]; + N32[label="expr y"]; + N33[label="expr y -= 1"]; + N34[label="(dummy_node)"]; + N35[label="expr \'a: loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }"]; + N36[label="expr z"]; + N37[label="expr 0"]; + N38[label="expr z == 0"]; + N39[label="expr break"]; + N40[label="(dummy_node)"]; + N41[label="expr \"unreachable\""]; + N42[label="block { break ; \"unreachable\"; }"]; + N43[label="expr if z == 0 { break ; \"unreachable\"; }"]; + N44[label="expr 1"]; + N45[label="expr z"]; + N46[label="expr z -= 1"]; + N47[label="block { if z == 0 { break ; \"unreachable\"; } z -= 1; }"]; + N48[label="expr x"]; + N49[label="expr 10"]; + N50[label="expr x > 10"]; + N51[label="expr continue \'a"]; + N52[label="(dummy_node)"]; + N53[label="expr \"unreachable\""]; + N54[label="block { continue \'a ; \"unreachable\"; }"]; + N55[label="expr if x > 10 { continue \'a ; \"unreachable\"; }"]; + N56[label="block {\l if y == 0 { break ; \"unreachable\"; }\l y -= 1;\l \'a: loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }\l if x > 10 { continue \'a ; \"unreachable\"; }\l}\l"]; + N57[label="block {\l if x == 0 { break ; \"unreachable\"; }\l x -= 1;\l \'a:\l loop {\l if y == 0 { break ; \"unreachable\"; }\l y -= 1;\l \'a: loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }\l if x > 10 { continue \'a ; \"unreachable\"; }\l }\l}\l"]; + N58[label="block {\l let mut x = 25;\l let mut y = 25;\l let mut z = 25;\l \'a:\l loop {\l if x == 0 { break ; \"unreachable\"; }\l x -= 1;\l \'a:\l loop {\l if y == 0 { break ; \"unreachable\"; }\l y -= 1;\l \'a: loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }\l if x > 10 { continue \'a ; \"unreachable\"; }\l }\l }\l}\l"]; + N0 -> N2; + N2 -> N3; + N3 -> N4; + N4 -> N5; + N5 -> N6; + N6 -> N7; + N7 -> N8; + N8 -> N10; + N10 -> N11; + N11 -> N12; + N12 -> N13; + N13 -> N9[label="exiting scope_0 expr break,\lexiting scope_1 stmt break ;,\lexiting scope_2 block { break ; \"unreachable\"; },\lexiting scope_3 expr if x == 0 { break ; \"unreachable\"; },\lexiting scope_4 stmt if x == 0 { break ; \"unreachable\"; },\lexiting scope_5 block {\l if x == 0 { break ; \"unreachable\"; }\l x -= 1;\l \'a:\l loop {\l if y == 0 { break ; \"unreachable\"; }\l y -= 1;\l \'a: loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }\l if x > 10 { continue \'a ; \"unreachable\"; }\l }\l}\l"]; + N14 -> N15; + N15 -> N16; + N12 -> N17; + N16 -> N17; + N17 -> N18; + N18 -> N19; + N19 -> N20; + N20 -> N21; + N21 -> N23; + N23 -> N24; + N24 -> N25; + N25 -> N26; + N26 -> N22[label="exiting scope_0 expr break,\lexiting scope_1 stmt break ;,\lexiting scope_2 block { break ; \"unreachable\"; },\lexiting scope_3 expr if y == 0 { break ; \"unreachable\"; },\lexiting scope_4 stmt if y == 0 { break ; \"unreachable\"; },\lexiting scope_5 block {\l if y == 0 { break ; \"unreachable\"; }\l y -= 1;\l \'a: loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }\l if x > 10 { continue \'a ; \"unreachable\"; }\l}\l"]; + N27 -> N28; + N28 -> N29; + N25 -> N30; + N29 -> N30; + N30 -> N31; + N31 -> N32; + N32 -> N33; + N33 -> N34; + N34 -> N36; + N36 -> N37; + N37 -> N38; + N38 -> N39; + N39 -> N35[label="exiting scope_0 expr break,\lexiting scope_1 stmt break ;,\lexiting scope_2 block { break ; \"unreachable\"; },\lexiting scope_3 expr if z == 0 { break ; \"unreachable\"; },\lexiting scope_4 stmt if z == 0 { break ; \"unreachable\"; },\lexiting scope_5 block { if z == 0 { break ; \"unreachable\"; } z -= 1; }"]; + N40 -> N41; + N41 -> N42; + N38 -> N43; + N42 -> N43; + N43 -> N44; + N44 -> N45; + N45 -> N46; + N46 -> N47; + N47 -> N34; + N35 -> N48; + N48 -> N49; + N49 -> N50; + N50 -> N51; + N51 -> N21[label="exiting scope_0 expr continue \'a,\lexiting scope_1 stmt continue \'a ;,\lexiting scope_2 block { continue \'a ; \"unreachable\"; },\lexiting scope_3 expr if x > 10 { continue \'a ; \"unreachable\"; },\lexiting scope_4 block {\l if y == 0 { break ; \"unreachable\"; }\l y -= 1;\l \'a: loop { if z == 0 { break ; \"unreachable\"; } z -= 1; }\l if x > 10 { continue \'a ; \"unreachable\"; }\l}\l"]; + N52 -> N53; + N53 -> N54; + N50 -> N55; + N54 -> N55; + N55 -> N56; + N56 -> N21; + N22 -> N57; + N57 -> N8; + N9 -> N58; + N58 -> N1; +} diff --git a/src/test/run-make/graphviz-flowgraph/f25.rs b/src/test/run-make/graphviz-flowgraph/f25.rs new file mode 100644 index 0000000000000..2ee2e48fd10e0 --- /dev/null +++ b/src/test/run-make/graphviz-flowgraph/f25.rs @@ -0,0 +1,36 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[allow(unreachable_code)] +pub fn expr_while_25() { + let mut x = 25; + let mut y = 25; + let mut z = 25; + + 'a: loop { + if x == 0 { break; "unreachable"; } + x -= 1; + + 'a: loop { + if y == 0 { break; "unreachable"; } + y -= 1; + + 'a: loop { + if z == 0 { break; "unreachable"; } + z -= 1; + } + + if x > 10 { + continue 'a; + "unreachable"; + } + } + } +} From 4c2a8bbc097b4ba0c317c07a095238aed128899d Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 18 Jun 2014 15:27:49 +0200 Subject: [PATCH 9/9] Adapt test case to match current set of emitted warnings. (or lack thereof.) PR 14739 injected the new message that this removes from one test case: borrowck-vec-pattern-loan-from-mut.rs When reviewing the test case, I was not able to convince myself that the error message was a legitimate thing to start emitting. Niko did not see an obvious reason for it either, so I am going to remove it and wait for someone (maybe Cameron Zwarich) to explain to me why we should be emitting it. --- src/test/compile-fail/borrowck-vec-pattern-loan-from-mut.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/compile-fail/borrowck-vec-pattern-loan-from-mut.rs b/src/test/compile-fail/borrowck-vec-pattern-loan-from-mut.rs index ff029ce624c9b..393ec8b0b1b3b 100644 --- a/src/test/compile-fail/borrowck-vec-pattern-loan-from-mut.rs +++ b/src/test/compile-fail/borrowck-vec-pattern-loan-from-mut.rs @@ -12,7 +12,7 @@ fn a() { let mut v = vec!(1, 2, 3); let vb: &mut [int] = v.as_mut_slice(); match vb { - [_a, ..tail] => { //~ ERROR cannot use `vb[..]` because it was mutably borrowed + [_a, ..tail] => { v.push(tail[0] + tail[1]); //~ ERROR cannot borrow } _ => {}