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.)
This commit is contained in:
Felix S. Klock II 2014-05-21 14:49:16 +02:00
parent fef63e2f23
commit 75340f4176
5 changed files with 337 additions and 605 deletions

View File

@ -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,16 +763,18 @@ fn is_statement_scope(tcx: &ty::ctxt, region: ty::Region) -> bool {
}
}
impl BitwiseOperator for LoanDataFlowOperator {
#[inline]
fn join(&self, succ: uint, pred: uint) -> uint {
succ | pred // loans from both preds are in scope
}
}
impl DataFlowOperator for LoanDataFlowOperator {
#[inline]
fn initial_value(&self) -> bool {
false // no loans in scope by default
}
#[inline]
fn join(&self, succ: uint, pred: uint) -> uint {
succ | pred // loans from both preds are in scope
}
}
impl Repr for Loan {

View File

@ -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
}
}

File diff suppressed because it is too large Load Diff

View File

@ -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,

View File

@ -55,7 +55,7 @@ pub struct Edge<E> {
pub data: E,
}
#[deriving(PartialEq, Show)]
#[deriving(Clone, PartialEq, Show)]
pub struct NodeIndex(pub uint);
pub static InvalidNodeIndex: NodeIndex = NodeIndex(uint::MAX);