diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index b909269e1538e..075a629de04da 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -260,6 +260,19 @@ impl<'tcx> Mir<'tcx> { debug_assert!(location.statement_index < block.statements.len()); block.statements[location.statement_index].make_nop() } + + /// Returns the source info associated with `location`. + pub fn source_info(&self, location: Location) -> &SourceInfo { + let block = &self[location.block]; + let stmts = &block.statements; + let idx = location.statement_index; + if location.statement_index < stmts.len() { + &stmts[idx].source_info + } else { + assert!(location.statement_index == stmts.len()); + &block.terminator().source_info + } + } } #[derive(Clone, Debug)] diff --git a/src/librustc_borrowck/borrowck/gather_loans/move_error.rs b/src/librustc_borrowck/borrowck/gather_loans/move_error.rs index de3f6f083256d..1f2b917bdb994 100644 --- a/src/librustc_borrowck/borrowck/gather_loans/move_error.rs +++ b/src/librustc_borrowck/borrowck/gather_loans/move_error.rs @@ -14,6 +14,7 @@ use rustc::middle::mem_categorization::Categorization; use rustc::middle::mem_categorization::NoteClosureEnv; use rustc::middle::mem_categorization::InteriorOffsetKind as Kind; use rustc::ty; +use rustc_mir::util::borrowck_errors::{BorrowckErrors, Origin}; use syntax::ast; use syntax_pos; use errors::DiagnosticBuilder; @@ -134,7 +135,7 @@ fn group_errors_with_same_origin<'tcx>(errors: &Vec>) } // (keep in sync with gather_moves::check_and_get_illegal_move_origin ) -fn report_cannot_move_out_of<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, +fn report_cannot_move_out_of<'a, 'tcx>(bccx: &'a BorrowckCtxt<'a, 'tcx>, move_from: mc::cmt<'tcx>) -> DiagnosticBuilder<'a> { match move_from.cat { @@ -142,43 +143,21 @@ fn report_cannot_move_out_of<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>, Categorization::Deref(_, mc::Implicit(..)) | Categorization::Deref(_, mc::UnsafePtr(..)) | Categorization::StaticItem => { - let mut err = struct_span_err!(bccx, move_from.span, E0507, - "cannot move out of {}", - move_from.descriptive_string(bccx.tcx)); - err.span_label( - move_from.span, - format!("cannot move out of {}", move_from.descriptive_string(bccx.tcx)) - ); - err + bccx.cannot_move_out_of( + move_from.span, &move_from.descriptive_string(bccx.tcx), Origin::Ast) } - Categorization::Interior(ref b, mc::InteriorElement(ik)) => { - let type_name = match (&b.ty.sty, ik) { - (&ty::TyArray(_, _), Kind::Index) => "array", - (&ty::TySlice(_), _) => "slice", - _ => { - span_bug!(move_from.span, "this path should not cause illegal move"); - }, - }; - let mut err = struct_span_err!(bccx, move_from.span, E0508, - "cannot move out of type `{}`, \ - a non-copy {}", - b.ty, type_name); - err.span_label(move_from.span, "cannot move out of here"); - err + bccx.cannot_move_out_of_interior_noncopy( + move_from.span, b.ty, ik == Kind::Index, Origin::Ast) } Categorization::Downcast(ref b, _) | Categorization::Interior(ref b, mc::InteriorField(_)) => { match b.ty.sty { ty::TyAdt(def, _) if def.has_dtor(bccx.tcx) => { - let mut err = struct_span_err!(bccx, move_from.span, E0509, - "cannot move out of type `{}`, \ - which implements the `Drop` trait", - b.ty); - err.span_label(move_from.span, "cannot move out of here"); - err - }, + bccx.cannot_move_out_of_interior_of_drop( + move_from.span, b.ty, Origin::Ast) + } _ => { span_bug!(move_from.span, "this path should not cause illegal move"); } diff --git a/src/librustc_borrowck/diagnostics.rs b/src/librustc_borrowck/diagnostics.rs index 29c35e23d4ee3..031dbcb1ebb91 100644 --- a/src/librustc_borrowck/diagnostics.rs +++ b/src/librustc_borrowck/diagnostics.rs @@ -317,272 +317,6 @@ fn main() { ``` "##, -E0507: r##" -You tried to move out of a value which was borrowed. Erroneous code example: - -```compile_fail,E0507 -use std::cell::RefCell; - -struct TheDarkKnight; - -impl TheDarkKnight { - fn nothing_is_true(self) {} -} - -fn main() { - let x = RefCell::new(TheDarkKnight); - - x.borrow().nothing_is_true(); // error: cannot move out of borrowed content -} -``` - -Here, the `nothing_is_true` method takes the ownership of `self`. However, -`self` cannot be moved because `.borrow()` only provides an `&TheDarkKnight`, -which is a borrow of the content owned by the `RefCell`. To fix this error, -you have three choices: - -* Try to avoid moving the variable. -* Somehow reclaim the ownership. -* Implement the `Copy` trait on the type. - -Examples: - -``` -use std::cell::RefCell; - -struct TheDarkKnight; - -impl TheDarkKnight { - fn nothing_is_true(&self) {} // First case, we don't take ownership -} - -fn main() { - let x = RefCell::new(TheDarkKnight); - - x.borrow().nothing_is_true(); // ok! -} -``` - -Or: - -``` -use std::cell::RefCell; - -struct TheDarkKnight; - -impl TheDarkKnight { - fn nothing_is_true(self) {} -} - -fn main() { - let x = RefCell::new(TheDarkKnight); - let x = x.into_inner(); // we get back ownership - - x.nothing_is_true(); // ok! -} -``` - -Or: - -``` -use std::cell::RefCell; - -#[derive(Clone, Copy)] // we implement the Copy trait -struct TheDarkKnight; - -impl TheDarkKnight { - fn nothing_is_true(self) {} -} - -fn main() { - let x = RefCell::new(TheDarkKnight); - - x.borrow().nothing_is_true(); // ok! -} -``` - -Moving a member out of a mutably borrowed struct will also cause E0507 error: - -```compile_fail,E0507 -struct TheDarkKnight; - -impl TheDarkKnight { - fn nothing_is_true(self) {} -} - -struct Batcave { - knight: TheDarkKnight -} - -fn main() { - let mut cave = Batcave { - knight: TheDarkKnight - }; - let borrowed = &mut cave; - - borrowed.knight.nothing_is_true(); // E0507 -} -``` - -It is fine only if you put something back. `mem::replace` can be used for that: - -``` -# struct TheDarkKnight; -# impl TheDarkKnight { fn nothing_is_true(self) {} } -# struct Batcave { knight: TheDarkKnight } -use std::mem; - -let mut cave = Batcave { - knight: TheDarkKnight -}; -let borrowed = &mut cave; - -mem::replace(&mut borrowed.knight, TheDarkKnight).nothing_is_true(); // ok! -``` - -You can find more information about borrowing in the rust-book: -http://doc.rust-lang.org/book/first-edition/references-and-borrowing.html -"##, - -E0508: r##" -A value was moved out of a non-copy fixed-size array. - -Example of erroneous code: - -```compile_fail,E0508 -struct NonCopy; - -fn main() { - let array = [NonCopy; 1]; - let _value = array[0]; // error: cannot move out of type `[NonCopy; 1]`, - // a non-copy fixed-size array -} -``` - -The first element was moved out of the array, but this is not -possible because `NonCopy` does not implement the `Copy` trait. - -Consider borrowing the element instead of moving it: - -``` -struct NonCopy; - -fn main() { - let array = [NonCopy; 1]; - let _value = &array[0]; // Borrowing is allowed, unlike moving. -} -``` - -Alternatively, if your type implements `Clone` and you need to own the value, -consider borrowing and then cloning: - -``` -#[derive(Clone)] -struct NonCopy; - -fn main() { - let array = [NonCopy; 1]; - // Now you can clone the array element. - let _value = array[0].clone(); -} -``` -"##, - -E0509: r##" -This error occurs when an attempt is made to move out of a value whose type -implements the `Drop` trait. - -Example of erroneous code: - -```compile_fail,E0509 -struct FancyNum { - num: usize -} - -struct DropStruct { - fancy: FancyNum -} - -impl Drop for DropStruct { - fn drop(&mut self) { - // Destruct DropStruct, possibly using FancyNum - } -} - -fn main() { - let drop_struct = DropStruct{fancy: FancyNum{num: 5}}; - let fancy_field = drop_struct.fancy; // Error E0509 - println!("Fancy: {}", fancy_field.num); - // implicit call to `drop_struct.drop()` as drop_struct goes out of scope -} -``` - -Here, we tried to move a field out of a struct of type `DropStruct` which -implements the `Drop` trait. However, a struct cannot be dropped if one or -more of its fields have been moved. - -Structs implementing the `Drop` trait have an implicit destructor that gets -called when they go out of scope. This destructor may use the fields of the -struct, so moving out of the struct could make it impossible to run the -destructor. Therefore, we must think of all values whose type implements the -`Drop` trait as single units whose fields cannot be moved. - -This error can be fixed by creating a reference to the fields of a struct, -enum, or tuple using the `ref` keyword: - -``` -struct FancyNum { - num: usize -} - -struct DropStruct { - fancy: FancyNum -} - -impl Drop for DropStruct { - fn drop(&mut self) { - // Destruct DropStruct, possibly using FancyNum - } -} - -fn main() { - let drop_struct = DropStruct{fancy: FancyNum{num: 5}}; - let ref fancy_field = drop_struct.fancy; // No more errors! - println!("Fancy: {}", fancy_field.num); - // implicit call to `drop_struct.drop()` as drop_struct goes out of scope -} -``` - -Note that this technique can also be used in the arms of a match expression: - -``` -struct FancyNum { - num: usize -} - -enum DropEnum { - Fancy(FancyNum) -} - -impl Drop for DropEnum { - fn drop(&mut self) { - // Destruct DropEnum, possibly using FancyNum - } -} - -fn main() { - // Creates and enum of type `DropEnum`, which implements `Drop` - let drop_enum = DropEnum::Fancy(FancyNum{num: 10}); - match drop_enum { - // Creates a reference to the inside of `DropEnum::Fancy` - DropEnum::Fancy(ref fancy_field) => // No error! - println!("It was fancy-- {}!", fancy_field.num), - } - // implicit call to `drop_enum.drop()` as drop_enum goes out of scope -} -``` -"##, - E0595: r##" Closures cannot mutate immutable captured variables. diff --git a/src/librustc_mir/borrow_check.rs b/src/librustc_mir/borrow_check.rs index 14b6e31913250..db6a0ee4ba5d3 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, @@ -1106,9 +1133,7 @@ impl<'c, 'b, 'a: 'b+'c, 'gcx, 'tcx: 'a> MirBorrowckCtxt<'c, 'b, 'a, 'gcx, 'tcx> // Retrieve span of given borrow from the current MIR representation fn retrieve_borrow_span(&self, borrow: &BorrowData) -> Span { - self.mir.basic_blocks()[borrow.location.block] - .statements[borrow.location.statement_index] - .source_info.span + self.mir.source_info(borrow.location).span } } diff --git a/src/librustc_mir/dataflow/move_paths/builder.rs b/src/librustc_mir/dataflow/move_paths/builder.rs index 86298c3b83e29..0790d937cebf0 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), @@ -85,7 +84,9 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> { assert_eq!(path_map_ent, move_path); move_path } +} +impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { /// This creates a MovePath for a given lvalue, returning an `MovePathError` /// if that lvalue can't be moved from. /// @@ -94,13 +95,15 @@ 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::Local(local) => Ok(self.builder.data.rev_lookup.locals[local]), + Lvalue::Static(..) => { + let span = self.builder.mir.source_info(self.loc).span; + Err(MoveError::cannot_move_out_of(span, Static)) + } Lvalue::Projection(ref proj) => { self.move_path_for_projection(lval, proj) } @@ -116,37 +119,52 @@ 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); + let mir = self.builder.mir; + let tcx = self.builder.tcx; + let lv_ty = proj.base.ty(mir, tcx).to_ty(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::TyAdt(adt, _) if adt.has_dtor(self.tcx) && !adt.is_box() => - return Err(MovePathError::IllegalMove), + ty::TyRef(..) | ty::TyRawPtr(..) => + return Err(MoveError::cannot_move_out_of(mir.source_info(self.loc).span, + BorrowedContent)), + ty::TyAdt(adt, _) if adt.has_dtor(tcx) && !adt.is_box() => + return Err(MoveError::cannot_move_out_of(mir.source_info(self.loc).span, + 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( + mir.source_info(self.loc).span, + 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( + mir.source_info(self.loc).span, + InteriorOfArray { + elem_ty, is_index: true + })), _ => { // FIXME: still badly broken } }, _ => {} }; - match self.data.rev_lookup.projections.entry((base, proj.elem.lift())) { + match self.builder.data.rev_lookup.projections.entry((base, proj.elem.lift())) { Entry::Occupied(ent) => Ok(*ent.get()), Entry::Vacant(ent) => { - let path = Self::new_move_path( - &mut self.data.move_paths, - &mut self.data.path_map, + let path = MoveDataBuilder::new_move_path( + &mut self.builder.data.move_paths, + &mut self.builder.data.path_map, Some(base), lval.clone() ); @@ -155,8 +173,10 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> { } } } +} - fn finalize(self) -> MoveData<'tcx> { +impl<'a, 'tcx> MoveDataBuilder<'a, '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 +188,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() { @@ -197,6 +223,22 @@ pub(super) fn gather_moves<'a, 'tcx>(mir: &Mir<'tcx>, impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> { fn gather_statement(&mut self, loc: Location, stmt: &Statement<'tcx>) { debug!("gather_statement({:?}, {:?})", loc, stmt); + (Gatherer { builder: self, loc }).gather_statement(stmt); + } + + fn gather_terminator(&mut self, loc: Location, term: &Terminator<'tcx>) { + debug!("gather_terminator({:?}, {:?})", loc, term); + (Gatherer { builder: self, loc }).gather_terminator(term); + } +} + +struct Gatherer<'b, 'a: 'b, 'tcx: 'a> { + builder: &'b mut MoveDataBuilder<'a, 'tcx>, + loc: Location, +} + +impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { + fn gather_statement(&mut self, stmt: &Statement<'tcx>) { match stmt.kind { StatementKind::Assign(ref lval, ref rval) => { self.create_move_path(lval); @@ -206,7 +248,7 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> { // the exterior. self.create_move_path(&lval.clone().deref()); } - self.gather_rvalue(loc, rval); + self.gather_rvalue(rval); } StatementKind::StorageLive(_) | StatementKind::StorageDead(_) => {} @@ -221,22 +263,22 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> { } } - fn gather_rvalue(&mut self, loc: Location, rvalue: &Rvalue<'tcx>) { + fn gather_rvalue(&mut self, rvalue: &Rvalue<'tcx>) { match *rvalue { Rvalue::Use(ref operand) | Rvalue::Repeat(ref operand, _) | Rvalue::Cast(_, ref operand, _) | Rvalue::UnaryOp(_, ref operand) => { - self.gather_operand(loc, operand) + self.gather_operand(operand) } Rvalue::BinaryOp(ref _binop, ref lhs, ref rhs) | Rvalue::CheckedBinaryOp(ref _binop, ref lhs, ref rhs) => { - self.gather_operand(loc, lhs); - self.gather_operand(loc, rhs); + self.gather_operand(lhs); + self.gather_operand(rhs); } Rvalue::Aggregate(ref _kind, ref operands) => { for operand in operands { - self.gather_operand(loc, operand); + self.gather_operand(operand); } } Rvalue::Ref(..) | @@ -258,8 +300,7 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> { } } - fn gather_terminator(&mut self, loc: Location, term: &Terminator<'tcx>) { - debug!("gather_terminator({:?}, {:?})", loc, term); + fn gather_terminator(&mut self, term: &Terminator<'tcx>) { match term.kind { TerminatorKind::Goto { target: _ } | TerminatorKind::Resume | @@ -267,7 +308,7 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> { TerminatorKind::Unreachable => { } TerminatorKind::Return => { - self.gather_move(loc, &Lvalue::Local(RETURN_POINTER)); + self.gather_move(&Lvalue::Local(RETURN_POINTER)); } TerminatorKind::Assert { .. } | @@ -276,20 +317,20 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> { } TerminatorKind::Yield { ref value, .. } => { - self.gather_operand(loc, value); + self.gather_operand(value); } TerminatorKind::Drop { ref location, target: _, unwind: _ } => { - self.gather_move(loc, location); + self.gather_move(location); } TerminatorKind::DropAndReplace { ref location, ref value, .. } => { self.create_move_path(location); - self.gather_operand(loc, value); + self.gather_operand(value); } TerminatorKind::Call { ref func, ref args, ref destination, cleanup: _ } => { - self.gather_operand(loc, func); + self.gather_operand(func); for arg in args { - self.gather_operand(loc, arg); + self.gather_operand(arg); } if let Some((ref destination, _bb)) = *destination { self.create_move_path(destination); @@ -298,40 +339,38 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> { } } - fn gather_operand(&mut self, loc: Location, operand: &Operand<'tcx>) { + fn gather_operand(&mut self, operand: &Operand<'tcx>) { match *operand { Operand::Constant(..) => {} // not-a-move Operand::Consume(ref lval) => { // a move - self.gather_move(loc, lval); + self.gather_move(lval); } } } - fn gather_move(&mut self, loc: Location, lval: &Lvalue<'tcx>) { - debug!("gather_move({:?}, {:?})", loc, lval); + fn gather_move(&mut self, lval: &Lvalue<'tcx>) { + debug!("gather_move({:?}, {:?})", self.loc, lval); - let lv_ty = lval.ty(self.mir, self.tcx).to_ty(self.tcx); - if !lv_ty.moves_by_default(self.tcx, self.param_env, DUMMY_SP) { - debug!("gather_move({:?}, {:?}) - {:?} is Copy. skipping", loc, lval, lv_ty); + let tcx = self.builder.tcx; + let lv_ty = lval.ty(self.builder.mir, tcx).to_ty(tcx); + if !lv_ty.moves_by_default(tcx, self.builder.param_env, DUMMY_SP) { + debug!("gather_move({:?}, {:?}) - {:?} is Copy. skipping", self.loc, lval, lv_ty); return } 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.builder.errors.push(error); + return; } }; - let move_out = self.data.moves.push(MoveOut { path: path, source: loc }); + let move_out = self.builder.data.moves.push(MoveOut { path: path, source: self.loc }); debug!("gather_move({:?}, {:?}): adding move {:?} of {:?}", - loc, lval, move_out, path); + self.loc, lval, move_out, path); - self.data.path_map[path].push(move_out); - self.data.loc_map[loc].push(move_out); + self.builder.data.path_map[path].push(move_out); + self.builder.data.loc_map[self.loc].push(move_out); } } diff --git a/src/librustc_mir/dataflow/move_paths/mod.rs b/src/librustc_mir/dataflow/move_paths/mod.rs index d2d8064984682..9369156a223c8 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::{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(span: Span, kind: IllegalMoveOriginKind<'tcx>) -> Self { + let origin = IllegalMoveOrigin { span, 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/diagnostics.rs b/src/librustc_mir/diagnostics.rs index 950bdff1d0f8f..645af0bff64de 100644 --- a/src/librustc_mir/diagnostics.rs +++ b/src/librustc_mir/diagnostics.rs @@ -999,6 +999,272 @@ fn print_fancy_ref(fancy_ref: &FancyNum){ ``` "##, +E0507: r##" +You tried to move out of a value which was borrowed. Erroneous code example: + +```compile_fail,E0507 +use std::cell::RefCell; + +struct TheDarkKnight; + +impl TheDarkKnight { + fn nothing_is_true(self) {} +} + +fn main() { + let x = RefCell::new(TheDarkKnight); + + x.borrow().nothing_is_true(); // error: cannot move out of borrowed content +} +``` + +Here, the `nothing_is_true` method takes the ownership of `self`. However, +`self` cannot be moved because `.borrow()` only provides an `&TheDarkKnight`, +which is a borrow of the content owned by the `RefCell`. To fix this error, +you have three choices: + +* Try to avoid moving the variable. +* Somehow reclaim the ownership. +* Implement the `Copy` trait on the type. + +Examples: + +``` +use std::cell::RefCell; + +struct TheDarkKnight; + +impl TheDarkKnight { + fn nothing_is_true(&self) {} // First case, we don't take ownership +} + +fn main() { + let x = RefCell::new(TheDarkKnight); + + x.borrow().nothing_is_true(); // ok! +} +``` + +Or: + +``` +use std::cell::RefCell; + +struct TheDarkKnight; + +impl TheDarkKnight { + fn nothing_is_true(self) {} +} + +fn main() { + let x = RefCell::new(TheDarkKnight); + let x = x.into_inner(); // we get back ownership + + x.nothing_is_true(); // ok! +} +``` + +Or: + +``` +use std::cell::RefCell; + +#[derive(Clone, Copy)] // we implement the Copy trait +struct TheDarkKnight; + +impl TheDarkKnight { + fn nothing_is_true(self) {} +} + +fn main() { + let x = RefCell::new(TheDarkKnight); + + x.borrow().nothing_is_true(); // ok! +} +``` + +Moving a member out of a mutably borrowed struct will also cause E0507 error: + +```compile_fail,E0507 +struct TheDarkKnight; + +impl TheDarkKnight { + fn nothing_is_true(self) {} +} + +struct Batcave { + knight: TheDarkKnight +} + +fn main() { + let mut cave = Batcave { + knight: TheDarkKnight + }; + let borrowed = &mut cave; + + borrowed.knight.nothing_is_true(); // E0507 +} +``` + +It is fine only if you put something back. `mem::replace` can be used for that: + +``` +# struct TheDarkKnight; +# impl TheDarkKnight { fn nothing_is_true(self) {} } +# struct Batcave { knight: TheDarkKnight } +use std::mem; + +let mut cave = Batcave { + knight: TheDarkKnight +}; +let borrowed = &mut cave; + +mem::replace(&mut borrowed.knight, TheDarkKnight).nothing_is_true(); // ok! +``` + +You can find more information about borrowing in the rust-book: +http://doc.rust-lang.org/book/first-edition/references-and-borrowing.html +"##, + +E0508: r##" +A value was moved out of a non-copy fixed-size array. + +Example of erroneous code: + +```compile_fail,E0508 +struct NonCopy; + +fn main() { + let array = [NonCopy; 1]; + let _value = array[0]; // error: cannot move out of type `[NonCopy; 1]`, + // a non-copy fixed-size array +} +``` + +The first element was moved out of the array, but this is not +possible because `NonCopy` does not implement the `Copy` trait. + +Consider borrowing the element instead of moving it: + +``` +struct NonCopy; + +fn main() { + let array = [NonCopy; 1]; + let _value = &array[0]; // Borrowing is allowed, unlike moving. +} +``` + +Alternatively, if your type implements `Clone` and you need to own the value, +consider borrowing and then cloning: + +``` +#[derive(Clone)] +struct NonCopy; + +fn main() { + let array = [NonCopy; 1]; + // Now you can clone the array element. + let _value = array[0].clone(); +} +``` +"##, + +E0509: r##" +This error occurs when an attempt is made to move out of a value whose type +implements the `Drop` trait. + +Example of erroneous code: + +```compile_fail,E0509 +struct FancyNum { + num: usize +} + +struct DropStruct { + fancy: FancyNum +} + +impl Drop for DropStruct { + fn drop(&mut self) { + // Destruct DropStruct, possibly using FancyNum + } +} + +fn main() { + let drop_struct = DropStruct{fancy: FancyNum{num: 5}}; + let fancy_field = drop_struct.fancy; // Error E0509 + println!("Fancy: {}", fancy_field.num); + // implicit call to `drop_struct.drop()` as drop_struct goes out of scope +} +``` + +Here, we tried to move a field out of a struct of type `DropStruct` which +implements the `Drop` trait. However, a struct cannot be dropped if one or +more of its fields have been moved. + +Structs implementing the `Drop` trait have an implicit destructor that gets +called when they go out of scope. This destructor may use the fields of the +struct, so moving out of the struct could make it impossible to run the +destructor. Therefore, we must think of all values whose type implements the +`Drop` trait as single units whose fields cannot be moved. + +This error can be fixed by creating a reference to the fields of a struct, +enum, or tuple using the `ref` keyword: + +``` +struct FancyNum { + num: usize +} + +struct DropStruct { + fancy: FancyNum +} + +impl Drop for DropStruct { + fn drop(&mut self) { + // Destruct DropStruct, possibly using FancyNum + } +} + +fn main() { + let drop_struct = DropStruct{fancy: FancyNum{num: 5}}; + let ref fancy_field = drop_struct.fancy; // No more errors! + println!("Fancy: {}", fancy_field.num); + // implicit call to `drop_struct.drop()` as drop_struct goes out of scope +} +``` + +Note that this technique can also be used in the arms of a match expression: + +``` +struct FancyNum { + num: usize +} + +enum DropEnum { + Fancy(FancyNum) +} + +impl Drop for DropEnum { + fn drop(&mut self) { + // Destruct DropEnum, possibly using FancyNum + } +} + +fn main() { + // Creates and enum of type `DropEnum`, which implements `Drop` + let drop_enum = DropEnum::Fancy(FancyNum{num: 10}); + match drop_enum { + // Creates a reference to the inside of `DropEnum::Fancy` + DropEnum::Fancy(ref fancy_field) => // No error! + println!("It was fancy-- {}!", fancy_field.num), + } + // implicit call to `drop_enum.drop()` as drop_enum goes out of scope +} +``` +"##, + } register_diagnostics! { diff --git a/src/librustc_mir/transform/elaborate_drops.rs b/src/librustc_mir/transform/elaborate_drops.rs index c833904adbaea..be1b794ecdfab 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 ceff52409b2f0..8d6458d793474 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 = diff --git a/src/librustc_mir/util/borrowck_errors.rs b/src/librustc_mir/util/borrowck_errors.rs index 81e3cd7f61fbc..37d53ca829e85 100644 --- a/src/librustc_mir/util/borrowck_errors.rs +++ b/src/librustc_mir/util/borrowck_errors.rs @@ -248,6 +248,52 @@ pub trait BorrowckErrors { { self.cannot_assign(span, &format!("immutable static item `{}`", desc), o) } + + fn cannot_move_out_of(&self, move_from_span: Span, move_from_desc: &str, o: Origin) + -> DiagnosticBuilder + { + let mut err = struct_span_err!(self, move_from_span, E0507, + "cannot move out of {}{OGN}", + move_from_desc, OGN=o); + err.span_label( + move_from_span, + format!("cannot move out of {}", move_from_desc)); + err + } + + fn cannot_move_out_of_interior_noncopy(&self, + move_from_span: Span, + ty: ty::Ty, + is_index: bool, + o: Origin) + -> DiagnosticBuilder + { + let type_name = match (&ty.sty, is_index) { + (&ty::TyArray(_, _), true) => "array", + (&ty::TySlice(_), _) => "slice", + _ => span_bug!(move_from_span, "this path should not cause illegal move"), + }; + let mut err = struct_span_err!(self, move_from_span, E0508, + "cannot move out of type `{}`, \ + a non-copy {}{OGN}", + ty, type_name, OGN=o); + err.span_label(move_from_span, "cannot move out of here"); + err + } + + fn cannot_move_out_of_interior_of_drop(&self, + move_from_span: Span, + container_ty: ty::Ty, + o: Origin) + -> DiagnosticBuilder + { + let mut err = struct_span_err!(self, move_from_span, E0509, + "cannot move out of type `{}`, \ + which implements the `Drop` trait{OGN}", + container_ty, OGN=o); + err.span_label(move_from_span, "cannot move out of here"); + err + } } impl<'b, 'tcx, 'gcx> BorrowckErrors for TyCtxt<'b, 'tcx, 'gcx> { diff --git a/src/test/compile-fail/borrowck/borrowck-fn-in-const-a.rs b/src/test/compile-fail/borrowck/borrowck-fn-in-const-a.rs index 3098807f272f3..fcdcf198c2852 100644 --- a/src/test/compile-fail/borrowck/borrowck-fn-in-const-a.rs +++ b/src/test/compile-fail/borrowck/borrowck-fn-in-const-a.rs @@ -8,12 +8,17 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// revisions: ast mir +//[mir]compile-flags: -Z emit-end-regions -Z borrowck-mir + // Check that we check fns appearing in constant declarations. // Issue #22382. const MOVE: fn(&String) -> String = { fn broken(x: &String) -> String { - return *x //~ ERROR cannot move + return *x //[ast]~ ERROR cannot move out of borrowed content [E0507] + //[mir]~^ ERROR (Ast) [E0507] + //[mir]~| ERROR (Mir) [E0507] } broken }; diff --git a/src/test/compile-fail/borrowck/borrowck-move-in-irrefut-pat.rs b/src/test/compile-fail/borrowck/borrowck-move-in-irrefut-pat.rs index ec505faf88502..99b5ef794c2cd 100644 --- a/src/test/compile-fail/borrowck/borrowck-move-in-irrefut-pat.rs +++ b/src/test/compile-fail/borrowck/borrowck-move-in-irrefut-pat.rs @@ -8,19 +8,28 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// revisions: ast mir +//[mir]compile-flags: -Z emit-end-regions -Z borrowck-mir + fn with(f: F) where F: FnOnce(&String) {} fn arg_item(&_x: &String) {} - //~^ ERROR cannot move out of borrowed content + //[ast]~^ ERROR cannot move out of borrowed content [E0507] + //[mir]~^^ ERROR (Ast) [E0507] + //[mir]~| ERROR (Mir) [E0507] fn arg_closure() { with(|&_x| ()) - //~^ ERROR cannot move out of borrowed content + //[ast]~^ ERROR cannot move out of borrowed content [E0507] + //[mir]~^^ ERROR (Ast) [E0507] + //[mir]~| ERROR (Mir) [E0507] } fn let_pat() { let &_x = &"hi".to_string(); - //~^ ERROR cannot move out of borrowed content + //[ast]~^ ERROR cannot move out of borrowed content [E0507] + //[mir]~^^ ERROR (Ast) [E0507] + //[mir]~| ERROR (Mir) [E0507] } pub fn main() {} diff --git a/src/test/compile-fail/borrowck/borrowck-move-out-of-overloaded-auto-deref.rs b/src/test/compile-fail/borrowck/borrowck-move-out-of-overloaded-auto-deref.rs index bf4c74741368c..c7e1ea1b758a1 100644 --- a/src/test/compile-fail/borrowck/borrowck-move-out-of-overloaded-auto-deref.rs +++ b/src/test/compile-fail/borrowck/borrowck-move-out-of-overloaded-auto-deref.rs @@ -8,9 +8,14 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// revisions: ast mir +//[mir]compile-flags: -Z emit-end-regions -Z borrowck-mir + use std::rc::Rc; pub fn main() { let _x = Rc::new(vec![1, 2]).into_iter(); - //~^ ERROR cannot move out of borrowed content + //[ast]~^ ERROR cannot move out of borrowed content [E0507] + //[mir]~^^ ERROR (Ast) [E0507] + //[mir]~| ERROR (Mir) [E0507] } diff --git a/src/test/compile-fail/borrowck/borrowck-move-out-of-static-item.rs b/src/test/compile-fail/borrowck/borrowck-move-out-of-static-item.rs index 8b83b945fd148..9e8021fd108a6 100644 --- a/src/test/compile-fail/borrowck/borrowck-move-out-of-static-item.rs +++ b/src/test/compile-fail/borrowck/borrowck-move-out-of-static-item.rs @@ -8,6 +8,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// revisions: ast mir +//[mir]compile-flags: -Z emit-end-regions -Z borrowck-mir + // Ensure that moves out of static items is forbidden struct Foo { @@ -22,5 +25,7 @@ fn test(f: Foo) { } fn main() { - test(BAR); //~ ERROR cannot move out of static item + test(BAR); //[ast]~ ERROR cannot move out of static item [E0507] + //[mir]~^ ERROR (Ast) [E0507] + //[mir]~| ERROR (Mir) [E0507] } diff --git a/src/test/compile-fail/borrowck/borrowck-move-out-of-struct-with-dtor.rs b/src/test/compile-fail/borrowck/borrowck-move-out-of-struct-with-dtor.rs index 16302d276cee2..982f31b1341c1 100644 --- a/src/test/compile-fail/borrowck/borrowck-move-out-of-struct-with-dtor.rs +++ b/src/test/compile-fail/borrowck/borrowck-move-out-of-struct-with-dtor.rs @@ -8,6 +8,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// revisions: ast mir +//[mir]compile-flags: -Z emit-end-regions -Z borrowck-mir + struct S {f:String} impl Drop for S { fn drop(&mut self) { println!("{}", self.f); } @@ -16,17 +19,23 @@ impl Drop for S { fn move_in_match() { match (S {f:"foo".to_string()}) { S {f:_s} => {} - //~^ ERROR cannot move out of type `S`, which implements the `Drop` trait + //[ast]~^ ERROR cannot move out of type `S`, which implements the `Drop` trait [E0509] + //[mir]~^^ ERROR (Ast) [E0509] + //[mir]~| ERROR (Mir) [E0509] } } fn move_in_let() { let S {f:_s} = S {f:"foo".to_string()}; - //~^ ERROR cannot move out of type `S`, which implements the `Drop` trait + //[ast]~^ ERROR cannot move out of type `S`, which implements the `Drop` trait [E0509] + //[mir]~^^ ERROR (Ast) [E0509] + //[mir]~| ERROR (Mir) [E0509] } fn move_in_fn_arg(S {f:_s}: S) { - //~^ ERROR cannot move out of type `S`, which implements the `Drop` trait + //[ast]~^ ERROR cannot move out of type `S`, which implements the `Drop` trait [E0509] + //[mir]~^^ ERROR (Ast) [E0509] + //[mir]~| ERROR (Mir) [E0509] } fn main() {} diff --git a/src/test/compile-fail/borrowck/borrowck-struct-update-with-dtor.rs b/src/test/compile-fail/borrowck/borrowck-struct-update-with-dtor.rs index c364788a9cc8d..4a1828c69582e 100644 --- a/src/test/compile-fail/borrowck/borrowck-struct-update-with-dtor.rs +++ b/src/test/compile-fail/borrowck/borrowck-struct-update-with-dtor.rs @@ -8,6 +8,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// revisions: ast mir +//[mir]compile-flags: -Z emit-end-regions -Z borrowck-mir + // Issue 4691: Ensure that functional-struct-update can only copy, not // move, when the struct implements Drop. @@ -20,12 +23,16 @@ impl Drop for T { fn drop(&mut self) { } } fn f(s0:S) { let _s2 = S{a: 2, ..s0}; - //~^ error: cannot move out of type `S`, which implements the `Drop` trait + //[ast]~^ error: cannot move out of type `S`, which implements the `Drop` trait + //[mir]~^^ ERROR (Ast) [E0509] + //[mir]~| ERROR (Mir) [E0509] } fn g(s0:T) { let _s2 = T{a: 2, ..s0}; - //~^ error: cannot move out of type `T`, which implements the `Drop` trait + //[ast]~^ error: cannot move out of type `T`, which implements the `Drop` trait + //[mir]~^^ ERROR (Ast) [E0509] + //[mir]~| ERROR (Mir) [E0509] } fn main() { } diff --git a/src/test/compile-fail/borrowck/move-in-static-initializer-issue-38520.rs b/src/test/compile-fail/borrowck/move-in-static-initializer-issue-38520.rs index 3c1980e5b366c..7f3120cc83edc 100644 --- a/src/test/compile-fail/borrowck/move-in-static-initializer-issue-38520.rs +++ b/src/test/compile-fail/borrowck/move-in-static-initializer-issue-38520.rs @@ -8,6 +8,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// revisions: ast mir +//[mir]compile-flags: -Z emit-end-regions -Z borrowck-mir + // Regression test for #38520. Check that moves of `Foo` are not // permitted as `Foo` is not copy (even in a static/const // initializer). @@ -21,8 +24,12 @@ const fn get(x: Foo) -> usize { } const X: Foo = Foo(22); -static Y: usize = get(*&X); //~ ERROR E0507 -const Z: usize = get(*&X); //~ ERROR E0507 +static Y: usize = get(*&X); //[ast]~ ERROR E0507 + //[mir]~^ ERROR (Ast) [E0507] + //[mir]~| ERROR (Mir) [E0507] +const Z: usize = get(*&X); //[ast]~ ERROR E0507 + //[mir]~^ ERROR (Ast) [E0507] + //[mir]~| ERROR (Mir) [E0507] fn main() { }