Skip to content

Commit

Permalink
Auto merge of #44171 - eddyb:scope, r=nikomatsakis
Browse files Browse the repository at this point in the history 
Use hir::ItemLocalId instead of ast::NodeId in rustc::middle::region::CodeExtent.

This is an alternative to @michaelwoerister's #43887, changing `CodeExtent` instead of `ReScope`.

The benefit here is that the same `Region`s are used same-crate and cross-crate, while preserving the incremental recompilation properties of the stable `hir::ItemLocalId`.

Only places which needed to get back to the `ast::NodeId` from `CodeExtent` was its `span` method, used in error reporting - passing the `&RegionMaps` down allowed using `hir_to_node_id`.
`rustc::cfg` and `dataflow` also had to be converted to `hir::ItemLocalId` because of their interactions with `CodeExtent`, especially in `borrowck`, and from that we have 3 more `hir_to_node_id` calls: `cfg::graphviz` node labels, `borrowck` move reporting, and the `unconditional_recursion` lint.

Out of all of those, *only* the lint actually makes a decision (on whether code will compile) based on the result of the conversion, the others only use it to know how to print information to the user.
So I think we're safe to say that the bulk of the code working with a `CodeExtent` is fine with local IDs.

r? @nikomatsakis
  • Loading branch information
@bors
bors committed Sep 1, 2017
2 parents 45d31e7 + e4996ec commit a59a6d8
Show file tree
Hide file tree
Showing 51 changed files with 670 additions and 757 deletions.
93 changes: 47 additions & 46 deletions src/librustc/cfg/construct.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use rustc_data_structures::graph;
use cfg::*;
use middle::region::CodeExtent;
use ty::{self, TyCtxt};
use syntax::ast;
use syntax::ptr::P;

use hir::{self, PatKind};
Expand All @@ -30,13 +29,13 @@ struct CFGBuilder<'a, 'tcx: 'a> {

#[derive(Copy, Clone)]
struct BlockScope {
block_expr_id: ast::NodeId, // id of breakable block expr node
block_expr_id: hir::ItemLocalId, // id of breakable block expr node
break_index: CFGIndex, // where to go on `break`
}

