From 43fb82d2fa5152d7b1a97bbe42d26221ff068661 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 4 Oct 2017 11:02:26 +0200 Subject: [PATCH] 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. --- src/librustc_mir/borrow_check.rs | 29 +++++++- .../dataflow/move_paths/builder.rs | 70 +++++++++++-------- src/librustc_mir/dataflow/move_paths/mod.rs | 31 +++++++- src/librustc_mir/transform/elaborate_drops.rs | 2 +- src/librustc_mir/transform/rustc_peek.rs | 2 +- 5 files changed, 99 insertions(+), 35 deletions(-) diff --git a/src/librustc_mir/borrow_check.rs b/src/librustc_mir/borrow_check.rs index 0ad22f91855..5c45f925dd7 100644 --- a/src/librustc_mir/borrow_check.rs +++ b/src/librustc_mir/borrow_check.rs @@ -30,6 +30,7 @@ use dataflow::{MoveDataParamEnv}; use dataflow::{BitDenotation, BlockSets, DataflowResults, DataflowResultsConsumer}; use dataflow::{MaybeInitializedLvals, MaybeUninitializedLvals}; use dataflow::{Borrows, BorrowData, BorrowIndex}; +use dataflow::move_paths::{MoveError, IllegalMoveOriginKind}; use dataflow::move_paths::{HasMoveData, MoveData, MovePathIndex, LookupResult}; 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); 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 dead_unwinds = IdxSetBuf::new_empty(mir.basic_blocks().len()); let flow_borrows = do_dataflow(tcx, mir, id, &attributes, &dead_unwinds, diff --git a/src/librustc_mir/dataflow/move_paths/builder.rs b/src/librustc_mir/dataflow/move_paths/builder.rs index 86298c3b83e..572fffdf4c0 100644 --- a/src/librustc_mir/dataflow/move_paths/builder.rs +++ b/src/librustc_mir/dataflow/move_paths/builder.rs @@ -22,17 +22,15 @@ use std::mem; use super::abs_domain::Lift; 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>, tcx: TyCtxt<'a, 'tcx, 'tcx>, param_env: ty::ParamEnv<'tcx>, data: MoveData<'tcx>, -} - -pub enum MovePathError { - IllegalMove, - UnionMove { path: MovePathIndex }, + errors: Vec>, } impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> { @@ -47,6 +45,7 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> { mir, tcx, param_env, + errors: Vec::new(), data: MoveData { moves: IndexVec::new(), loc_map: LocationMap::new(mir), @@ -94,13 +93,12 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> { /// /// Maybe we should have separate "borrowck" and "moveck" modes. fn move_path_for(&mut self, lval: &Lvalue<'tcx>) - -> Result + -> Result> { debug!("lookup({:?})", lval); match *lval { Lvalue::Local(local) => Ok(self.data.rev_lookup.locals[local]), - // error: can't move out of a static - Lvalue::Static(..) => Err(MovePathError::IllegalMove), + Lvalue::Static(..) => Err(MoveError::cannot_move_out_of(Static)), Lvalue::Projection(ref 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, lval: &Lvalue<'tcx>, proj: &LvalueProjection<'tcx>) - -> Result + -> Result> { let base = try!(self.move_path_for(&proj.base)); let lv_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx); match lv_ty.sty { - // error: can't move out of borrowed content - ty::TyRef(..) | ty::TyRawPtr(..) => return Err(MovePathError::IllegalMove), - // error: can't move out of struct with destructor + ty::TyRef(..) | ty::TyRawPtr(..) => + return Err(MoveError::cannot_move_out_of(BorrowedContent)), 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 ty::TyAdt(adt, _) if adt.is_union() => - return Err(MovePathError::UnionMove { path: base }), - // error: can't move out of a slice - ty::TySlice(..) => - return Err(MovePathError::IllegalMove), - ty::TyArray(..) => match proj.elem { - // error: can't move out of an array - ProjectionElem::Index(..) => return Err(MovePathError::IllegalMove), + return Err(MoveError::UnionMove { path: base }), + ty::TySlice(elem_ty) => + return Err(MoveError::cannot_move_out_of(InteriorOfSlice { + elem_ty, is_index: match proj.elem { + ProjectionElem::Index(..) => true, + _ => false + }, + })), + 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 } @@ -156,7 +161,7 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> { } } - fn finalize(self) -> MoveData<'tcx> { + fn finalize(self) -> Result, (MoveData<'tcx>, Vec>)> { debug!("{}", { debug!("moves for {:?}:", self.mir.span); for (j, mo) in self.data.moves.iter_enumerated() { @@ -168,14 +173,20 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> { } "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>, tcx: TyCtxt<'a, 'tcx, 'tcx>, param_env: ty::ParamEnv<'tcx>) - -> MoveData<'tcx> { + -> Result, + (MoveData<'tcx>, Vec>)> { let mut builder = MoveDataBuilder::new(mir, tcx, param_env); 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) { - Ok(path) | Err(MovePathError::UnionMove { path }) => path, - Err(MovePathError::IllegalMove) => { - // Moving out of a bad path. Eventually, this should be a MIR - // borrowck error instead of a bug. - span_bug!(self.mir.span, - "Broken MIR: moving out of lvalue {:?}: {:?} at {:?}", - lval, lv_ty, loc); + Ok(path) | Err(MoveError::UnionMove { path }) => path, + Err(error @ MoveError::IllegalMove { .. }) => { + self.errors.push(error); + return; } }; let move_out = self.data.moves.push(MoveOut { path: path, source: loc }); diff --git a/src/librustc_mir/dataflow/move_paths/mod.rs b/src/librustc_mir/dataflow/move_paths/mod.rs index d2d80649846..61f64e7373d 100644 --- a/src/librustc_mir/dataflow/move_paths/mod.rs +++ b/src/librustc_mir/dataflow/move_paths/mod.rs @@ -13,6 +13,7 @@ use rustc::ty::{self, TyCtxt}; use rustc::mir::*; use rustc::util::nodemap::FxHashMap; use rustc_data_structures::indexed_vec::{IndexVec}; +use syntax_pos::{DUMMY_SP, Span}; use std::fmt; 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> { pub fn gather_moves(mir: &Mir<'tcx>, tcx: TyCtxt<'a, 'tcx, 'tcx>, param_env: ty::ParamEnv<'tcx>) - -> Self { + -> Result>)> { builder::gather_moves(mir, tcx, param_env) } } diff --git a/src/librustc_mir/transform/elaborate_drops.rs b/src/librustc_mir/transform/elaborate_drops.rs index c833904adba..be1b794ecdf 100644 --- a/src/librustc_mir/transform/elaborate_drops.rs +++ b/src/librustc_mir/transform/elaborate_drops.rs @@ -45,7 +45,7 @@ impl MirPass for ElaborateDrops { } let id = src.item_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 mir = &*mir; let env = MoveDataParamEnv { diff --git a/src/librustc_mir/transform/rustc_peek.rs b/src/librustc_mir/transform/rustc_peek.rs index ceff52409b2..8d6458d7934 100644 --- a/src/librustc_mir/transform/rustc_peek.rs +++ b/src/librustc_mir/transform/rustc_peek.rs @@ -45,7 +45,7 @@ impl MirPass for SanityCheck { let attributes = tcx.get_attrs(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 dead_unwinds = IdxSetBuf::new_empty(mir.basic_blocks().len()); let flow_inits =