From 97c1711894ec3d8dfabe04f8e7218e86496b1ff9 Mon Sep 17 00:00:00 2001 From: James Miller Date: Thu, 19 Feb 2015 15:27:25 +0100 Subject: [PATCH] Distinguish between AST and various Dummy nodes in CFG. (Factoring of aatch CFG code, Part 1.) --- src/librustc/lint/builtin.rs | 2 +- src/librustc/middle/cfg/construct.rs | 73 +++++++++++++++------------- src/librustc/middle/cfg/graphviz.rs | 4 +- src/librustc/middle/cfg/mod.rs | 22 +++++++-- src/librustc/middle/dataflow.rs | 6 +-- src/librustc_borrowck/graphviz.rs | 2 +- src/librustc_trans/trans/base.rs | 2 +- 7 files changed, 65 insertions(+), 46 deletions(-) diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs index 1db993fdafd..6d34d546f3d 100644 --- a/src/librustc/lint/builtin.rs +++ b/src/librustc/lint/builtin.rs @@ -1848,7 +1848,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 d95dfb6feae..e8df9dfa1b1 100644 --- a/src/librustc/middle/cfg/construct.rs +++ b/src/librustc/middle/cfg/construct.rs @@ -36,13 +36,13 @@ 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 { @@ -61,10 +61,6 @@ pub fn construct(tcx: &ty::ctxt, exit: fn_exit} } -fn add_initial_dummy_node(g: &mut CFGGraph) -> CFGIndex { - g.add_node(CFGNodeData { id: ast::DUMMY_NODE_ID }) -} - impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { fn block(&mut self, blk: &ast::Block, pred: CFGIndex) -> CFGIndex { let mut stmts_exit = pred; @@ -74,19 +70,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 +111,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(_) => { @@ -178,7 +174,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { 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 +194,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 +215,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 +243,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 +279,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, @@ -323,7 +319,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { // let discr_exit = self.expr(&**discr, pred); // 1 - let expr_exit = self.add_node(expr.id, &[]); + let expr_exit = self.add_ast_node(expr.id, &[]); let mut cond_exit = discr_exit; for arm in arms { cond_exit = self.add_dummy_node(&[cond_exit]); // 2 @@ -354,30 +350,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 +450,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 +477,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,17 +504,26 @@ 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 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 { + fn add_ast_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!(id != ast::DUMMY_NODE_ID); + self.add_node(CFGNodeData::AST(id), preds) + } + + fn add_unreachable_node(&mut self) -> CFGIndex { + self.add_node(CFGNodeData::Unreachable, &[]) + } + + fn add_node(&mut self, data: CFGNodeData, preds: &[CFGIndex]) -> CFGIndex { + let node = self.graph.add_node(data); + if let CFGNodeData::AST(id) = data { assert!(!self.exit_map.contains_key(&id)); self.exit_map.insert(id, node); } diff --git a/src/librustc/middle/cfg/graphviz.rs b/src/librustc/middle/cfg/graphviz.rs index 14c6ff01e0e..0c5eca3c129 100644 --- a/src/librustc/middle/cfg/graphviz.rs +++ b/src/librustc/middle/cfg/graphviz.rs @@ -65,10 +65,10 @@ impl<'a, 'ast> dot::Labeller<'a, Node<'a>, Edge<'a>> for LabelledCFG<'a, 'ast> { 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 { + } 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); + 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()) diff --git a/src/librustc/middle/cfg/mod.rs b/src/librustc/middle/cfg/mod.rs index 0ca146a295e..71f6c738522 100644 --- a/src/librustc/middle/cfg/mod.rs +++ b/src/librustc/middle/cfg/mod.rs @@ -26,9 +26,23 @@ pub struct CFG { 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 + } + } } pub struct CFGEdgeData { @@ -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 cf33cd71365..3f0555e93e6 100644 --- a/src/librustc/middle/dataflow.rs +++ b/src/librustc/middle/dataflow.rs @@ -157,8 +157,8 @@ 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 node.data.id() != ast::DUMMY_NODE_ID { + index.insert(node.data.id(), node_idx); } true }); @@ -482,7 +482,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_borrowck/graphviz.rs b/src/librustc_borrowck/graphviz.rs index 4465000d8d8..a2c9930c0ed 100644 --- a/src/librustc_borrowck/graphviz.rs +++ b/src/librustc_borrowck/graphviz.rs @@ -52,7 +52,7 @@ 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; + let id = n.1.data.id(); debug!("dataflow_for({:?}, id={}) {:?}", e, id, self.variants); let mut sets = "".to_string(); let mut seen_one = false; diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index 9c0aa9f6957..b520031d65c 100644 --- a/src/librustc_trans/trans/base.rs +++ b/src/librustc_trans/trans/base.rs @@ -1353,7 +1353,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();