#[derive(Copy, Clone)]
struct LoopScope {
loop_id: ast::NodeId, // id of loop/while node
loop_id: hir::ItemLocalId, // id of loop/while node
continue_index: CFGIndex, // where to go on a `loop`
break_index: CFGIndex, // where to go on a `break`
}
Expand Down Expand Up @@ -70,6 +69,7 @@ pub fn construct<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
cfg_builder.add_contained_edge(body_exit, fn_exit);
let CFGBuilder { graph, .. } = cfg_builder;
CFG {
owner_def_id,
graph,
entry,
exit: fn_exit,
Expand All @@ -79,10 +79,10 @@ pub fn construct<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
fn block(&mut self, blk: &hir::Block, pred: CFGIndex) -> CFGIndex {
if blk.targeted_by_break {
let expr_exit = self.add_ast_node(blk.id, &[]);
let expr_exit = self.add_ast_node(blk.hir_id.local_id, &[]);

self.breakable_block_scopes.push(BlockScope {
block_expr_id: blk.id,
block_expr_id: blk.hir_id.local_id,
break_index: expr_exit,
});

Expand All @@ -104,21 +104,22 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {

let expr_exit = self.opt_expr(&blk.expr, stmts_exit);

self.add_ast_node(blk.id, &[expr_exit])
self.add_ast_node(blk.hir_id.local_id, &[expr_exit])
}
}

fn stmt(&mut self, stmt: &hir::Stmt, pred: CFGIndex) -> CFGIndex {
let hir_id = self.tcx.hir.node_to_hir_id(stmt.node.id());
match stmt.node {
hir::StmtDecl(ref decl, id) => {
hir::StmtDecl(ref decl, _) => {
let exit = self.decl(&decl, pred);
self.add_ast_node(id, &[exit])
self.add_ast_node(hir_id.local_id, &[exit])
}

hir::StmtExpr(ref expr, id) |
hir::StmtSemi(ref expr, id) => {
hir::StmtExpr(ref expr, _) |
hir::StmtSemi(ref expr, _) => {
let exit = self.expr(&expr, pred);
self.add_ast_node(id, &[exit])
self.add_ast_node(hir_id.local_id, &[exit])
}
}
}
Expand All @@ -140,31 +141,31 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
PatKind::Path(_) |
PatKind::Lit(..) |
PatKind::Range(..) |
PatKind::Wild => self.add_ast_node(pat.id, &[pred]),
PatKind::Wild => self.add_ast_node(pat.hir_id.local_id, &[pred]),

PatKind::Box(ref subpat) |
PatKind::Ref(ref subpat, _) |
PatKind::Binding(.., Some(ref subpat)) => {
let subpat_exit = self.pat(&subpat, pred);
self.add_ast_node(pat.id, &[subpat_exit])
self.add_ast_node(pat.hir_id.local_id, &[subpat_exit])
}

PatKind::TupleStruct(_, ref subpats, _) |
PatKind::Tuple(ref subpats, _) => {
let pats_exit = self.pats_all(subpats.iter(), pred);
self.add_ast_node(pat.id, &[pats_exit])
self.add_ast_node(pat.hir_id.local_id, &[pats_exit])
}

PatKind::Struct(_, ref subpats, _) => {
let pats_exit = self.pats_all(subpats.iter().map(|f| &f.node.pat), pred);
self.add_ast_node(pat.id, &[pats_exit])
self.add_ast_node(pat.hir_id.local_id, &[pats_exit])
}

PatKind::Slice(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_ast_node(pat.id, &[post_exit])
self.add_ast_node(pat.hir_id.local_id, &[post_exit])
}
}
}
Expand All @@ -180,7 +181,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
match expr.node {
hir::ExprBlock(ref blk) => {
let blk_exit = self.block(&blk, pred);
self.add_ast_node(expr.id, &[blk_exit])
self.add_ast_node(expr.hir_id.local_id, &[blk_exit])
}

hir::ExprIf(ref cond, ref then, None) => {
Expand All @@ -200,7 +201,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
//
let cond_exit = self.expr(&cond, pred); // 1
let then_exit = self.expr(&then, cond_exit); // 2
self.add_ast_node(expr.id, &[cond_exit, then_exit]) // 3,4
self.add_ast_node(expr.hir_id.local_id, &[cond_exit, then_exit]) // 3,4
}

hir::ExprIf(ref cond, ref then, Some(ref otherwise)) => {
Expand All @@ -221,7 +222,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
let cond_exit = self.expr(&cond, pred); // 1
let then_exit = self.expr(&then, cond_exit); // 2
let else_exit = self.expr(&otherwise, cond_exit); // 3
self.add_ast_node(expr.id, &[then_exit, else_exit]) // 4, 5
self.add_ast_node(expr.hir_id.local_id, &[then_exit, else_exit]) // 4, 5
}

hir::ExprWhile(ref cond, ref body, _) => {
Expand All @@ -245,12 +246,12 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
let loopback = self.add_dummy_node(&[pred]); // 1

// Create expr_exit without pred (cond_exit)
let expr_exit = self.add_ast_node(expr.id, &[]); // 3
let expr_exit = self.add_ast_node(expr.hir_id.local_id, &[]); // 3

// The LoopScope needs to be on the loop_scopes stack while evaluating the
// condition and the body of the loop (both can break out of the loop)
self.loop_scopes.push(LoopScope {
loop_id: expr.id,
loop_id: expr.hir_id.local_id,
continue_index: loopback,
break_index: expr_exit
});
Expand Down Expand Up @@ -282,9 +283,9 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
// may cause additional edges.

let loopback = self.add_dummy_node(&[pred]); // 1
let expr_exit = self.add_ast_node(expr.id, &[]); // 2
let expr_exit = self.add_ast_node(expr.hir_id.local_id, &[]); // 2
self.loop_scopes.push(LoopScope {
loop_id: expr.id,
loop_id: expr.hir_id.local_id,
continue_index: loopback,
break_index: expr_exit,
});
Expand All @@ -295,7 +296,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
}

hir::ExprMatch(ref discr, ref arms, _) => {
self.match_(expr.id, &discr, &arms, pred)
self.match_(expr.hir_id.local_id, &discr, &arms, pred)
}

hir::ExprBinary(op, ref l, ref r) if op.node.is_lazy() => {
Expand All @@ -315,30 +316,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_ast_node(expr.id, &[l_exit, r_exit]) // 3,4
self.add_ast_node(expr.hir_id.local_id, &[l_exit, r_exit]) // 3,4
}

hir::ExprRet(ref v) => {
let v_exit = self.opt_expr(v, pred);
let b = self.add_ast_node(expr.id, &[v_exit]);
let b = self.add_ast_node(expr.hir_id.local_id, &[v_exit]);
self.add_returning_edge(expr, b);
self.add_unreachable_node()
}

hir::ExprBreak(destination, ref opt_expr) => {
let v = self.opt_expr(opt_expr, pred);
let (scope_id, break_dest) =
let (target_scope, break_dest) =
self.find_scope_edge(expr, destination, ScopeCfKind::Break);
let b = self.add_ast_node(expr.id, &[v]);
self.add_exiting_edge(expr, b, scope_id, break_dest);
let b = self.add_ast_node(expr.hir_id.local_id, &[v]);
self.add_exiting_edge(expr, b, target_scope, break_dest);
self.add_unreachable_node()
}

hir::ExprAgain(destination) => {
let (scope_id, cont_dest) =
let (target_scope, cont_dest) =
self.find_scope_edge(expr, destination, ScopeCfKind::Continue);
let a = self.add_ast_node(expr.id, &[pred]);
self.add_exiting_edge(expr, a, scope_id, cont_dest);
let a = self.add_ast_node(expr.hir_id.local_id, &[pred]);
self.add_exiting_edge(expr, a, target_scope, cont_dest);
self.add_unreachable_node()
}

Expand Down Expand Up @@ -397,7 +398,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
hir::ExprInlineAsm(_, ref outputs, ref inputs) => {
let post_outputs = self.exprs(outputs.iter().map(|e| &*e), pred);
let post_inputs = self.exprs(inputs.iter().map(|e| &*e), post_outputs);
self.add_ast_node(expr.id, &[post_inputs])
self.add_ast_node(expr.hir_id.local_id, &[post_inputs])
}

hir::ExprClosure(..) |
Expand Down Expand Up @@ -444,10 +445,10 @@ 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_ast_node(expr.id, &[subexprs_exit])
self.add_ast_node(expr.hir_id.local_id, &[subexprs_exit])
}

fn match_(&mut self, id: ast::NodeId, discr: &hir::Expr,
fn match_(&mut self, id: hir::ItemLocalId, discr: &hir::Expr,
arms: &[hir::Arm], pred: CFGIndex) -> CFGIndex {
// The CFG for match expression is quite complex, so no ASCII
// art for it (yet).
Expand Down Expand Up @@ -552,8 +553,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
self.add_node(CFGNodeData::Dummy, preds)
}

fn add_ast_node(&mut self, id: ast::NodeId, preds: &[CFGIndex]) -> CFGIndex {
assert!(id != ast::DUMMY_NODE_ID);
fn add_ast_node(&mut self, id: hir::ItemLocalId, preds: &[CFGIndex]) -> CFGIndex {
self.add_node(CFGNodeData::AST(id), preds)
}

Expand All @@ -579,14 +579,13 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
fn add_exiting_edge(&mut self,
from_expr: &hir::Expr,
from_index: CFGIndex,
scope_id: ast::NodeId,
target_scope: CodeExtent,
to_index: CFGIndex) {
let mut data = CFGEdgeData { exiting_scopes: vec![] };
let mut scope = CodeExtent::Misc(from_expr.id);
let target_scope = CodeExtent::Misc(scope_id);
let mut scope = CodeExtent::Misc(from_expr.hir_id.local_id);
let region_maps = self.tcx.region_maps(self.owner_def_id);
while scope != target_scope {
data.exiting_scopes.push(scope.node_id());
data.exiting_scopes.push(scope.item_local_id());
scope = region_maps.encl_scope(scope);
}
self.graph.add_edge(from_index, to_index, data);
Expand All @@ -607,13 +606,14 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
fn find_scope_edge(&self,
expr: &hir::Expr,
destination: hir::Destination,
scope_cf_kind: ScopeCfKind) -> (ast::NodeId, CFGIndex) {
scope_cf_kind: ScopeCfKind) -> (CodeExtent, CFGIndex) {

match destination.target_id {
hir::ScopeTarget::Block(block_expr_id) => {
for b in &self.breakable_block_scopes {
if b.block_expr_id == block_expr_id {
return (block_expr_id, match scope_cf_kind {
if b.block_expr_id == self.tcx.hir.node_to_hir_id(block_expr_id).local_id {
let scope_id = self.tcx.hir.node_to_hir_id(block_expr_id).local_id;
return (CodeExtent::Misc(scope_id), match scope_cf_kind {
ScopeCfKind::Break => b.break_index,
ScopeCfKind::Continue => bug!("can't continue to block"),
});
Expand All @@ -623,8 +623,9 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
}
hir::ScopeTarget::Loop(hir::LoopIdResult::Ok(loop_id)) => {
for l in &self.loop_scopes {
if l.loop_id == loop_id {
return (loop_id, match scope_cf_kind {
if l.loop_id == self.tcx.hir.node_to_hir_id(loop_id).local_id {
let scope_id = self.tcx.hir.node_to_hir_id(loop_id).local_id;
return (CodeExtent::Misc(scope_id), match scope_cf_kind {
ScopeCfKind::Break => l.break_index,
ScopeCfKind::Continue => l.continue_index,
});
Expand Down
64 changes: 34 additions & 30 deletions src/librustc/cfg/graphviz.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,40 +15,48 @@
use graphviz as dot;
use graphviz::IntoCow;

use syntax::ast;

use hir::map as hir_map;
use cfg;
use hir;
use ty::TyCtxt;

pub type Node<'a> = (cfg::CFGIndex, &'a cfg::CFGNode);
pub type Edge<'a> = &'a cfg::CFGEdge;

pub struct LabelledCFG<'a, 'hir: 'a> {
pub hir_map: &'a hir_map::Map<'hir>,
pub struct LabelledCFG<'a, 'tcx: 'a> {
pub tcx: TyCtxt<'a, 'tcx, 'tcx>,
pub cfg: &'a cfg::CFG,
pub name: String,
/// `labelled_edges` controls whether we emit labels on the edges
pub labelled_edges: bool,
}

fn replace_newline_with_backslash_l(s: String) -> String {
// Replacing newlines with \\l causes each line to be left-aligned,
// improving presentation of (long) pretty-printed expressions.
if s.contains("\n") {
let mut s = s.replace("\n", "\\l");
// Apparently left-alignment applies to the line that precedes
// \l, not the line that follows; so, add \l at end of string
// if not already present, ensuring last line gets left-aligned
// as well.
let mut last_two: Vec<_> =
s.chars().rev().take(2).collect();
last_two.reverse();
if last_two != ['\\', 'l'] {
s.push_str("\\l");
impl<'a, 'tcx> LabelledCFG<'a, 'tcx> {
fn local_id_to_string(&self, local_id: hir::ItemLocalId) -> String {
assert!(self.cfg.owner_def_id.is_local());
let node_id = self.tcx.hir.hir_to_node_id(hir::HirId {
owner: self.tcx.hir.def_index_to_hir_id(self.cfg.owner_def_id.index).owner,
local_id
});
let s = self.tcx.hir.node_to_string(node_id);

// Replacing newlines with \\l causes each line to be left-aligned,
// improving presentation of (long) pretty-printed expressions.
if s.contains("\n") {
let mut s = s.replace("\n", "\\l");
// Apparently left-alignment applies to the line that precedes
// \l, not the line that follows; so, add \l at end of string
// if not already present, ensuring last line gets left-aligned
// as well.
let mut last_two: Vec<_> =
s.chars().rev().take(2).collect();
last_two.reverse();
if last_two != ['\\', 'l'] {
s.push_str("\\l");
}
s
} else {
s
}
s
} else {
s
}
}

Expand All @@ -66,12 +74,10 @@ impl<'a, 'hir> dot::Labeller<'a> for LabelledCFG<'a, 'hir> {
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() == hir::DUMMY_ITEM_LOCAL_ID {
dot::LabelText::LabelStr("(dummy_node)".into_cow())
} else {
let s = self.hir_map.node_to_string(n.data.id());
// left-aligns the lines
let s = replace_newline_with_backslash_l(s);
let s = self.local_id_to_string(n.data.id());
dot::LabelText::EscStr(s.into_cow())
}
}
Expand All @@ -82,15 +88,13 @@ impl<'a, 'hir> dot::Labeller<'a> for LabelledCFG<'a, 'hir> {
return dot::LabelText::EscStr(label.into_cow());
}
let mut put_one = false;
for (i, &node_id) in e.data.exiting_scopes.iter().enumerate() {
for (i, &id) in e.data.exiting_scopes.iter().enumerate() {
if put_one {
label.push_str(",\\l");
} else {
put_one = true;
}
let s = self.hir_map.node_to_string(node_id);
// left-aligns the lines
let s = replace_newline_with_backslash_l(s);
let s = self.local_id_to_string(id);
label.push_str(&format!("exiting scope_{} {}",
i,
&s[..]));
Expand Down
Loading

0 comments on commit a59a6d8

Please sign in to comment.