OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Rollup merge of #45877 - mikhail-m1:mir-borrowck-act-on-moved, r=arielb1

Browse Source 
restore move out dataflow, add report of move out errors

fix https://github.com/rust-lang/rust/issues/45363
r? @arielb1
This commit is contained in:
Guillaume Gomez 2017-11-11 13:38:07 +01:00 committed by GitHub
commit 5d07d73ffb
7 changed files with 301 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

107
src/librustc_mir/borrow_check.rs
View File

@ -30,9 +30,10 @@ use dataflow::{do_dataflow};
use dataflow::{MoveDataParamEnv};
use dataflow::{BitDenotation, BlockSets, DataflowResults, DataflowResultsConsumer};
use dataflow::{MaybeInitializedLvals, MaybeUninitializedLvals};
use dataflow::{MovingOutStatements};
use dataflow::{Borrows, BorrowData, BorrowIndex};
use dataflow::move_paths::{MoveError, IllegalMoveOriginKind};
use dataflow::move_paths::{HasMoveData, MoveData, MovePathIndex, LookupResult};
use dataflow::move_paths::{HasMoveData, MoveData, MovePathIndex, LookupResult, MoveOutIndex};
use util::borrowck_errors::{BorrowckErrors, Origin};
use self::MutateMode::{JustWrite, WriteAndRead};
@ -129,6 +130,9 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
let flow_uninits = do_dataflow(tcx, mir, id, &attributes, &dead_unwinds,
MaybeUninitializedLvals::new(tcx, mir, &mdpe),
|bd, i| &bd.move_data().move_paths[i]);
let flow_move_outs = do_dataflow(tcx, mir, id, &attributes, &dead_unwinds,
MovingOutStatements::new(tcx, mir, &mdpe),
|bd, i| &bd.move_data().moves[i]);
let mut mbcx = MirBorrowckCtxt {
tcx: tcx,
@ -141,7 +145,8 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
let mut state = InProgress::new(flow_borrows,
flow_inits,
flow_uninits);
flow_uninits,
flow_move_outs);
mbcx.analyze_results(&mut state); // entry point for DataflowResultsConsumer
}
@ -161,6 +166,7 @@ pub struct InProgress<'b, 'gcx: 'tcx, 'tcx: 'b> {
borrows: FlowInProgress<Borrows<'b, 'gcx, 'tcx>>,
inits: FlowInProgress<MaybeInitializedLvals<'b, 'gcx, 'tcx>>,
uninits: FlowInProgress<MaybeUninitializedLvals<'b, 'gcx, 'tcx>>,
move_outs: FlowInProgress<MovingOutStatements<'b, 'gcx, 'tcx>>,
}
struct FlowInProgress<BD> where BD: BitDenotation {
@ -185,7 +191,8 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> DataflowResultsConsumer<'b, 'tcx>
fn reset_to_entry_of(&mut self, bb: BasicBlock, flow_state: &mut Self::FlowState) {
flow_state.each_flow(|b| b.reset_to_entry_of(bb),
|i| i.reset_to_entry_of(bb),
|u| u.reset_to_entry_of(bb));
|u| u.reset_to_entry_of(bb),
|m| m.reset_to_entry_of(bb));
}
fn reconstruct_statement_effect(&mut self,
@ -193,7 +200,8 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> DataflowResultsConsumer<'b, 'tcx>
flow_state: &mut Self::FlowState) {
flow_state.each_flow(|b| b.reconstruct_statement_effect(location),
|i| i.reconstruct_statement_effect(location),
|u| u.reconstruct_statement_effect(location));
|u| u.reconstruct_statement_effect(location),
|m| m.reconstruct_statement_effect(location));
}
fn apply_local_effect(&mut self,
@ -201,7 +209,8 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> DataflowResultsConsumer<'b, 'tcx>
flow_state: &mut Self::FlowState) {
flow_state.each_flow(|b| b.apply_local_effect(),
|i| i.apply_local_effect(),
|u| u.apply_local_effect());
|u| u.apply_local_effect(),
|m| m.apply_local_effect());
}
fn reconstruct_terminator_effect(&mut self,
@ -209,7 +218,8 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> DataflowResultsConsumer<'b, 'tcx>
flow_state: &mut Self::FlowState) {
flow_state.each_flow(|b| b.reconstruct_terminator_effect(location),
|i| i.reconstruct_terminator_effect(location),
|u| u.reconstruct_terminator_effect(location));
|u| u.reconstruct_terminator_effect(location),
|m| m.reconstruct_terminator_effect(location));
}
fn visit_block_entry(&mut self,
@ -671,6 +681,7 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> MirBorrowckCtxt<'c, 'b, 'a, 'gcx, 'tcx>
let lvalue = self.base_path(lvalue_span.0);
let maybe_uninits = &flow_state.uninits;
let curr_move_outs = &flow_state.move_outs.curr_state;
// Bad scenarios:
//
@ -712,7 +723,9 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> MirBorrowckCtxt<'c, 'b, 'a, 'gcx, 'tcx>
match self.move_path_closest_to(lvalue) {
Ok(mpi) => {
if maybe_uninits.curr_state.contains(&mpi) {
self.report_use_of_moved(context, desired_action, lvalue_span);
self.report_use_of_moved_or_uninitialized(context, desired_action,
lvalue_span, mpi,
curr_move_outs);
return; // don't bother finding other problems.
}
}
@ -737,8 +750,10 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> MirBorrowckCtxt<'c, 'b, 'a, 'gcx, 'tcx>
debug!("check_if_path_is_moved part2 lvalue: {:?}", lvalue);
if let Some(mpi) = self.move_path_for_lvalue(lvalue) {
if let Some(_) = maybe_uninits.has_any_child_of(mpi) {
self.report_use_of_moved(context, desired_action, lvalue_span);
if let Some(child_mpi) = maybe_uninits.has_any_child_of(mpi) {
self.report_use_of_moved_or_uninitialized(context, desired_action,
lvalue_span, child_mpi,
curr_move_outs);
return; // don't bother finding other problems.
}
}
@ -1083,17 +1098,47 @@ mod prefixes {
}
impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> MirBorrowckCtxt<'c, 'b, 'a, 'gcx, 'tcx> {
fn report_use_of_moved(&mut self,
fn report_use_of_moved_or_uninitialized(&mut self,
_context: Context,
desired_action: &str,
(lvalue, span): (&Lvalue, Span)) {
self.tcx.cannot_act_on_uninitialized_variable(span,
desired_action,
&self.describe_lvalue(lvalue),
Origin::Mir)
.span_label(span, format!("use of possibly uninitialized `{}`",
self.describe_lvalue(lvalue)))
.emit();
(lvalue, span): (&Lvalue, Span),
mpi: MovePathIndex,
curr_move_out: &IdxSetBuf<MoveOutIndex>) {
let mois = self.move_data.path_map[mpi].iter().filter(
|moi| curr_move_out.contains(moi)).collect::<Vec<_>>();
if mois.is_empty() {
self.tcx.cannot_act_on_uninitialized_variable(span,
desired_action,
&self.describe_lvalue(lvalue),
Origin::Mir)
.span_label(span, format!("use of possibly uninitialized `{}`",
self.describe_lvalue(lvalue)))
.emit();
} else {
let msg = ""; //FIXME: add "partially " or "collaterally "
let mut err = self.tcx.cannot_act_on_moved_value(span,
desired_action,
msg,
&self.describe_lvalue(lvalue),
Origin::Mir);
err.span_label(span, format!("value {} here after move", desired_action));
for moi in mois {
let move_msg = ""; //FIXME: add " (into closure)"
let move_span = self.mir.source_info(self.move_data.moves[*moi].source).span;
if span == move_span {
err.span_label(span,
format!("value moved{} here in previous iteration of loop",
move_msg));
} else {
err.span_label(move_span, format!("value moved{} here", move_msg));
};
}
//FIXME: add note for closure
err.emit();
}
}
fn report_move_out_while_borrowed(&mut self,
@ -1396,26 +1441,31 @@ impl ContextKind {
impl<'b, 'gcx, 'tcx> InProgress<'b, 'gcx, 'tcx> {
pub(super) fn new(borrows: DataflowResults<Borrows<'b, 'gcx, 'tcx>>,
inits: DataflowResults<MaybeInitializedLvals<'b, 'gcx, 'tcx>>,
uninits: DataflowResults<MaybeUninitializedLvals<'b, 'gcx, 'tcx>>)
uninits: DataflowResults<MaybeUninitializedLvals<'b, 'gcx, 'tcx>>,
move_out: DataflowResults<MovingOutStatements<'b, 'gcx, 'tcx>>)
-> Self {
InProgress {
borrows: FlowInProgress::new(borrows),
inits: FlowInProgress::new(inits),
uninits: FlowInProgress::new(uninits),
move_outs: FlowInProgress::new(move_out)
}
}
fn each_flow<XB, XI, XU>(&mut self,
mut xform_borrows: XB,
mut xform_inits: XI,
mut xform_uninits: XU) where
fn each_flow<XB, XI, XU, XM>(&mut self,
mut xform_borrows: XB,
mut xform_inits: XI,
mut xform_uninits: XU,
mut xform_move_outs: XM) where
XB: FnMut(&mut FlowInProgress<Borrows<'b, 'gcx, 'tcx>>),
XI: FnMut(&mut FlowInProgress<MaybeInitializedLvals<'b, 'gcx, 'tcx>>),
XU: FnMut(&mut FlowInProgress<MaybeUninitializedLvals<'b, 'gcx, 'tcx>>),
XM: FnMut(&mut FlowInProgress<MovingOutStatements<'b, 'gcx, 'tcx>>),
{
xform_borrows(&mut self.borrows);
xform_inits(&mut self.inits);
xform_uninits(&mut self.uninits);
xform_move_outs(&mut self.move_outs);
}
fn summary(&self) -> String {
@ -1461,6 +1511,17 @@ impl<'b, 'gcx, 'tcx> InProgress<'b, 'gcx, 'tcx> {
&self.uninits.base_results.operator().move_data().move_paths[mpi_uninit];
s.push_str(&format!("{}", move_path));
});
s.push_str("] ");
s.push_str("move_out: [");
let mut saw_one = false;
self.move_outs.each_state_bit(|mpi_move_out| {
if saw_one { s.push_str(", "); };
saw_one = true;
let move_out =
&self.move_outs.base_results.operator().move_data().moves[mpi_move_out];
s.push_str(&format!("{:?}", move_out));
});
s.push_str("]");
return s;

175
src/librustc_mir/dataflow/impls/mod.rs
View File

@ -14,13 +14,16 @@
use rustc::ty::TyCtxt;
use rustc::mir::{self, Mir, Location};
use rustc_data_structures::bitslice::BitSlice; // adds set_bit/get_bit to &[usize] bitvector rep.
use rustc_data_structures::bitslice::{BitwiseOperator};
use rustc_data_structures::indexed_set::{IdxSet};
use rustc_data_structures::indexed_vec::Idx;
use super::MoveDataParamEnv;
use util::elaborate_drops::DropFlagState;
use super::move_paths::{HasMoveData, MoveData, MovePathIndex};
use super::move_paths::{HasMoveData, MoveData, MoveOutIndex, MovePathIndex};
use super::move_paths::LookupResult;
use super::{BitDenotation, BlockSets, DataflowOperator};
use super::drop_flag_effects_for_function_entry;
@ -205,6 +208,40 @@ impl<'a, 'gcx, 'tcx: 'a> HasMoveData<'tcx> for DefinitelyInitializedLvals<'a, 'g
fn move_data(&self) -> &MoveData<'tcx> { &self.mdpe.move_data }
}
/// `MovingOutStatements` tracks the statements that perform moves out
/// of particular l-values. More precisely, it tracks whether the
/// *effect* of such moves (namely, the uninitialization of the
/// l-value in question) can reach some point in the control-flow of
/// the function, or if that effect is "killed" by some intervening
/// operation reinitializing that l-value.
///
/// The resulting dataflow is a more enriched version of
/// `MaybeUninitializedLvals`. Both structures on their own only tell
/// you if an l-value *might* be uninitialized at a given point in the
/// control flow. But `MovingOutStatements` also includes the added
/// data of *which* particular statement causing the deinitialization
/// that the borrow checker's error message may need to report.
#[allow(dead_code)]
pub struct MovingOutStatements<'a, 'gcx: 'tcx, 'tcx: 'a> {
tcx: TyCtxt<'a, 'gcx, 'tcx>,
mir: &'a Mir<'tcx>,
mdpe: &'a MoveDataParamEnv<'gcx, 'tcx>,
}
impl<'a, 'gcx: 'tcx, 'tcx: 'a> MovingOutStatements<'a, 'gcx, 'tcx> {
pub fn new(tcx: TyCtxt<'a, 'gcx, 'tcx>,
mir: &'a Mir<'tcx>,
mdpe: &'a MoveDataParamEnv<'gcx, 'tcx>)
-> Self
{
MovingOutStatements { tcx: tcx, mir: mir, mdpe: mdpe }
}
}
impl<'a, 'gcx, 'tcx> HasMoveData<'tcx> for MovingOutStatements<'a, 'gcx, 'tcx> {
fn move_data(&self) -> &MoveData<'tcx> { &self.mdpe.move_data }
}
impl<'a, 'gcx, 'tcx> MaybeInitializedLvals<'a, 'gcx, 'tcx> {
fn update_bits(sets: &mut BlockSets<MovePathIndex>, path: MovePathIndex,
state: DropFlagState)
@ -399,6 +436,128 @@ impl<'a, 'gcx, 'tcx> BitDenotation for DefinitelyInitializedLvals<'a, 'gcx, 'tcx
}
}
impl<'a, 'gcx, 'tcx> BitDenotation for MovingOutStatements<'a, 'gcx, 'tcx> {
type Idx = MoveOutIndex;
fn name() -> &'static str { "moving_out" }
fn bits_per_block(&self) -> usize {
self.move_data().moves.len()
}
fn start_block_effect(&self, _sets: &mut BlockSets<MoveOutIndex>) {
// no move-statements have been executed prior to function
// execution, so this method has no effect on `_sets`.
}
fn statement_effect(&self,
sets: &mut BlockSets<MoveOutIndex>,
location: Location) {
let (tcx, mir, move_data) = (self.tcx, self.mir, self.move_data());
let stmt = &mir[location.block].statements[location.statement_index];
let loc_map = &move_data.loc_map;
let path_map = &move_data.path_map;
let rev_lookup = &move_data.rev_lookup;
debug!("stmt {:?} at loc {:?} moves out of move_indexes {:?}",
stmt, location, &loc_map[location]);
for move_index in &loc_map[location] {
// Every path deinitialized by a *particular move*
// has corresponding bit, "gen'ed" (i.e. set)
// here, in dataflow vector
zero_to_one(sets.gen_set.words_mut(), *move_index);
}
let bits_per_block = self.bits_per_block();
match stmt.kind {
mir::StatementKind::SetDiscriminant { .. } => {
span_bug!(stmt.source_info.span, "SetDiscriminant should not exist in borrowck");
}
mir::StatementKind::Assign(ref lvalue, ref rvalue) => {
// assigning into this `lvalue` kills all
// MoveOuts from it, and *also* all MoveOuts
// for children and associated fragment sets.
match rvalue.initialization_state() {
mir::tcx::RvalueInitializationState::Shallow => {
if let LookupResult::Exact(mpi) = rev_lookup.find(lvalue) {
for moi in &path_map[mpi] {
assert!(moi.index() < bits_per_block);
sets.kill_set.add(&moi);
}
}
}
mir::tcx::RvalueInitializationState::Deep => {
on_lookup_result_bits(tcx,
mir,
move_data,
rev_lookup.find(lvalue),
|mpi| for moi in &path_map[mpi] {
assert!(moi.index() < bits_per_block);
sets.kill_set.add(&moi);
});
}
}
}
mir::StatementKind::StorageLive(_) |
mir::StatementKind::StorageDead(_) |
mir::StatementKind::InlineAsm { .. } |
mir::StatementKind::EndRegion(_) |
mir::StatementKind::Validate(..) |
mir::StatementKind::Nop => {}
}
}
fn terminator_effect(&self,
sets: &mut BlockSets<MoveOutIndex>,
location: Location)
{
let (mir, move_data) = (self.mir, self.move_data());
let term = mir[location.block].terminator();
let loc_map = &move_data.loc_map;
debug!("terminator {:?} at loc {:?} moves out of move_indexes {:?}",
term, location, &loc_map[location]);
let bits_per_block = self.bits_per_block();
for move_index in &loc_map[location] {
assert!(move_index.index() < bits_per_block);
zero_to_one(sets.gen_set.words_mut(), *move_index);
}
match term.kind {
mir::TerminatorKind::DropAndReplace { ref location, .. } => {
on_lookup_result_bits(self.tcx,
mir,
move_data,
move_data.rev_lookup.find(location),
|mpi| for moi in &move_data.path_map[mpi] {
assert!(moi.index() < bits_per_block);
sets.kill_set.add(&moi);
});
}
_ => {}
}
}
fn propagate_call_return(&self,
in_out: &mut IdxSet<MoveOutIndex>,
_call_bb: mir::BasicBlock,
_dest_bb: mir::BasicBlock,
dest_lval: &mir::Lvalue) {
let move_data = self.move_data();
let bits_per_block = self.bits_per_block();
let path_map = &move_data.path_map;
on_lookup_result_bits(self.tcx,
self.mir,
move_data,
move_data.rev_lookup.find(dest_lval),
|mpi| for moi in &path_map[mpi] {
assert!(moi.index() < bits_per_block);
in_out.remove(&moi);
});
}
}
fn zero_to_one(bitvec: &mut [usize], move_index: MoveOutIndex) {
let retval = bitvec.set_bit(move_index.index());
assert!(retval);
}
impl<'a, 'gcx, 'tcx> BitwiseOperator for MaybeInitializedLvals<'a, 'gcx, 'tcx> {
#[inline]
fn join(&self, pred1: usize, pred2: usize) -> usize {
@ -420,6 +579,13 @@ impl<'a, 'gcx, 'tcx> BitwiseOperator for DefinitelyInitializedLvals<'a, 'gcx, 't
}
}
impl<'a, 'gcx, 'tcx> BitwiseOperator for MovingOutStatements<'a, 'gcx, 'tcx> {
#[inline]
fn join(&self, pred1: usize, pred2: usize) -> usize {
pred1 | pred2 // moves from both preds are in scope
}
}
// The way that dataflow fixed point iteration works, you want to
// start at bottom and work your way to a fixed point. Control-flow
// merges will apply the `join` operator to each block entry's current
@ -450,3 +616,10 @@ impl<'a, 'gcx, 'tcx> DataflowOperator for DefinitelyInitializedLvals<'a, 'gcx, '
true // bottom = initialized (start_block_effect counters this at outset)
}
}
impl<'a, 'gcx, 'tcx> DataflowOperator for MovingOutStatements<'a, 'gcx, 'tcx> {
#[inline]
fn bottom_value() -> bool {
false // bottom = no loans in scope by default
}
}

2
src/librustc_mir/dataflow/mod.rs
View File

@ -26,7 +26,7 @@ use std::usize;
pub use self::impls::{MaybeStorageLive};
pub use self::impls::{MaybeInitializedLvals, MaybeUninitializedLvals};
pub use self::impls::{DefinitelyInitializedLvals};
pub use self::impls::{DefinitelyInitializedLvals, MovingOutStatements};
pub use self::impls::borrows::{Borrows, BorrowData, BorrowIndex};
pub(crate) use self::drop_flag_effects::*;

2
src/test/compile-fail/borrowck/borrowck-drop-from-guard.rs
View File

@ -19,6 +19,6 @@ fn main()
match Some(42) {
Some(_) if { drop(my_str); false } => {}
Some(_) => {}
None => { foo(my_str); } //~ ERROR (Mir) [E0381]
None => { foo(my_str); } //~ ERROR (Mir) [E0382]
}
}

4
src/test/compile-fail/borrowck/borrowck-uninit-field-access.rs
View File

@ -39,11 +39,11 @@ fn main() {
let _moved = line1.origin;
let _ = line1.origin.x + 1; //[ast]~ ERROR use of collaterally moved value: `line1.origin.x`
//[mir]~^ [E0382]
//[mir]~| (Mir) [E0381]
//[mir]~| (Mir) [E0382]
let mut line2 = Line::default();
let _moved = (line2.origin, line2.middle);
line2.consume(); //[ast]~ ERROR use of partially moved value: `line2` [E0382]
//[mir]~^ [E0382]
//[mir]~| (Mir) [E0381]
//[mir]~| (Mir) [E0382]
}

19
src/test/ui/borrowck/borrowck-reinit.rs Normal file
View File

@ -0,0 +1,19 @@
// 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.
// compile-flags: -Z borrowck-mir -Z emit-end-regions
fn main() {
let mut x = Box::new(0);
let _u = x; // error shouldn't note this move
x = Box::new(1);
drop(x);
let _ = (1,x);
}

20
src/test/ui/borrowck/borrowck-reinit.stderr Normal file
View File

@ -0,0 +1,20 @@
error[E0382]: use of moved value: `x` (Ast)
--> $DIR/borrowck-reinit.rs:18:16
|
17 | drop(x);
| - value moved here
18 | let _ = (1,x);
| ^ value used here after move
|
= note: move occurs because `x` has type `std::boxed::Box<i32>`, which does not implement the `Copy` trait
error[E0382]: use of moved value: `x` (Mir)
--> $DIR/borrowck-reinit.rs:18:16
|
17 | drop(x);
| - value moved here
18 | let _ = (1,x);
| ^ value use here after move
error: aborting due to 2 previous errors