From 5af0fa603f0b3f1708b1661b410a2419ea256456 Mon Sep 17 00:00:00 2001 From: James Miller Date: Sat, 7 Feb 2015 23:09:45 +1300 Subject: [PATCH 1/3] Improve CFG construction for match expressions Previously, for each arm, each pattern would be visited, then the guard, then the body. All arms would be visited in parallel. Now, the patterns in each arm are visited in parallel, but after each pattern, the guard is visited. Furthermore, an edge is added from each guard to the next, reflecting the fact that a guard may be evaluated multiple times, and that multiple guards may be evaluated. This also refactors the CFG a little to use explicit node types for entry, exit, dummy and unreachable nodes. --- src/librustc/lint/builtin.rs | 2 +- src/librustc/middle/cfg/construct.rs | 298 +++++++++++++++-------- src/librustc/middle/cfg/mod.rs | 26 +- src/librustc/middle/dataflow.rs | 147 ++++++----- src/librustc/middle/expr_use_visitor.rs | 6 +- src/librustc/middle/graph.rs | 4 +- src/librustc/middle/pat_util.rs | 15 ++ src/librustc_borrowck/borrowck/mod.rs | 25 +- src/librustc_trans/trans/base.rs | 2 +- src/libsyntax/ext/build.rs | 2 +- src/libsyntax/ext/expand.rs | 2 +- src/libsyntax/fold.rs | 10 +- src/test/compile-fail/move-in-guard-1.rs | 25 ++ src/test/compile-fail/move-in-guard-2.rs | 25 ++ src/test/run-pass/move-guard-const.rs | 25 ++ 15 files changed, 426 insertions(+), 188 deletions(-) create mode 100644 src/test/compile-fail/move-in-guard-1.rs create mode 100644 src/test/compile-fail/move-in-guard-2.rs create mode 100644 src/test/run-pass/move-guard-const.rs diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 638ecf4572d5d..70cb8ac96347c 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -1846,7 +1846,7 @@ impl LintPass for UnconditionalRecursion { continue } visited.insert(cfg_id); - let node_id = cfg.graph.node_data(idx).id; + let node_id = cfg.graph.node_data(idx).id(); // is this a recursive call? if node_id != ast::DUMMY_NODE_ID && checker(cx.tcx, impl_node_id, id, name, node_id) { diff --git a/src/librustc/middle/cfg/construct.rs b/src/librustc/middle/cfg/construct.rs index d39b94a202e4a..82cbc72a222db 100644 --- a/src/librustc/middle/cfg/construct.rs +++ b/src/librustc/middle/cfg/construct.rs @@ -11,19 +11,22 @@ use middle::cfg::*; use middle::def; use middle::graph; +use middle::pat_util; use middle::region::CodeExtent; use middle::ty; + +use util::nodemap::FnvHashMap; + use syntax::ast; use syntax::ast_util; use syntax::ptr::P; -use util::nodemap::NodeMap; struct CFGBuilder<'a, 'tcx: 'a> { tcx: &'a ty::ctxt<'tcx>, - exit_map: NodeMap, graph: CFGGraph, fn_exit: CFGIndex, loop_scopes: Vec, + has_unreachable: bool, } #[derive(Copy)] @@ -36,33 +39,38 @@ struct LoopScope { pub fn construct(tcx: &ty::ctxt, blk: &ast::Block) -> CFG { let mut graph = graph::Graph::new(); - let entry = add_initial_dummy_node(&mut graph); + let entry = graph.add_node(CFGNodeData::Entry); // `fn_exit` is target of return exprs, which lies somewhere // outside input `blk`. (Distinguishing `fn_exit` and `block_exit` // also resolves chicken-and-egg problem that arises if you try to // have return exprs jump to `block_exit` during construction.) - let fn_exit = add_initial_dummy_node(&mut graph); + let fn_exit = graph.add_node(CFGNodeData::Exit); let block_exit; let mut cfg_builder = CFGBuilder { - exit_map: NodeMap(), graph: graph, fn_exit: fn_exit, tcx: tcx, - loop_scopes: Vec::new() + loop_scopes: Vec::new(), + has_unreachable: false, }; block_exit = cfg_builder.block(blk, entry); cfg_builder.add_contained_edge(block_exit, fn_exit); - let CFGBuilder {exit_map, graph, ..} = cfg_builder; - CFG {exit_map: exit_map, - graph: graph, - entry: entry, - exit: fn_exit} -} + let CFGBuilder {graph, has_unreachable, ..} = cfg_builder; + -fn add_initial_dummy_node(g: &mut CFGGraph) -> CFGIndex { - g.add_node(CFGNodeData { id: ast::DUMMY_NODE_ID }) + let cfg = CFG { + graph: graph, + entry: entry, + exit: fn_exit + }; + + if has_unreachable { + remove_unreachable(cfg) + } else { + cfg + } } impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { @@ -74,19 +82,19 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { let expr_exit = self.opt_expr(&blk.expr, stmts_exit); - self.add_node(blk.id, &[expr_exit]) + self.add_ast_node(blk.id, &[expr_exit]) } fn stmt(&mut self, stmt: &ast::Stmt, pred: CFGIndex) -> CFGIndex { match stmt.node { ast::StmtDecl(ref decl, id) => { let exit = self.decl(&**decl, pred); - self.add_node(id, &[exit]) + self.add_ast_node(id, &[exit]) } ast::StmtExpr(ref expr, id) | ast::StmtSemi(ref expr, id) => { let exit = self.expr(&**expr, pred); - self.add_node(id, &[exit]) + self.add_ast_node(id, &[exit]) } ast::StmtMac(..) => { @@ -115,33 +123,33 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { ast::PatLit(..) | ast::PatRange(..) | ast::PatWild(_) => { - self.add_node(pat.id, &[pred]) + self.add_ast_node(pat.id, &[pred]) } ast::PatBox(ref subpat) | ast::PatRegion(ref subpat, _) | ast::PatIdent(_, _, Some(ref subpat)) => { let subpat_exit = self.pat(&**subpat, pred); - self.add_node(pat.id, &[subpat_exit]) + self.add_ast_node(pat.id, &[subpat_exit]) } ast::PatEnum(_, Some(ref subpats)) | ast::PatTup(ref subpats) => { let pats_exit = self.pats_all(subpats.iter(), pred); - self.add_node(pat.id, &[pats_exit]) + self.add_ast_node(pat.id, &[pats_exit]) } ast::PatStruct(_, ref subpats, _) => { let pats_exit = self.pats_all(subpats.iter().map(|f| &f.node.pat), pred); - self.add_node(pat.id, &[pats_exit]) + self.add_ast_node(pat.id, &[pats_exit]) } ast::PatVec(ref pre, ref vec, ref post) => { let pre_exit = self.pats_all(pre.iter(), pred); let vec_exit = self.pats_all(vec.iter(), pre_exit); let post_exit = self.pats_all(post.iter(), vec_exit); - self.add_node(pat.id, &[post_exit]) + self.add_ast_node(pat.id, &[post_exit]) } ast::PatMac(_) => { @@ -157,28 +165,11 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { pats.fold(pred, |pred, pat| self.pat(&**pat, pred)) } - fn pats_any(&mut self, - pats: &[P], - pred: CFGIndex) -> CFGIndex { - //! Handles case where just one of the patterns must match. - - if pats.len() == 1 { - self.pat(&*pats[0], pred) - } else { - let collect = self.add_dummy_node(&[]); - for pat in pats { - let pat_exit = self.pat(&**pat, pred); - self.add_contained_edge(pat_exit, collect); - } - collect - } - } - fn expr(&mut self, expr: &ast::Expr, pred: CFGIndex) -> CFGIndex { match expr.node { ast::ExprBlock(ref blk) => { let blk_exit = self.block(&**blk, pred); - self.add_node(expr.id, &[blk_exit]) + self.add_ast_node(expr.id, &[blk_exit]) } ast::ExprIf(ref cond, ref then, None) => { @@ -198,7 +189,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { // let cond_exit = self.expr(&**cond, pred); // 1 let then_exit = self.block(&**then, cond_exit); // 2 - self.add_node(expr.id, &[cond_exit, then_exit]) // 3,4 + self.add_ast_node(expr.id, &[cond_exit, then_exit]) // 3,4 } ast::ExprIf(ref cond, ref then, Some(ref otherwise)) => { @@ -219,7 +210,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { let cond_exit = self.expr(&**cond, pred); // 1 let then_exit = self.block(&**then, cond_exit); // 2 let else_exit = self.expr(&**otherwise, cond_exit); // 3 - self.add_node(expr.id, &[then_exit, else_exit]) // 4, 5 + self.add_ast_node(expr.id, &[then_exit, else_exit]) // 4, 5 } ast::ExprIfLet(..) => { @@ -247,7 +238,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { // Is the condition considered part of the loop? let loopback = self.add_dummy_node(&[pred]); // 1 let cond_exit = self.expr(&**cond, loopback); // 2 - let expr_exit = self.add_node(expr.id, &[cond_exit]); // 3 + let expr_exit = self.add_ast_node(expr.id, &[cond_exit]); // 3 self.loop_scopes.push(LoopScope { loop_id: expr.id, continue_index: loopback, @@ -283,7 +274,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { // may cause additional edges. let loopback = self.add_dummy_node(&[pred]); // 1 - let expr_exit = self.add_node(expr.id, &[]); // 2 + let expr_exit = self.add_ast_node(expr.id, &[]); // 2 self.loop_scopes.push(LoopScope { loop_id: expr.id, continue_index: loopback, @@ -296,45 +287,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { } ast::ExprMatch(ref discr, ref arms, _) => { - // - // [pred] - // | - // v 1 - // [discr] - // | - // v 2 - // [cond1] - // / \ - // | \ - // v 3 \ - // [pat1] \ - // | | - // v 4 | - // [guard1] | - // | | - // | | - // v 5 v - // [body1] [cond2] - // | / \ - // | ... ... - // | | | - // v 6 v v - // [.....expr.....] - // - let discr_exit = self.expr(&**discr, pred); // 1 - - let expr_exit = self.add_node(expr.id, &[]); - let mut cond_exit = discr_exit; - for arm in arms { - cond_exit = self.add_dummy_node(&[cond_exit]); // 2 - let pats_exit = self.pats_any(&arm.pats[], - cond_exit); // 3 - let guard_exit = self.opt_expr(&arm.guard, - pats_exit); // 4 - let body_exit = self.expr(&*arm.body, guard_exit); // 5 - self.add_contained_edge(body_exit, expr_exit); // 6 - } - expr_exit + self.match_(expr.id, &**discr, &arms[], pred) } ast::ExprBinary(op, ref l, ref r) if ast_util::lazy_binop(op.node) => { @@ -354,30 +307,30 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { // let l_exit = self.expr(&**l, pred); // 1 let r_exit = self.expr(&**r, l_exit); // 2 - self.add_node(expr.id, &[l_exit, r_exit]) // 3,4 + self.add_ast_node(expr.id, &[l_exit, r_exit]) // 3,4 } ast::ExprRet(ref v) => { let v_exit = self.opt_expr(v, pred); - let b = self.add_node(expr.id, &[v_exit]); + let b = self.add_ast_node(expr.id, &[v_exit]); self.add_returning_edge(expr, b); - self.add_node(ast::DUMMY_NODE_ID, &[]) + self.add_unreachable_node() } ast::ExprBreak(label) => { let loop_scope = self.find_scope(expr, label); - let b = self.add_node(expr.id, &[pred]); + let b = self.add_ast_node(expr.id, &[pred]); self.add_exiting_edge(expr, b, loop_scope, loop_scope.break_index); - self.add_node(ast::DUMMY_NODE_ID, &[]) + self.add_unreachable_node() } ast::ExprAgain(label) => { let loop_scope = self.find_scope(expr, label); - let a = self.add_node(expr.id, &[pred]); + let a = self.add_ast_node(expr.id, &[pred]); self.add_exiting_edge(expr, a, loop_scope, loop_scope.continue_index); - self.add_node(ast::DUMMY_NODE_ID, &[]) + self.add_unreachable_node() } ast::ExprVec(ref elems) => { @@ -454,7 +407,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { let &(_, ref expr, _) = a; &**expr }), post_inputs); - self.add_node(expr.id, &[post_outputs]) + self.add_ast_node(expr.id, &[post_outputs]) } ast::ExprMac(..) | @@ -481,7 +434,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { let func_or_rcvr_exit = self.expr(func_or_rcvr, pred); let ret = self.straightline(call_expr, func_or_rcvr_exit, args); if return_ty.diverges() { - self.add_node(ast::DUMMY_NODE_ID, &[]) + self.add_unreachable_node() } else { ret } @@ -508,22 +461,122 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { //! Handles case of an expression that evaluates `subexprs` in order let subexprs_exit = self.exprs(subexprs, pred); - self.add_node(expr.id, &[subexprs_exit]) + self.add_ast_node(expr.id, &[subexprs_exit]) + } + + fn match_(&mut self, id: ast::NodeId, discr: &ast::Expr, + arms: &[ast::Arm], pred: CFGIndex) -> CFGIndex { + // The CFG for match expression is quite complex, so no ASCII art for it. + // + // The CFG generated below matches roughly what trans puts out. Each pattern and guard is + // visited in parallel, with arms containing multiple patterns generating multiple nodes for + // the same guard expression. The guard expressions chain into each other from top to + // bottom, with a specific exception to allow some additional valid programs (explained + // below). Trans differs slightly in that the pattern matching may continue after a guard + // but the visible behaviour should be the same. + // + // What is going on is explained in further comments. + + // Visit the discriminant expression + let discr_exit = self.expr(discr, pred); + + // Add a node for the exit of the match expression as a whole. + let expr_exit = self.add_ast_node(id, &[]); + + + // Keep track of the previous guard expressions + let mut prev_guards = Vec::new(); + // Track if the previous pattern contained bindings or wildcards + let mut prev_has_bindings = false; + + for arm in arms { + // Add an exit node for when we've visited all the patterns and the + // guard (if there is one) in the arm. + let arm_exit = self.add_dummy_node(&[]); + + for pat in &arm.pats { + // Visit the pattern, coming from the discriminant exit + let mut pat_exit = self.pat(&**pat, discr_exit); + + // If there is a guard expression, handle it here + if let Some(ref guard) = arm.guard { + // Add a dummy node for the previous guard expression to target + let guard_start = self.add_dummy_node(&[pat_exit]); + // Visit the guard expression + let guard_exit = self.expr(&**guard, guard_start); + + let this_has_bindings = pat_util::pat_contains_bindings_or_wild( + &self.tcx.def_map, &**pat); + + // If both this pattern and the previous pattern were free of guards, they must + // consist only of "constant" patterns. Since we can't match an all-constant + // pattern, fail the guard, then match *another* all-constant pattern. This is + // because if the previous pattern matches, then we *can't* match this one, + // unless all the constants are the same (which is rejected by `check_match`). + // + // We can use this to be smarter about the flow along guards. If the previous + // pattern matched, then we know we won't visit the guard in this one (whether + // or not the guard succeeded), if the previous pattern failed, then we know the + // guard for that pattern won't have been visited. Thus, it isn't possible to + // visit both the previous guard and the current one when both patterns consist + // only of constant sub-patterns. + // + // However, if the above does not hold, then all previous guards need to be + // wired to visit the current guard pattern. + if prev_has_bindings || this_has_bindings { + while let Some(prev) = prev_guards.pop() { + self.add_contained_edge(prev, guard_start); + } + } + + prev_has_bindings = this_has_bindings; + + // Push the guard onto the list of previous guards + prev_guards.push(guard_exit); + + // Update the exit node for the pattern + pat_exit = guard_exit; + } + + // Add an edge from the exit of this pattern to the exit of the arm + self.add_contained_edge(pat_exit, arm_exit); + } + + // Visit the body of this arm + let body_exit = self.expr(&arm.body, arm_exit); + + // Link the body to the exit of the expression + self.add_contained_edge(body_exit, expr_exit); + } + + expr_exit } fn add_dummy_node(&mut self, preds: &[CFGIndex]) -> CFGIndex { - self.add_node(ast::DUMMY_NODE_ID, preds) + self.add_node(CFGNodeData::Dummy, preds) } - fn add_node(&mut self, id: ast::NodeId, preds: &[CFGIndex]) -> CFGIndex { - assert!(!self.exit_map.contains_key(&id)); - let node = self.graph.add_node(CFGNodeData {id: id}); - if id != ast::DUMMY_NODE_ID { - assert!(!self.exit_map.contains_key(&id)); - self.exit_map.insert(id, node); + fn add_ast_node(&mut self, id: ast::NodeId, preds: &[CFGIndex]) -> CFGIndex { + if id == ast::DUMMY_NODE_ID { + self.add_dummy_node(preds) + } else { + let data = CFGNodeData::AST(id); + self.add_node(data, preds) } + } + + fn add_unreachable_node(&mut self) -> CFGIndex { + self.has_unreachable = true; + self.add_node(CFGNodeData::Unreachable, &[]) + } + + fn add_node(&mut self, data: CFGNodeData, preds: &[CFGIndex]) -> CFGIndex { + let node = self.graph.add_node(data); for &pred in preds { - self.add_contained_edge(pred, node); + // Don't add edges from unreachable nodes + if *self.graph.node_data(pred) != CFGNodeData::Unreachable { + self.add_contained_edge(pred, node); + } } node } @@ -601,3 +654,46 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { self.tcx.method_map.borrow().contains_key(&method_call) } } + +fn remove_unreachable(cfg: CFG) -> CFG { + let mut new_graph = graph::Graph::new(); + + let mut new_entry = graph::InvalidNodeIndex; + let mut new_exit = graph::InvalidNodeIndex; + + let mut idx_map = FnvHashMap(); + + cfg.graph.each_node(|idx, node| { + if node.data != CFGNodeData::Unreachable { + let new_idx = new_graph.add_node(node.data); + + idx_map.insert(idx, new_idx); + + if idx == cfg.entry { + new_entry = new_idx; + } + if idx == cfg.exit { + new_exit = new_idx; + } + } + true + }); + + for e in cfg.graph.all_edges() { + let new_source = idx_map.get(&e.source()); + let new_target = idx_map.get(&e.target()); + + match (new_source, new_target) { + (Some(&src), Some(&tgt)) => { + new_graph.add_edge(src, tgt, e.data.clone()); + } + _ => () + } + } + + CFG { + graph: new_graph, + entry: new_entry, + exit: new_exit + } +} diff --git a/src/librustc/middle/cfg/mod.rs b/src/librustc/middle/cfg/mod.rs index 0ca146a295e13..96104ad8d504a 100644 --- a/src/librustc/middle/cfg/mod.rs +++ b/src/librustc/middle/cfg/mod.rs @@ -14,23 +14,37 @@ use middle::graph; use middle::ty; use syntax::ast; -use util::nodemap::NodeMap; mod construct; pub mod graphviz; pub struct CFG { - pub exit_map: NodeMap, pub graph: CFGGraph, pub entry: CFGIndex, pub exit: CFGIndex, } -#[derive(Copy)] -pub struct CFGNodeData { - pub id: ast::NodeId +#[derive(Copy, PartialEq)] +pub enum CFGNodeData { + AST(ast::NodeId), + Entry, + Exit, + Dummy, + Unreachable, } +impl CFGNodeData { + + pub fn id(&self) -> ast::NodeId { + if let CFGNodeData::AST(id) = *self { + id + } else { + ast::DUMMY_NODE_ID + } + } +} + +#[derive(Clone)] pub struct CFGEdgeData { pub exiting_scopes: Vec } @@ -50,6 +64,6 @@ impl CFG { } pub fn node_is_reachable(&self, id: ast::NodeId) -> bool { - self.graph.depth_traverse(self.entry).any(|node| node.id == id) + self.graph.depth_traverse(self.entry).any(|node| node.id() == id) } } diff --git a/src/librustc/middle/dataflow.rs b/src/librustc/middle/dataflow.rs index 7ba83c62496f3..2c1474bd3fe6d 100644 --- a/src/librustc/middle/dataflow.rs +++ b/src/librustc/middle/dataflow.rs @@ -53,7 +53,7 @@ pub struct DataFlowContext<'a, 'tcx: 'a, O> { // mapping from node to cfg node index // FIXME (#6298): Shouldn't this go with CFG? - nodeid_to_index: NodeMap, + nodeid_to_index: NodeMap>, // Bit sets per cfg node. The following three fields (`gens`, `kills`, // and `on_entry`) all have the same structure. For each id in @@ -88,16 +88,13 @@ struct PropagationContext<'a, 'b: 'a, 'tcx: 'b, O: 'a> { changed: bool } -fn to_cfgidx_or_die(id: ast::NodeId, index: &NodeMap) -> CFGIndex { - let opt_cfgindex = index.get(&id).map(|&i|i); - opt_cfgindex.unwrap_or_else(|| { - panic!("nodeid_to_index does not have entry for NodeId {}", id); - }) +fn get_cfg_indices<'a>(id: ast::NodeId, index: &'a NodeMap>) -> &'a [CFGIndex] { + let opt_indices = index.get(&id); + opt_indices.map(|v| &v[]).unwrap_or(&[]) } impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> { fn has_bitset_for_nodeid(&self, n: ast::NodeId) -> bool { - assert!(n != ast::DUMMY_NODE_ID); self.nodeid_to_index.contains_key(&n) } } @@ -116,35 +113,37 @@ impl<'a, 'tcx, O:DataFlowOperator> pprust::PpAnn for DataFlowContext<'a, 'tcx, O if self.has_bitset_for_nodeid(id) { assert!(self.bits_per_id > 0); - let cfgidx = to_cfgidx_or_die(id, &self.nodeid_to_index); - let (start, end) = self.compute_id_range(cfgidx); - let on_entry = &self.on_entry[start.. end]; - let entry_str = bits_to_string(on_entry); - - let gens = &self.gens[start.. end]; - let gens_str = if gens.iter().any(|&u| u != 0) { - format!(" gen: {}", bits_to_string(gens)) - } else { - "".to_string() - }; - - let kills = &self.kills[start .. end]; - let kills_str = if kills.iter().any(|&u| u != 0) { - format!(" kill: {}", bits_to_string(kills)) - } else { - "".to_string() - }; - - try!(ps.synth_comment(format!("id {}: {}{}{}", id, entry_str, - gens_str, kills_str))); - try!(pp::space(&mut ps.s)); + let indices = get_cfg_indices(id, &self.nodeid_to_index); + for &cfgidx in indices { + let (start, end) = self.compute_id_range(cfgidx); + let on_entry = &self.on_entry[start.. end]; + let entry_str = bits_to_string(on_entry); + + let gens = &self.gens[start.. end]; + let gens_str = if gens.iter().any(|&u| u != 0) { + format!(" gen: {}", bits_to_string(gens)) + } else { + "".to_string() + }; + + let kills = &self.kills[start .. end]; + let kills_str = if kills.iter().any(|&u| u != 0) { + format!(" kill: {}", bits_to_string(kills)) + } else { + "".to_string() + }; + + try!(ps.synth_comment(format!("id {}: {}{}{}", id, entry_str, + gens_str, kills_str))); + try!(pp::space(&mut ps.s)); + } } Ok(()) } } fn build_nodeid_to_index(decl: Option<&ast::FnDecl>, - cfg: &cfg::CFG) -> NodeMap { + cfg: &cfg::CFG) -> NodeMap> { let mut index = NodeMap(); // FIXME (#6298): Would it be better to fold formals from decl @@ -157,28 +156,38 @@ fn build_nodeid_to_index(decl: Option<&ast::FnDecl>, } cfg.graph.each_node(|node_idx, node| { - if node.data.id != ast::DUMMY_NODE_ID { - index.insert(node.data.id, node_idx); + if let cfg::CFGNodeData::AST(id) = node.data { + match index.entry(id).get() { + Ok(v) => v.push(node_idx), + Err(e) => { + e.insert(vec![node_idx]); + } + } } true }); return index; - fn add_entries_from_fn_decl(index: &mut NodeMap, + 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, + index: &'a mut NodeMap>, } let mut formals = Formals { entry: entry, index: index }; visit::walk_fn_decl(&mut formals, decl); impl<'a, 'v> visit::Visitor<'v> for Formals<'a> { fn visit_pat(&mut self, p: &ast::Pat) { - self.index.insert(p.id, self.entry); + match self.index.entry(p.id).get() { + Ok(v) => v.push(self.entry), + Err(e) => { + e.insert(vec![self.entry]); + } + } visit::walk_pat(self, p) } } @@ -227,26 +236,28 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> { //! Indicates that `id` generates `bit` debug!("{} add_gen(id={}, bit={})", self.analysis_name, id, bit); - assert!(self.nodeid_to_index.contains_key(&id)); assert!(self.bits_per_id > 0); - let cfgidx = to_cfgidx_or_die(id, &self.nodeid_to_index); - let (start, end) = self.compute_id_range(cfgidx); - let gens = &mut self.gens[start.. end]; - set_bit(gens, bit); + let indices = get_cfg_indices(id, &self.nodeid_to_index); + for &cfgidx in indices { + let (start, end) = self.compute_id_range(cfgidx); + let gens = &mut self.gens[start.. end]; + set_bit(gens, bit); + } } pub fn add_kill(&mut self, id: ast::NodeId, bit: uint) { //! Indicates that `id` kills `bit` debug!("{} add_kill(id={}, bit={})", self.analysis_name, id, bit); - assert!(self.nodeid_to_index.contains_key(&id)); assert!(self.bits_per_id > 0); - let cfgidx = to_cfgidx_or_die(id, &self.nodeid_to_index); - let (start, end) = self.compute_id_range(cfgidx); - let kills = &mut self.kills[start.. end]; - set_bit(kills, bit); + let indices = get_cfg_indices(id, &self.nodeid_to_index); + for &cfgidx in indices { + let (start, end) = self.compute_id_range(cfgidx); + let kills = &mut self.kills[start.. end]; + set_bit(kills, bit); + } } fn apply_gen_kill(&self, cfgidx: CFGIndex, bits: &mut [uint]) { @@ -279,7 +290,7 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> { } - pub fn each_bit_on_entry(&self, id: ast::NodeId, f: F) -> bool where + pub fn each_bit_on_entry(&self, id: ast::NodeId, mut f: F) -> bool where F: FnMut(uint) -> bool, { //! Iterates through each bit that is set on entry to `id`. @@ -287,8 +298,13 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> { if !self.has_bitset_for_nodeid(id) { return true; } - let cfgidx = to_cfgidx_or_die(id, &self.nodeid_to_index); - self.each_bit_for_node(Entry, cfgidx, f) + let indices = get_cfg_indices(id, &self.nodeid_to_index); + for &cfgidx in indices { + if !self.each_bit_for_node(Entry, cfgidx, |i| f(i)) { + return false; + } + } + return true; } pub fn each_bit_for_node(&self, e: EntryOrExit, cfgidx: CFGIndex, f: F) -> bool where @@ -320,7 +336,7 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> { self.each_bit(slice, f) } - pub fn each_gen_bit(&self, id: ast::NodeId, f: F) -> bool where + pub fn each_gen_bit(&self, id: ast::NodeId, mut f: F) -> bool where F: FnMut(uint) -> bool, { //! Iterates through each bit in the gen set for `id`. @@ -334,12 +350,17 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> { return true; } - let cfgidx = to_cfgidx_or_die(id, &self.nodeid_to_index); - let (start, end) = self.compute_id_range(cfgidx); - let gens = &self.gens[start.. end]; - debug!("{} each_gen_bit(id={}, gens={})", - self.analysis_name, id, bits_to_string(gens)); - self.each_bit(gens, f) + let indices = get_cfg_indices(id, &self.nodeid_to_index); + for &cfgidx in indices { + let (start, end) = self.compute_id_range(cfgidx); + let gens = &self.gens[start.. end]; + debug!("{} each_gen_bit(id={}, gens={})", + self.analysis_name, id, bits_to_string(gens)); + if !self.each_bit(gens, |i| f(i)) { + return false; + } + } + return true; } fn each_bit(&self, words: &[uint], mut f: F) -> bool where @@ -400,13 +421,15 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> { let mut changed = false; for &node_id in &edge.data.exiting_scopes { - let opt_cfg_idx = self.nodeid_to_index.get(&node_id).map(|&i|i); + let opt_cfg_idx = self.nodeid_to_index.get(&node_id); match opt_cfg_idx { - Some(cfg_idx) => { - let (start, end) = self.compute_id_range(cfg_idx); - let kills = &self.kills[start.. end]; - if bitwise(&mut orig_kills, kills, &Union) { - changed = true; + Some(indices) => { + for &cfg_idx in indices { + let (start, end) = self.compute_id_range(cfg_idx); + let kills = &self.kills[start.. end]; + if bitwise(&mut orig_kills, kills, &Union) { + changed = true; + } } } None => { @@ -482,7 +505,7 @@ impl<'a, 'b, 'tcx, O:DataFlowOperator> PropagationContext<'a, 'b, 'tcx, O> { cfg.graph.each_node(|node_index, node| { debug!("DataFlowContext::walk_cfg idx={:?} id={} begin in_out={}", - node_index, node.data.id, bits_to_string(in_out)); + node_index, node.data.id(), bits_to_string(in_out)); let (start, end) = self.dfcx.compute_id_range(node_index); diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index 5cc7502b5128d..fe1ad16c738df 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -957,7 +957,9 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,'tcx,TYPER> { let mut mode = Unknown; self.determine_pat_move_mode(cmt_discr.clone(), pat, &mut mode); let mode = mode.match_mode(); - self.walk_pat(cmt_discr, pat, mode); + + self.walk_pat(cmt_discr.clone(), pat, mode); + } /// Identifies any bindings within `pat` and accumulates within @@ -1006,6 +1008,7 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,'tcx,TYPER> { let typer = self.typer; let def_map = &self.tcx().def_map; let delegate = &mut self.delegate; + return_if_err!(mc.cat_pattern(cmt_discr.clone(), pat, |mc, cmt_pat, pat| { if pat_util::pat_is_binding(def_map, pat) { let tcx = typer.tcx(); @@ -1043,6 +1046,7 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,'tcx,TYPER> { debug!("walk_pat binding consuming pat"); delegate.consume_pat(pat, cmt_pat, mode); } + _ => { tcx.sess.span_bug( pat.span, diff --git a/src/librustc/middle/graph.rs b/src/librustc/middle/graph.rs index 989efdd235dff..f98742d73d8cc 100644 --- a/src/librustc/middle/graph.rs +++ b/src/librustc/middle/graph.rs @@ -61,12 +61,12 @@ impl Debug for Edge { } } -#[derive(Clone, Copy, PartialEq, Debug)] +#[derive(Clone, Copy, PartialEq, Debug, Eq, Hash)] pub struct NodeIndex(pub uint); #[allow(non_upper_case_globals)] pub const InvalidNodeIndex: NodeIndex = NodeIndex(uint::MAX); -#[derive(Copy, PartialEq, Debug)] +#[derive(Copy, PartialEq, Debug, Eq, Hash)] pub struct EdgeIndex(pub uint); #[allow(non_upper_case_globals)] pub const InvalidEdgeIndex: EdgeIndex = EdgeIndex(uint::MAX); diff --git a/src/librustc/middle/pat_util.rs b/src/librustc/middle/pat_util.rs index 01dc55c3eeec0..a7df2f4a5dafb 100644 --- a/src/librustc/middle/pat_util.rs +++ b/src/librustc/middle/pat_util.rs @@ -119,6 +119,21 @@ pub fn pat_contains_bindings(dm: &DefMap, pat: &ast::Pat) -> bool { contains_bindings } +/// Checks if the pattern contains any patterns that bind something to +/// an ident or wildcard, e.g. `foo`, or `Foo(_)`, `foo @ Bar(..)`, +pub fn pat_contains_bindings_or_wild(dm: &DefMap, pat: &ast::Pat) -> bool { + let mut contains_bindings = false; + walk_pat(pat, |p| { + if pat_is_binding_or_wild(dm, p) { + contains_bindings = true; + false // there's at least one binding/wildcard, can short circuit now. + } else { + true + } + }); + contains_bindings +} + pub fn simple_identifier<'a>(pat: &'a ast::Pat) -> Option<&'a ast::Ident> { match pat.node { ast::PatIdent(ast::BindByValue(_), ref path1, None) => { diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index 0bad55948822f..0f46f3438fed5 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -614,13 +614,24 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { }; let (suggestion, _) = move_suggestion(param_env, expr_span, expr_ty, ("moved by default", "")); - self.tcx.sess.span_note( - expr_span, - &format!("`{}` moved here{} because it has type `{}`, which is {}", - ol, - moved_lp_msg, - expr_ty.user_string(self.tcx), - suggestion)[]); + // If the two spans are the same, it's because the expression will be evaluated + // multiple times. Avoid printing the same span and adjust the wording so it makes + // more sense that it's from multiple evalutations. + if expr_span == use_span { + self.tcx.sess.note( + &format!("`{}` was previously moved here{} because it has type `{}`, \ + which is {}", ol, moved_lp_msg, + expr_ty.user_string(self.tcx), + suggestion)[]); + } else { + self.tcx.sess.span_note( + expr_span, + &format!("`{}` moved here{} because it has type `{}`, which is {}", + ol, + moved_lp_msg, + expr_ty.user_string(self.tcx), + suggestion)[]); + } } move_data::MovePat => { diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index 0edc2c213fb1d..367565a66a1f1 100644 --- a/src/librustc_trans/trans/base.rs +++ b/src/librustc_trans/trans/base.rs @@ -1379,7 +1379,7 @@ fn build_cfg(tcx: &ty::ctxt, id: ast::NodeId) -> (ast::NodeId, Option) // the clobbering of the existing value in the return slot. fn has_nested_returns(tcx: &ty::ctxt, cfg: &cfg::CFG, blk_id: ast::NodeId) -> bool { for n in cfg.graph.depth_traverse(cfg.entry) { - match tcx.map.find(n.id) { + match tcx.map.find(n.id()) { Some(ast_map::NodeExpr(ex)) => { if let ast::ExprRet(Some(ref ret_expr)) = ex.node { let mut visitor = FindNestedReturn::new(); diff --git a/src/libsyntax/ext/build.rs b/src/libsyntax/ext/build.rs index a7d1baf08beac..e98ecfc504fb5 100644 --- a/src/libsyntax/ext/build.rs +++ b/src/libsyntax/ext/build.rs @@ -862,7 +862,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> { attrs: vec!(), pats: pats, guard: None, - body: expr + body: expr, } } diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index fd7593f2a3b52..c32f7ade114b5 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -155,7 +155,7 @@ pub fn expand_expr(e: P, fld: &mut MacroExpander) -> P { attrs: vec![], pats: vec![pat_under], guard: Some(cond), - body: fld.cx.expr_block(then) + body: fld.cx.expr_block(then), }); elseopt.map(|elseopt| (elseopt, true)) } diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index b0ddb655882a8..bb1a1e910106c 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -373,12 +373,12 @@ pub fn noop_fold_view_path(view_path: P, fld: &mut T) -> P< }) } -pub fn noop_fold_arm(Arm {attrs, pats, guard, body}: Arm, fld: &mut T) -> Arm { +pub fn noop_fold_arm(arm: Arm, fld: &mut T) -> Arm { Arm { - attrs: attrs.move_map(|x| fld.fold_attribute(x)), - pats: pats.move_map(|x| fld.fold_pat(x)), - guard: guard.map(|x| fld.fold_expr(x)), - body: fld.fold_expr(body), + attrs: arm.attrs.move_map(|x| fld.fold_attribute(x)), + pats: arm.pats.move_map(|x| fld.fold_pat(x)), + guard: arm.guard.map(|x| fld.fold_expr(x)), + body: fld.fold_expr(arm.body), } } diff --git a/src/test/compile-fail/move-in-guard-1.rs b/src/test/compile-fail/move-in-guard-1.rs new file mode 100644 index 0000000000000..411688f914f81 --- /dev/null +++ b/src/test/compile-fail/move-in-guard-1.rs @@ -0,0 +1,25 @@ +// Copyright 2015 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. + +#![feature(box_syntax)] + +fn main() { + let x = box 1; + + let v = (1, 2); + + match v { + (1, _) if take(x) => (), + (_, 2) if take(x) => (), //~ ERROR use of moved value: `x` + _ => (), + } +} + +fn take(_: T) -> bool { false } diff --git a/src/test/compile-fail/move-in-guard-2.rs b/src/test/compile-fail/move-in-guard-2.rs new file mode 100644 index 0000000000000..f5b26ae1f8a9c --- /dev/null +++ b/src/test/compile-fail/move-in-guard-2.rs @@ -0,0 +1,25 @@ +// Copyright 2015 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. + +#![feature(box_syntax)] + +fn main() { + let x = box 1; + + let v = (1, 2); + + match v { + (1, _) | + (_, 2) if take(x) => (), //~ ERROR use of moved value: `x` + _ => (), + } +} + +fn take(_: T) -> bool { false } diff --git a/src/test/run-pass/move-guard-const.rs b/src/test/run-pass/move-guard-const.rs new file mode 100644 index 0000000000000..64c4f1fdbaeab --- /dev/null +++ b/src/test/run-pass/move-guard-const.rs @@ -0,0 +1,25 @@ +// Copyright 2015 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. + +#![feature(box_syntax)] + +fn main() { + let x = box 1; + + let v = (1, 2); + + match v { + (2, 1) if take(x) => (), + (1, 2) if take(x) => (), + _ => (), + } +} + +fn take(_: T) -> bool { false } From 7c28a142b0c81f7f9538c3bfb666f19cc1dc89a6 Mon Sep 17 00:00:00 2001 From: James Miller Date: Sat, 7 Feb 2015 23:12:17 +1300 Subject: [PATCH 2/3] Allow for customisation of the nodes when generating a Graphviz file. This allows the implementor of the Labeller trait to provide a set of attributes for the node. --- src/libgraphviz/lib.rs | 114 +++++++++++++++++++++++++++- src/librustc/middle/cfg/graphviz.rs | 58 ++++++++++---- src/librustc_borrowck/graphviz.rs | 22 +++--- 3 files changed, 168 insertions(+), 26 deletions(-) diff --git a/src/libgraphviz/lib.rs b/src/libgraphviz/lib.rs index 2d94ddaef1835..398fa7e6d2bbc 100644 --- a/src/libgraphviz/lib.rs +++ b/src/libgraphviz/lib.rs @@ -282,6 +282,7 @@ use self::LabelText::*; use std::borrow::IntoCow; +use std::fmt; use std::old_io; use std::string::CowString; use std::vec::CowVec; @@ -418,15 +419,119 @@ pub trait Labeller<'a,N,E> { LabelStr(self.node_id(n).name) } + fn node_attrs(&self, _: &N) -> NodeAttributes { + NodeAttributes::default() + } + /// Maps `e` to a label that will be used in the rendered output. /// The label need not be unique, and may be the empty string; the /// default is in fact the empty string. - fn edge_label(&'a self, e: &E) -> LabelText<'a> { - let _ignored = e; + fn edge_label(&'a self, _: &E) -> LabelText<'a> { LabelStr("".into_cow()) } } +#[derive(Copy,Clone)] +pub enum NodeShape { + Text, + Ellipse, + Oval, + Circle, + Egg, + Triangle, + Box, + Diamond, + Trapezium, + Parallelogram, + House, + Hexagon, + Octagon, + Note, + Tab, + Polygon { + sides: u8, + regular: bool + } +} + +#[derive(Copy,Clone)] +pub enum NodeStyle { + Filled, + Solid, + Dashed, + Dotted, + Bold, + Invis +} + +#[derive(Copy,Clone)] +pub struct NodeAttributes { + pub shape: NodeShape, + pub color: &'static str, + pub fillcolor: Option<&'static str>, + pub fontcolor: &'static str, + pub style: NodeStyle, +} + +impl NodeAttributes { + pub fn default() -> NodeAttributes { + NodeAttributes { + shape: NodeShape::Ellipse, + color: "black", + fillcolor: None, + fontcolor: "black", + style: NodeStyle::Solid, + } + } +} + +impl fmt::Display for NodeAttributes { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use self::NodeShape::*; + use self::NodeStyle::*; + + try!(f.write_str("shape=")); + match self.shape { + Text => try!(f.write_str("text")), + Ellipse => try!(f.write_str("ellipse")), + Oval => try!(f.write_str("oval")), + Circle => try!(f.write_str("circle")), + Egg => try!(f.write_str("egg")), + Triangle => try!(f.write_str("triangle")), + Box => try!(f.write_str("box")), + Diamond => try!(f.write_str("diamond")), + Trapezium => try!(f.write_str("trapezium")), + Parallelogram => try!(f.write_str("parallelogram")), + House => try!(f.write_str("house")), + Hexagon => try!(f.write_str("hexagon")), + Octagon => try!(f.write_str("octagon")), + Note => try!(f.write_str("note")), + Tab => try!(f.write_str("tab")), + Polygon { sides, regular } => { + try!(f.write_str("polygon,")); + try!(write!(f, "sides={}", sides)); + try!(write!(f, "regular={}", if regular { 1 } else { 0 })); + } + } + + try!(write!(f, ",color={}", self.color)); + if let Some(color) = self.fillcolor { + try!(write!(f, ",fillcolor={}", color)); + } + try!(write!(f, ",fontcolor={}", self.fontcolor)); + + try!(f.write_str(",style=")); + match self.style { + Filled => f.write_str("filled"), + Solid => f.write_str("solid"), + Dashed => f.write_str("dashed"), + Dotted => f.write_str("dotted"), + Bold => f.write_str("bold"), + Invis => f.write_str("invis"), + } + } +} + impl<'a> LabelText<'a> { pub fn label>(s: S) -> LabelText<'a> { LabelStr(s.into_cow()) @@ -561,8 +666,9 @@ pub fn render_opts<'a, N:Clone+'a, E:Clone+'a, G:Labeller<'a,N,E>+GraphWalk<'a,N try!(writeln(w, &[id.as_slice(), ";"])); } else { let escaped = g.node_label(n).escape(); - try!(writeln(w, &[id.as_slice(), - "[label=\"", &escaped, "\"];"])); + let attrs = g.node_attrs(n); + try!(write!(w, "{}[label=\"{}\",{}];\n", + id.as_slice(), &escaped, attrs)); } } diff --git a/src/librustc/middle/cfg/graphviz.rs b/src/librustc/middle/cfg/graphviz.rs index 1f0fe4f1acaea..13ba4e5ab9666 100644 --- a/src/librustc/middle/cfg/graphviz.rs +++ b/src/librustc/middle/cfg/graphviz.rs @@ -60,21 +60,54 @@ impl<'a, 'ast> dot::Labeller<'a, Node<'a>, Edge<'a>> for LabelledCFG<'a, 'ast> { dot::Id::new(format!("N{}", i.node_id())).ok().unwrap() } - fn node_label(&'a self, &(i, n): &Node<'a>) -> dot::LabelText<'a> { - if i == self.cfg.entry { - dot::LabelText::LabelStr("entry".into_cow()) - } else if i == self.cfg.exit { - dot::LabelText::LabelStr("exit".into_cow()) - } else if n.data.id == ast::DUMMY_NODE_ID { - dot::LabelText::LabelStr("(dummy_node)".into_cow()) - } else { - let s = self.ast_map.node_to_string(n.data.id); - // left-aligns the lines - let s = replace_newline_with_backslash_l(s); - dot::LabelText::EscStr(s.into_cow()) + fn node_label(&'a self, &(_, n): &Node<'a>) -> dot::LabelText<'a> { + match n.data { + cfg::CFGNodeData::Entry => dot::LabelText::LabelStr("".into_cow()), + cfg::CFGNodeData::Exit => dot::LabelText::LabelStr("".into_cow()), + cfg::CFGNodeData::Dummy => dot::LabelText::LabelStr("".into_cow()), + cfg::CFGNodeData::Unreachable => dot::LabelText::LabelStr("".into_cow()), + cfg::CFGNodeData::AST(id) if id == ast::DUMMY_NODE_ID => { + dot::LabelText::LabelStr("".into_cow()) + } + cfg::CFGNodeData::AST(id) => { + let s = self.ast_map.node_to_string(id); + // left-aligns the lines + let s = replace_newline_with_backslash_l(s); + dot::LabelText::EscStr(s.into_cow()) + } } } + fn node_attrs(&self, &(_, n): &Node) -> dot::NodeAttributes { + let mut attrs = dot::NodeAttributes::default(); + + match n.data { + cfg::CFGNodeData::Entry | + cfg::CFGNodeData::Exit => { + attrs.shape = dot::NodeShape::Diamond; + } + cfg::CFGNodeData::Dummy => { + attrs.shape = dot::NodeShape::Circle; + attrs.style = dot::NodeStyle::Filled; + attrs.color = "blue"; + } + cfg::CFGNodeData::Unreachable => { + attrs.color = "red"; + attrs.fontcolor = "red"; + } + cfg::CFGNodeData::AST(id) if id == ast::DUMMY_NODE_ID => { + attrs.shape = dot::NodeShape::Circle; + attrs.style = dot::NodeStyle::Filled; + attrs.color = "blue"; + } + cfg::CFGNodeData::AST(_) => { + attrs.shape = dot::NodeShape::Box; + } + } + + attrs + } + fn edge_label(&self, e: &Edge<'a>) -> dot::LabelText<'a> { let mut label = String::new(); if !self.labelled_edges { @@ -124,4 +157,3 @@ impl<'a, 'ast> dot::GraphWalk<'a, Node<'a>, Edge<'a>> for LabelledCFG<'a, 'ast> fn source(&'a self, edge: &Edge<'a>) -> Node<'a> { self.cfg.source(edge) } fn target(&'a self, edge: &Edge<'a>) -> Node<'a> { self.cfg.target(edge) } } - diff --git a/src/librustc_borrowck/graphviz.rs b/src/librustc_borrowck/graphviz.rs index 56bf3ae7fd5a3..07e06b8d3cbf8 100644 --- a/src/librustc_borrowck/graphviz.rs +++ b/src/librustc_borrowck/graphviz.rs @@ -20,7 +20,7 @@ use rustc::middle::cfg::graphviz as cfg_dot; use borrowck; use borrowck::{BorrowckCtxt, LoanPath}; use dot; -use rustc::middle::cfg::{CFGIndex}; +use rustc::middle::cfg::{self, CFGIndex}; use rustc::middle::dataflow::{DataFlowOperator, DataFlowContext, EntryOrExit}; use rustc::middle::dataflow; use std::rc::Rc; @@ -52,15 +52,16 @@ pub struct DataflowLabeller<'a, 'tcx: 'a> { impl<'a, 'tcx> DataflowLabeller<'a, 'tcx> { fn dataflow_for(&self, e: EntryOrExit, n: &Node<'a>) -> String { - let id = n.1.data.id; - debug!("dataflow_for({:?}, id={}) {:?}", e, id, self.variants); let mut sets = "".to_string(); - let mut seen_one = false; - for &variant in &self.variants { - if seen_one { sets.push_str(" "); } else { seen_one = true; } - sets.push_str(variant.short_name()); - sets.push_str(": "); - sets.push_str(&self.dataflow_for_variant(e, n, variant)[]); + if let cfg::CFGNodeData::AST(id) = n.1.data { + debug!("dataflow_for({:?}, id={}) {:?}", e, id, self.variants); + let mut seen_one = false; + for &variant in &self.variants { + if seen_one { sets.push_str(" "); } else { seen_one = true; } + sets.push_str(variant.short_name()); + sets.push_str(": "); + sets.push_str(&self.dataflow_for_variant(e, n, variant)[]); + } } sets } @@ -141,6 +142,9 @@ impl<'a, 'tcx> dot::Labeller<'a, Node<'a>, Edge<'a>> for DataflowLabeller<'a, 't .prefix_line(dot::LabelText::LabelStr(prefix.into_cow())) .suffix_line(dot::LabelText::LabelStr(suffix.into_cow())) } + fn node_attrs(&self, n: &Node<'a>) -> dot::NodeAttributes { + self.inner.node_attrs(n) + } fn edge_label(&'a self, e: &Edge<'a>) -> dot::LabelText<'a> { self.inner.edge_label(e) } } From bc9815dabf983bc9d7807fc0b681af519f158608 Mon Sep 17 00:00:00 2001 From: James Miller Date: Sat, 7 Feb 2015 23:14:22 +1300 Subject: [PATCH 3/3] Show matches when suffix is not unique --- src/librustc_driver/pretty.rs | 41 +++++++++++++++++------------------ 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/librustc_driver/pretty.rs b/src/librustc_driver/pretty.rs index 5dfef6c775e1e..8825940cb392c 100644 --- a/src/librustc_driver/pretty.rs +++ b/src/librustc_driver/pretty.rs @@ -388,31 +388,30 @@ impl UserIdentifiedItem { } fn to_one_node_id(self, user_option: &str, sess: &Session, map: &ast_map::Map) -> ast::NodeId { - let fail_because = |is_wrong_because| -> ast::NodeId { - let message = - format!("{} needs NodeId (int) or unique \ - path suffix (b::c::d); got {}, which {}", - user_option, - self.reconstructed_input(), - is_wrong_because); - sess.fatal(&message[]) - }; - - let mut saw_node = ast::DUMMY_NODE_ID; - let mut seen = 0; + let mut seen = Vec::new(); for node in self.all_matching_node_ids(map) { - saw_node = node; - seen += 1; - if seen > 1 { - fail_because("does not resolve uniquely"); - } + seen.push(node); } - if seen == 0 { - fail_because("does not resolve to any item"); + if seen.len() == 0 { + sess.fatal(&format!("{} needs NodeId (int) or unique \ + path suffix (b::c::d); got {}, which does not resolve to any item", + user_option, + self.reconstructed_input())[]); + } else if seen.len() > 1 { + let mut list = String::new(); + for node in seen { + list.push_str(&format!("\n {}", map.node_to_string(node))[]); + } + + sess.fatal(&format!(r#"\ +{} needs NodeId (int) or unique path suffix (b::c::d); +got {} which does not resolve to a unique item + + Candidiates:{}"#, user_option, self.reconstructed_input(), list)[]); } - assert!(seen == 1); - return saw_node; + assert!(seen.len() == 1); + return seen[0]; } }