mathieu/granite-rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Auto merge of #24330 - pnkfelix:issue-24267, r=nikomatsakis

Browse source 
Extend rustc::middle::dataflow to allow filtering kills from flow-exits.

Fix borrowck analysis so that it will not treat a break that pops through an assignment
```rust
x = { ... break; ... }
```
as a kill of the "moved-out" bit for `x`.

Fix #24267.

[breaking-change], but really, its only breaking code that was already buggy.
This commit is contained in:
bors 2015-04-15 21:05:16 +00:00
commit 07f807d01f
4 changed files with 108 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

80
src/librustc/middle/dataflow.rs
View file

@ -64,8 +64,14 @@ pub struct DataFlowContext<'a, 'tcx: 'a, O> {
/// bits generated as we exit the cfg node. Updated by `add_gen()`.
gens: Vec<usize>,
/// bits killed as we exit the cfg node. Updated by `add_kill()`.
kills: Vec<usize>,
/// bits killed as we exit the cfg node, or non-locally jump over
/// it. Updated by `add_kill(KillFrom::ScopeEnd)`.
scope_kills: Vec<usize>,
/// bits killed as we exit the cfg node directly; if it is jumped
/// over, e.g. via `break`, the kills are not reflected in the
/// jump's effects. Updated by `add_kill(KillFrom::Execution)`.
action_kills: Vec<usize>,
/// bits that are valid on entry to the cfg node. Updated by
/// `propagate()`.
@ -130,15 +136,23 @@ impl<'a, 'tcx, O:DataFlowOperator> pprust::PpAnn for DataFlowContext<'a, 'tcx, O
"".to_string()
};
let kills = &self.kills[start .. end];
let kills_str = if kills.iter().any(|&u| u != 0) {
format!(" kill: {}", bits_to_string(kills))
let action_kills = &self.action_kills[start .. end];
let action_kills_str = if action_kills.iter().any(|&u| u != 0) {
format!(" action_kill: {}", bits_to_string(action_kills))
} else {
"".to_string()
};
try!(ps.synth_comment(format!("id {}: {}{}{}", id, entry_str,
gens_str, kills_str)));
let scope_kills = &self.scope_kills[start .. end];
let scope_kills_str = if scope_kills.iter().any(|&u| u != 0) {
format!(" scope_kill: {}", bits_to_string(scope_kills))
} else {
"".to_string()
};
try!(ps.synth_comment(
format!("id {}: {}{}{}{}", id, entry_str,
gens_str, action_kills_str, scope_kills_str)));
try!(pp::space(&mut ps.s));
}
Ok(())
@ -187,6 +201,25 @@ fn build_nodeid_to_index(decl: Option<&ast::FnDecl>,
}
}
/// Flag used by `add_kill` to indicate whether the provided kill
/// takes effect only when control flows directly through the node in
/// question, or if the kill's effect is associated with any
/// control-flow directly through or indirectly over the node.
#[derive(Copy, Clone, PartialEq, Debug)]
pub enum KillFrom {
/// A `ScopeEnd` kill is one that takes effect when any control
/// flow goes over the node. A kill associated with the end of the
/// scope of a variable declaration `let x;` is an example of a
/// `ScopeEnd` kill.
ScopeEnd,
/// An `Execution` kill is one that takes effect only when control
/// flow goes through the node to completion. A kill associated
/// with an assignment statement `x = expr;` is an example of an
/// `Execution` kill.
Execution,
}
impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
pub fn new(tcx: &'a ty::ctxt<'tcx>,
analysis_name: &'static str,
@ -206,8 +239,10 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
let entry = if oper.initial_value() { usize::MAX } else {0};
let gens: Vec<_> = repeat(0).take(num_nodes * words_per_id).collect();
let kills: Vec<_> = repeat(0).take(num_nodes * words_per_id).collect();
let zeroes: Vec<_> = repeat(0).take(num_nodes * words_per_id).collect();
let gens: Vec<_> = zeroes.clone();
let kills1: Vec<_> = zeroes.clone();
let kills2: Vec<_> = zeroes;
let on_entry: Vec<_> = repeat(entry).take(num_nodes * words_per_id).collect();
let nodeid_to_index = build_nodeid_to_index(decl, cfg);
@ -220,7 +255,8 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
bits_per_id: bits_per_id,
oper: oper,
gens: gens,
kills: kills,
action_kills: kills1,
scope_kills: kills2,
on_entry: on_entry
}
}
@ -240,7 +276,7 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
}
}
pub fn add_kill(&mut self, id: ast::NodeId, bit: usize) {
pub fn add_kill(&mut self, kind: KillFrom, id: ast::NodeId, bit: usize) {
//! Indicates that `id` kills `bit`
debug!("{} add_kill(id={}, bit={})",
self.analysis_name, id, bit);
@ -250,7 +286,10 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
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];
let kills = match kind {
KillFrom::Execution => &mut self.action_kills[start.. end],
KillFrom::ScopeEnd => &mut self.scope_kills[start.. end],
};
set_bit(kills, bit);
}
}
@ -264,7 +303,9 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
let (start, end) = self.compute_id_range(cfgidx);
let gens = &self.gens[start.. end];
bitwise(bits, gens, &Union);
let kills = &self.kills[start.. end];
let kills = &self.action_kills[start.. end];
bitwise(bits, kills, &Subtract);
let kills = &self.scope_kills[start.. end];
bitwise(bits, kills, &Subtract);
debug!("{} apply_gen_kill(cfgidx={:?}, bits={}) [after]",
@ -278,7 +319,8 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
assert!(start < self.gens.len());
assert!(end <= self.gens.len());
assert!(self.gens.len() == self.kills.len());
assert!(self.gens.len() == self.action_kills.len());
assert!(self.gens.len() == self.scope_kills.len());
assert!(self.gens.len() == self.on_entry.len());
(start, end)
@ -412,7 +454,7 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
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[start.. end].to_vec();
let mut orig_kills = self.scope_kills[start.. end].to_vec();
let mut changed = false;
for &node_id in &edge.data.exiting_scopes {
@ -421,8 +463,12 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
Some(indices) => {
for &cfg_idx in indices {
let (start, end) = self.compute_id_range(cfg_idx);
let kills = &self.kills[start.. end];
let kills = &self.scope_kills[start.. end];
if bitwise(&mut orig_kills, kills, &Union) {
debug!("scope exits: scope id={} \
(node={:?} of {:?}) added killset: {}",
node_id, cfg_idx, indices,
bits_to_string(kills));
changed = true;
}
}
@ -436,7 +482,7 @@ impl<'a, 'tcx, O:DataFlowOperator> DataFlowContext<'a, 'tcx, O> {
}
if changed {
let bits = &mut self.kills[start.. end];
let bits = &mut self.scope_kills[start.. end];
debug!("{} add_kills_from_flow_exits flow_exit={:?} bits={} [before]",
self.analysis_name, flow_exit, mut_bits_to_string(bits));
bits.clone_from_slice(&orig_kills[..]);

3
src/librustc_borrowck/borrowck/mod.rs
View file

@ -24,6 +24,7 @@ use rustc::middle::cfg;
use rustc::middle::dataflow::DataFlowContext;
use rustc::middle::dataflow::BitwiseOperator;
use rustc::middle::dataflow::DataFlowOperator;
use rustc::middle::dataflow::KillFrom;
use rustc::middle::expr_use_visitor as euv;
use rustc::middle::mem_categorization as mc;
use rustc::middle::region;
@ -167,7 +168,7 @@ fn build_borrowck_dataflow_data<'a, 'tcx>(this: &mut BorrowckCtxt<'a, 'tcx>,
all_loans.len());
for (loan_idx, loan) in all_loans.iter().enumerate() {
loan_dfcx.add_gen(loan.gen_scope.node_id(), loan_idx);
loan_dfcx.add_kill(loan.kill_scope.node_id(), loan_idx);
loan_dfcx.add_kill(KillFrom::ScopeEnd, loan.kill_scope.node_id(), loan_idx);
}
loan_dfcx.add_kills_from_flow_exits(cfg);
loan_dfcx.propagate(cfg, body);

19
src/librustc_borrowck/borrowck/move_data.rs
View file

@ -18,6 +18,7 @@ use rustc::middle::cfg;
use rustc::middle::dataflow::DataFlowContext;
use rustc::middle::dataflow::BitwiseOperator;
use rustc::middle::dataflow::DataFlowOperator;
use rustc::middle::dataflow::KillFrom;
use rustc::middle::expr_use_visitor as euv;
use rustc::middle::ty;
use rustc::util::nodemap::{FnvHashMap, NodeSet};
@ -473,11 +474,13 @@ impl<'tcx> MoveData<'tcx> {
for (i, assignment) in self.var_assignments.borrow().iter().enumerate() {
dfcx_assign.add_gen(assignment.id, i);
self.kill_moves(assignment.path, assignment.id, dfcx_moves);
self.kill_moves(assignment.path, assignment.id,
KillFrom::Execution, dfcx_moves);
}
for assignment in &*self.path_assignments.borrow() {
self.kill_moves(assignment.path, assignment.id, dfcx_moves);
self.kill_moves(assignment.path, assignment.id,
KillFrom::Execution, dfcx_moves);
}
// Kill all moves related to a variable `x` when
@ -487,7 +490,8 @@ impl<'tcx> MoveData<'tcx> {
LpVar(..) | LpUpvar(..) | LpDowncast(..) => {
let kill_scope = path.loan_path.kill_scope(tcx);
let path = *self.path_map.borrow().get(&path.loan_path).unwrap();
self.kill_moves(path, kill_scope.node_id(), dfcx_moves);
self.kill_moves(path, kill_scope.node_id(),
KillFrom::ScopeEnd, dfcx_moves);
}
LpExtend(..) => {}
}
@ -500,7 +504,9 @@ impl<'tcx> MoveData<'tcx> {
match lp.kind {
LpVar(..) | LpUpvar(..) | LpDowncast(..) => {
let kill_scope = lp.kill_scope(tcx);
dfcx_assign.add_kill(kill_scope.node_id(), assignment_index);
dfcx_assign.add_kill(KillFrom::ScopeEnd,
kill_scope.node_id(),
assignment_index);
}
LpExtend(..) => {
tcx.sess.bug("var assignment for non var path");
@ -568,6 +574,7 @@ impl<'tcx> MoveData<'tcx> {
fn kill_moves(&self,
path: MovePathIndex,
kill_id: ast::NodeId,
kill_kind: KillFrom,
dfcx_moves: &mut MoveDataFlow) {
// We can only perform kills for paths that refer to a unique location,
// since otherwise we may kill a move from one location with an
@ -576,7 +583,9 @@ impl<'tcx> MoveData<'tcx> {
let loan_path = self.path_loan_path(path);
if loan_path_is_precise(&*loan_path) {
self.each_applicable_move(path, |move_index| {
dfcx_moves.add_kill(kill_id, move_index.get());
debug!("kill_moves add_kill {:?} kill_id={} move_index={}",
kill_kind, kill_id, move_index.get());
dfcx_moves.add_kill(kill_kind, kill_id, move_index.get());
true
});
}

29
src/test/compile-fail/issue-24267-flow-exit.rs Normal file
View file

@ -0,0 +1,29 @@
// 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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// Ensure that we reject code when a nonlocal exit (`break`,
// `continue`) causes us to pop over a needed assignment.
pub fn main() {
foo1();
foo2();
}
pub fn foo1() {
let x: i32;
loop { x = break; }
println!("{}", x); //~ ERROR use of possibly uninitialized variable: `x`
}
pub fn foo2() {
let x: i32;
for _ in 0..10 { x = continue; }
println!("{}", x); //~ ERROR use of possibly uninitialized variable: `x`
}