mir-borrowck: Gather move errors during MoveData construction and report them.

Currently is using DUMMY_SP as the associated span; a follow-up commit
will pass in appropriate spans when constructing the errors.
This commit is contained in:
Felix S. Klock II 2017-10-04 11:02:26 +02:00
parent fdd7d13c24
commit 43fb82d2fa
5 changed files with 99 additions and 35 deletions

View File

@ -30,6 +30,7 @@ use dataflow::{MoveDataParamEnv};
use dataflow::{BitDenotation, BlockSets, DataflowResults, DataflowResultsConsumer}; use dataflow::{BitDenotation, BlockSets, DataflowResults, DataflowResultsConsumer};
use dataflow::{MaybeInitializedLvals, MaybeUninitializedLvals}; use dataflow::{MaybeInitializedLvals, MaybeUninitializedLvals};
use dataflow::{Borrows, BorrowData, BorrowIndex}; 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};
use util::borrowck_errors::{BorrowckErrors, Origin}; use util::borrowck_errors::{BorrowckErrors, Origin};
@ -59,7 +60,33 @@ fn mir_borrowck<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) {
let param_env = tcx.param_env(def_id); let param_env = tcx.param_env(def_id);
tcx.infer_ctxt().enter(|_infcx| { tcx.infer_ctxt().enter(|_infcx| {
let move_data = MoveData::gather_moves(mir, tcx, param_env); let move_data = match MoveData::gather_moves(mir, tcx, param_env) {
Ok(move_data) => move_data,
Err((move_data, move_errors)) => {
for move_error in move_errors {
let (span, kind): (Span, IllegalMoveOriginKind) = match move_error {
MoveError::UnionMove { .. } =>
unimplemented!("dont know how to report union move errors yet."),
MoveError::IllegalMove { cannot_move_out_of: o } => (o.span, o.kind),
};
let origin = Origin::Mir;
let mut err = match kind {
IllegalMoveOriginKind::Static =>
tcx.cannot_move_out_of(span, "static item", origin),
IllegalMoveOriginKind::BorrowedContent =>
tcx.cannot_move_out_of(span, "borrowed_content", origin),
IllegalMoveOriginKind::InteriorOfTypeWithDestructor { container_ty: ty } =>
tcx.cannot_move_out_of_interior_of_drop(span, ty, origin),
IllegalMoveOriginKind::InteriorOfSlice { elem_ty: ty, is_index } =>
tcx.cannot_move_out_of_interior_noncopy(span, ty, is_index, origin),
IllegalMoveOriginKind::InteriorOfArray { elem_ty: ty, is_index } =>
tcx.cannot_move_out_of_interior_noncopy(span, ty, is_index, origin),
};
err.emit();
}
move_data
}
};
let mdpe = MoveDataParamEnv { move_data: move_data, param_env: param_env }; let mdpe = MoveDataParamEnv { move_data: move_data, param_env: param_env };
let dead_unwinds = IdxSetBuf::new_empty(mir.basic_blocks().len()); let dead_unwinds = IdxSetBuf::new_empty(mir.basic_blocks().len());
let flow_borrows = do_dataflow(tcx, mir, id, &attributes, &dead_unwinds, let flow_borrows = do_dataflow(tcx, mir, id, &attributes, &dead_unwinds,

View File

@ -22,17 +22,15 @@ use std::mem;
use super::abs_domain::Lift; use super::abs_domain::Lift;
use super::{LocationMap, MoveData, MovePath, MovePathLookup, MovePathIndex, MoveOut, MoveOutIndex}; use super::{LocationMap, MoveData, MovePath, MovePathLookup, MovePathIndex, MoveOut, MoveOutIndex};
use super::{MoveError};
use super::IllegalMoveOriginKind::*;
pub(super) struct MoveDataBuilder<'a, 'tcx: 'a> { struct MoveDataBuilder<'a, 'tcx: 'a> {
mir: &'a Mir<'tcx>, mir: &'a Mir<'tcx>,
tcx: TyCtxt<'a, 'tcx, 'tcx>, tcx: TyCtxt<'a, 'tcx, 'tcx>,
param_env: ty::ParamEnv<'tcx>, param_env: ty::ParamEnv<'tcx>,
data: MoveData<'tcx>, data: MoveData<'tcx>,
} errors: Vec<MoveError<'tcx>>,
pub enum MovePathError {
IllegalMove,
UnionMove { path: MovePathIndex },
} }
impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> { impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
@ -47,6 +45,7 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
mir, mir,
tcx, tcx,
param_env, param_env,
errors: Vec::new(),
data: MoveData { data: MoveData {
moves: IndexVec::new(), moves: IndexVec::new(),
loc_map: LocationMap::new(mir), loc_map: LocationMap::new(mir),
@ -94,13 +93,12 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
/// ///
/// Maybe we should have separate "borrowck" and "moveck" modes. /// Maybe we should have separate "borrowck" and "moveck" modes.
fn move_path_for(&mut self, lval: &Lvalue<'tcx>) fn move_path_for(&mut self, lval: &Lvalue<'tcx>)
-> Result<MovePathIndex, MovePathError> -> Result<MovePathIndex, MoveError<'tcx>>
{ {
debug!("lookup({:?})", lval); debug!("lookup({:?})", lval);
match *lval { match *lval {
Lvalue::Local(local) => Ok(self.data.rev_lookup.locals[local]), Lvalue::Local(local) => Ok(self.data.rev_lookup.locals[local]),
// error: can't move out of a static Lvalue::Static(..) => Err(MoveError::cannot_move_out_of(Static)),
Lvalue::Static(..) => Err(MovePathError::IllegalMove),
Lvalue::Projection(ref proj) => { Lvalue::Projection(ref proj) => {
self.move_path_for_projection(lval, proj) self.move_path_for_projection(lval, proj)
} }
@ -116,25 +114,32 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
fn move_path_for_projection(&mut self, fn move_path_for_projection(&mut self,
lval: &Lvalue<'tcx>, lval: &Lvalue<'tcx>,
proj: &LvalueProjection<'tcx>) proj: &LvalueProjection<'tcx>)
-> Result<MovePathIndex, MovePathError> -> Result<MovePathIndex, MoveError<'tcx>>
{ {
let base = try!(self.move_path_for(&proj.base)); let base = try!(self.move_path_for(&proj.base));
let lv_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx); let lv_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx);
match lv_ty.sty { match lv_ty.sty {
// error: can't move out of borrowed content ty::TyRef(..) | ty::TyRawPtr(..) =>
ty::TyRef(..) | ty::TyRawPtr(..) => return Err(MovePathError::IllegalMove), return Err(MoveError::cannot_move_out_of(BorrowedContent)),
// error: can't move out of struct with destructor
ty::TyAdt(adt, _) if adt.has_dtor(self.tcx) && !adt.is_box() => ty::TyAdt(adt, _) if adt.has_dtor(self.tcx) && !adt.is_box() =>
return Err(MovePathError::IllegalMove), return Err(MoveError::cannot_move_out_of(InteriorOfTypeWithDestructor {
container_ty: lv_ty
})),
// move out of union - always move the entire union // move out of union - always move the entire union
ty::TyAdt(adt, _) if adt.is_union() => ty::TyAdt(adt, _) if adt.is_union() =>
return Err(MovePathError::UnionMove { path: base }), return Err(MoveError::UnionMove { path: base }),
// error: can't move out of a slice ty::TySlice(elem_ty) =>
ty::TySlice(..) => return Err(MoveError::cannot_move_out_of(InteriorOfSlice {
return Err(MovePathError::IllegalMove), elem_ty, is_index: match proj.elem {
ty::TyArray(..) => match proj.elem { ProjectionElem::Index(..) => true,
// error: can't move out of an array _ => false
ProjectionElem::Index(..) => return Err(MovePathError::IllegalMove), },
})),
ty::TyArray(elem_ty, _num_elems) => match proj.elem {
ProjectionElem::Index(..) =>
return Err(MoveError::cannot_move_out_of(InteriorOfArray {
elem_ty, is_index: true
})),
_ => { _ => {
// FIXME: still badly broken // FIXME: still badly broken
} }
@ -156,7 +161,7 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
} }
} }
fn finalize(self) -> MoveData<'tcx> { fn finalize(self) -> Result<MoveData<'tcx>, (MoveData<'tcx>, Vec<MoveError<'tcx>>)> {
debug!("{}", { debug!("{}", {
debug!("moves for {:?}:", self.mir.span); debug!("moves for {:?}:", self.mir.span);
for (j, mo) in self.data.moves.iter_enumerated() { for (j, mo) in self.data.moves.iter_enumerated() {
@ -168,14 +173,20 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
} }
"done dumping moves" "done dumping moves"
}); });
self.data
if self.errors.len() > 0 {
Err((self.data, self.errors))
} else {
Ok(self.data)
}
} }
} }
pub(super) fn gather_moves<'a, 'tcx>(mir: &Mir<'tcx>, pub(super) fn gather_moves<'a, 'tcx>(mir: &Mir<'tcx>,
tcx: TyCtxt<'a, 'tcx, 'tcx>, tcx: TyCtxt<'a, 'tcx, 'tcx>,
param_env: ty::ParamEnv<'tcx>) param_env: ty::ParamEnv<'tcx>)
-> MoveData<'tcx> { -> Result<MoveData<'tcx>,
(MoveData<'tcx>, Vec<MoveError<'tcx>>)> {
let mut builder = MoveDataBuilder::new(mir, tcx, param_env); let mut builder = MoveDataBuilder::new(mir, tcx, param_env);
for (bb, block) in mir.basic_blocks().iter_enumerated() { for (bb, block) in mir.basic_blocks().iter_enumerated() {
@ -317,13 +328,10 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
} }
let path = match self.move_path_for(lval) { let path = match self.move_path_for(lval) {
Ok(path) | Err(MovePathError::UnionMove { path }) => path, Ok(path) | Err(MoveError::UnionMove { path }) => path,
Err(MovePathError::IllegalMove) => { Err(error @ MoveError::IllegalMove { .. }) => {
// Moving out of a bad path. Eventually, this should be a MIR self.errors.push(error);
// borrowck error instead of a bug. return;
span_bug!(self.mir.span,
"Broken MIR: moving out of lvalue {:?}: {:?} at {:?}",
lval, lv_ty, loc);
} }
}; };
let move_out = self.data.moves.push(MoveOut { path: path, source: loc }); let move_out = self.data.moves.push(MoveOut { path: path, source: loc });

View File

@ -13,6 +13,7 @@ use rustc::ty::{self, TyCtxt};
use rustc::mir::*; use rustc::mir::*;
use rustc::util::nodemap::FxHashMap; use rustc::util::nodemap::FxHashMap;
use rustc_data_structures::indexed_vec::{IndexVec}; use rustc_data_structures::indexed_vec::{IndexVec};
use syntax_pos::{DUMMY_SP, Span};
use std::fmt; use std::fmt;
use std::ops::{Index, IndexMut}; use std::ops::{Index, IndexMut};
@ -227,11 +228,39 @@ impl<'tcx> MovePathLookup<'tcx> {
} }
} }
#[derive(Debug)]
pub struct IllegalMoveOrigin<'tcx> {
pub(crate) span: Span,
pub(crate) kind: IllegalMoveOriginKind<'tcx>,
}
#[derive(Debug)]
pub(crate) enum IllegalMoveOriginKind<'tcx> {
Static,
BorrowedContent,
InteriorOfTypeWithDestructor { container_ty: ty::Ty<'tcx> },
InteriorOfSlice { elem_ty: ty::Ty<'tcx>, is_index: bool, },
InteriorOfArray { elem_ty: ty::Ty<'tcx>, is_index: bool, },
}
#[derive(Debug)]
pub enum MoveError<'tcx> {
IllegalMove { cannot_move_out_of: IllegalMoveOrigin<'tcx> },
UnionMove { path: MovePathIndex },
}
impl<'tcx> MoveError<'tcx> {
fn cannot_move_out_of(kind: IllegalMoveOriginKind<'tcx>) -> Self {
let origin = IllegalMoveOrigin { span: DUMMY_SP, kind: kind, };
MoveError::IllegalMove { cannot_move_out_of: origin }
}
}
impl<'a, 'tcx> MoveData<'tcx> { impl<'a, 'tcx> MoveData<'tcx> {
pub fn gather_moves(mir: &Mir<'tcx>, pub fn gather_moves(mir: &Mir<'tcx>,
tcx: TyCtxt<'a, 'tcx, 'tcx>, tcx: TyCtxt<'a, 'tcx, 'tcx>,
param_env: ty::ParamEnv<'tcx>) param_env: ty::ParamEnv<'tcx>)
-> Self { -> Result<Self, (Self, Vec<MoveError<'tcx>>)> {
builder::gather_moves(mir, tcx, param_env) builder::gather_moves(mir, tcx, param_env)
} }
} }

View File

@ -45,7 +45,7 @@ impl MirPass for ElaborateDrops {
} }
let id = src.item_id(); let id = src.item_id();
let param_env = tcx.param_env(tcx.hir.local_def_id(id)); let param_env = tcx.param_env(tcx.hir.local_def_id(id));
let move_data = MoveData::gather_moves(mir, tcx, param_env); let move_data = MoveData::gather_moves(mir, tcx, param_env).unwrap();
let elaborate_patch = { let elaborate_patch = {
let mir = &*mir; let mir = &*mir;
let env = MoveDataParamEnv { let env = MoveDataParamEnv {

View File

@ -45,7 +45,7 @@ impl MirPass for SanityCheck {
let attributes = tcx.get_attrs(def_id); let attributes = tcx.get_attrs(def_id);
let param_env = tcx.param_env(def_id); let param_env = tcx.param_env(def_id);
let move_data = MoveData::gather_moves(mir, tcx, param_env); let move_data = MoveData::gather_moves(mir, tcx, param_env).unwrap();
let mdpe = MoveDataParamEnv { move_data: move_data, param_env: param_env }; let mdpe = MoveDataParamEnv { move_data: move_data, param_env: param_env };
let dead_unwinds = IdxSetBuf::new_empty(mir.basic_blocks().len()); let dead_unwinds = IdxSetBuf::new_empty(mir.basic_blocks().len());
let flow_inits = let flow_inits =