From 341e5e3a6176737f62c02d9474fefbce31aeb85f Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Fri, 13 Jul 2018 21:56:04 +0100 Subject: [PATCH 1/4] Use MirBorrowckCtxt while reporting move errors --- src/librustc_mir/borrow_check/mod.rs | 19 ++++----- src/librustc_mir/borrow_check/move_errors.rs | 41 ++++++-------------- 2 files changed, 21 insertions(+), 39 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index c212c1b826bd9..2e0ab522e3a4b 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -34,7 +34,7 @@ use std::rc::Rc; use syntax_pos::Span; use dataflow::indexes::BorrowIndex; -use dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MovePathIndex}; +use dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MoveError, MovePathIndex}; use dataflow::Borrows; use dataflow::DataflowResultsConsumer; use dataflow::FlowAtLocation; @@ -148,13 +148,11 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( let mir = &mir; // no further changes let location_table = &LocationTable::new(mir); - let move_data: MoveData<'tcx> = match MoveData::gather_moves(mir, tcx) { - Ok(move_data) => move_data, - Err((move_data, move_errors)) => { - move_errors::report_move_errors(&mir, tcx, move_errors, &move_data); - move_data - } - }; + let (move_data, move_errors): (MoveData<'tcx>, Option>>) = + match MoveData::gather_moves(mir, tcx) { + Ok(move_data) => (move_data, None), + Err((move_data, move_errors)) => (move_data, Some(move_errors)), + }; let mdpe = MoveDataParamEnv { move_data: move_data, @@ -271,6 +269,9 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( polonius_output, ); + if let Some(errors) = move_errors { + mbcx.report_move_errors(errors); + } mbcx.analyze_results(&mut state); // entry point for DataflowResultsConsumer // For each non-user used mutable variable, check if it's been assigned from @@ -1975,7 +1976,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { ProjectionElem::Field(field, _ty) => { let base_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx); - if base_ty.is_closure() || base_ty.is_generator() { + if base_ty.is_closure() || base_ty.is_generator() { Some(field) } else { None diff --git a/src/librustc_mir/borrow_check/move_errors.rs b/src/librustc_mir/borrow_check/move_errors.rs index bc68708decbd5..d979851376a26 100644 --- a/src/librustc_mir/borrow_check/move_errors.rs +++ b/src/librustc_mir/borrow_check/move_errors.rs @@ -10,34 +10,15 @@ use rustc::hir; use rustc::mir::*; -use rustc::ty::{self, TyCtxt}; +use rustc::ty; use rustc_errors::DiagnosticBuilder; use syntax_pos::Span; -use dataflow::move_paths::{IllegalMoveOrigin, IllegalMoveOriginKind, MoveData}; +use borrow_check::MirBorrowckCtxt; +use dataflow::move_paths::{IllegalMoveOrigin, IllegalMoveOriginKind}; use dataflow::move_paths::{LookupResult, MoveError, MovePathIndex}; use util::borrowck_errors::{BorrowckErrors, Origin}; -pub(crate) fn report_move_errors<'gcx, 'tcx>( - mir: &Mir<'tcx>, - tcx: TyCtxt<'_, 'gcx, 'tcx>, - move_errors: Vec>, - move_data: &MoveData<'tcx>, -) { - MoveErrorCtxt { - mir, - tcx, - move_data, - }.report_errors(move_errors); -} - -#[derive(Copy, Clone)] -struct MoveErrorCtxt<'a, 'gcx: 'tcx, 'tcx: 'a> { - mir: &'a Mir<'tcx>, - tcx: TyCtxt<'a, 'gcx, 'tcx>, - move_data: &'a MoveData<'tcx>, -} - // Often when desugaring a pattern match we may have many individual moves in // MIR that are all part of one operation from the user's point-of-view. For // example: @@ -76,15 +57,15 @@ enum GroupedMoveError<'tcx> { }, } -impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> { - fn report_errors(self, move_errors: Vec>) { +impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { + pub(crate) fn report_move_errors(&self, move_errors: Vec>) { let grouped_errors = self.group_move_errors(move_errors); for error in grouped_errors { self.report(error); } } - fn group_move_errors(self, errors: Vec>) -> Vec> { + fn group_move_errors(&self, errors: Vec>) -> Vec> { let mut grouped_errors = Vec::new(); for error in errors { self.append_to_grouped_errors(&mut grouped_errors, error); @@ -93,7 +74,7 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> { } fn append_to_grouped_errors( - self, + &self, grouped_errors: &mut Vec>, error: MoveError<'tcx>, ) { @@ -158,7 +139,7 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> { } fn append_binding_error( - self, + &self, grouped_errors: &mut Vec>, kind: IllegalMoveOriginKind<'tcx>, move_from: &Place<'tcx>, @@ -236,7 +217,7 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> { }; } - fn report(self, error: GroupedMoveError<'tcx>) { + fn report(&self, error: GroupedMoveError<'tcx>) { let (mut err, err_span) = { let (span, kind): (Span, &IllegalMoveOriginKind) = match error { GroupedMoveError::MovesFromMatchPlace { span, ref kind, .. } @@ -279,7 +260,7 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> { } fn add_move_hints( - self, + &self, error: GroupedMoveError<'tcx>, err: &mut DiagnosticBuilder<'a>, span: Span, @@ -365,7 +346,7 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> { } } - fn suitable_to_remove_deref(self, proj: &PlaceProjection<'tcx>, snippet: &str) -> bool { + fn suitable_to_remove_deref(&self, proj: &PlaceProjection<'tcx>, snippet: &str) -> bool { let is_shared_ref = |ty: ty::Ty| match ty.sty { ty::TypeVariants::TyRef(.., hir::Mutability::MutImmutable) => true, _ => false, From 12412749ab209447611c6d071e187d787c6feeb2 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Fri, 13 Jul 2018 23:39:10 +0100 Subject: [PATCH 2/4] Add specific message when moving from upvars in a non-FnOnce closure --- src/librustc_mir/borrow_check/move_errors.rs | 32 ++++++++++++++++++- .../dataflow/move_paths/builder.rs | 4 +-- src/librustc_mir/dataflow/move_paths/mod.rs | 6 ++-- .../ui/borrowck/borrowck-in-static.nll.stderr | 4 +-- ...upvar-from-non-once-ref-closure.nll.stderr | 4 +-- src/test/ui/error-codes/E0161.nll.stderr | 16 ---------- src/test/ui/issue-4335.nll.stderr | 4 +-- ...owck-call-is-borrow-issue-12224.nll.stderr | 4 +-- 8 files changed, 44 insertions(+), 30 deletions(-) delete mode 100644 src/test/ui/error-codes/E0161.nll.stderr diff --git a/src/librustc_mir/borrow_check/move_errors.rs b/src/librustc_mir/borrow_check/move_errors.rs index d979851376a26..0b49260f88a4e 100644 --- a/src/librustc_mir/borrow_check/move_errors.rs +++ b/src/librustc_mir/borrow_check/move_errors.rs @@ -11,6 +11,7 @@ use rustc::hir; use rustc::mir::*; use rustc::ty; +use rustc_data_structures::indexed_vec::Idx; use rustc_errors::DiagnosticBuilder; use syntax_pos::Span; @@ -230,14 +231,43 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { IllegalMoveOriginKind::Static => { self.tcx.cannot_move_out_of(span, "static item", origin) } - IllegalMoveOriginKind::BorrowedContent { target_ty: ty } => { + IllegalMoveOriginKind::BorrowedContent { target_place: place } => { // Inspect the type of the content behind the // borrow to provide feedback about why this // was a move rather than a copy. + let ty = place.ty(self.mir, self.tcx).to_ty(self.tcx); match ty.sty { ty::TyArray(..) | ty::TySlice(..) => self .tcx .cannot_move_out_of_interior_noncopy(span, ty, None, origin), + ty::TyClosure(def_id, closure_substs) + if !self.mir.upvar_decls.is_empty() + && { + match place { + Place::Projection(ref proj) => { + proj.base == Place::Local(Local::new(1)) + } + Place::Local(_) | Place::Static(_) => unreachable!(), + } + } => + { + let closure_kind_ty = + closure_substs.closure_kind_ty(def_id, self.tcx); + let closure_kind = closure_kind_ty.to_opt_closure_kind(); + let place_description = match closure_kind { + Some(ty::ClosureKind::Fn) => { + "captured variable in an `Fn` closure" + } + Some(ty::ClosureKind::FnMut) => { + "captured variable in an `FnMut` closure" + } + Some(ty::ClosureKind::FnOnce) => { + bug!("closure kind does not match first argument type") + } + None => bug!("closure kind not inferred by borrowck"), + }; + self.tcx.cannot_move_out_of(span, place_description, origin) + } _ => self .tcx .cannot_move_out_of(span, "borrowed content", origin), diff --git a/src/librustc_mir/dataflow/move_paths/builder.rs b/src/librustc_mir/dataflow/move_paths/builder.rs index 9ffbe21e1e2cc..48236e5fdd145 100644 --- a/src/librustc_mir/dataflow/move_paths/builder.rs +++ b/src/librustc_mir/dataflow/move_paths/builder.rs @@ -132,11 +132,11 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> { let mir = self.builder.mir; let tcx = self.builder.tcx; let place_ty = proj.base.ty(mir, tcx).to_ty(tcx); - match place_ty.sty { + match place_ty.sty { ty::TyRef(..) | ty::TyRawPtr(..) => return Err(MoveError::cannot_move_out_of( self.loc, - BorrowedContent { target_ty: place.ty(mir, tcx).to_ty(tcx) })), + BorrowedContent { target_place: place.clone() })), ty::TyAdt(adt, _) if adt.has_dtor(tcx) && !adt.is_box() => return Err(MoveError::cannot_move_out_of(self.loc, InteriorOfTypeWithDestructor { diff --git a/src/librustc_mir/dataflow/move_paths/mod.rs b/src/librustc_mir/dataflow/move_paths/mod.rs index 3051a687eac70..54609674a47d4 100644 --- a/src/librustc_mir/dataflow/move_paths/mod.rs +++ b/src/librustc_mir/dataflow/move_paths/mod.rs @@ -282,9 +282,9 @@ pub(crate) enum IllegalMoveOriginKind<'tcx> { /// Illegal move due to attempt to move from behind a reference. BorrowedContent { - /// The content's type: if erroneous code was trying to move - /// from `*x` where `x: &T`, then this will be `T`. - target_ty: ty::Ty<'tcx>, + /// The place the reference refers to: if erroneous code was trying to + /// move from `(*x).f` this will be `*x`. + target_place: Place<'tcx>, }, /// Illegal move due to attempt to move from field of an ADT that diff --git a/src/test/ui/borrowck/borrowck-in-static.nll.stderr b/src/test/ui/borrowck/borrowck-in-static.nll.stderr index 927d8c3745885..05a022a726cd9 100644 --- a/src/test/ui/borrowck/borrowck-in-static.nll.stderr +++ b/src/test/ui/borrowck/borrowck-in-static.nll.stderr @@ -1,8 +1,8 @@ -error[E0507]: cannot move out of borrowed content +error[E0507]: cannot move out of captured variable in an `Fn` closure --> $DIR/borrowck-in-static.rs:15:17 | LL | Box::new(|| x) //~ ERROR cannot move out of captured outer variable - | ^ cannot move out of borrowed content + | ^ cannot move out of captured variable in an `Fn` closure error: aborting due to previous error diff --git a/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.nll.stderr b/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.nll.stderr index 7464e33e8c101..07a9f374b2c39 100644 --- a/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.nll.stderr +++ b/src/test/ui/borrowck/unboxed-closures-move-upvar-from-non-once-ref-closure.nll.stderr @@ -1,8 +1,8 @@ -error[E0507]: cannot move out of borrowed content +error[E0507]: cannot move out of captured variable in an `Fn` closure --> $DIR/unboxed-closures-move-upvar-from-non-once-ref-closure.rs:21:9 | LL | y.into_iter(); - | ^ cannot move out of borrowed content + | ^ cannot move out of captured variable in an `Fn` closure error: aborting due to previous error diff --git a/src/test/ui/error-codes/E0161.nll.stderr b/src/test/ui/error-codes/E0161.nll.stderr deleted file mode 100644 index 6aaff7433830d..0000000000000 --- a/src/test/ui/error-codes/E0161.nll.stderr +++ /dev/null @@ -1,16 +0,0 @@ -error[E0507]: cannot move out of borrowed content - --> $DIR/E0161.rs:14:28 - | -LL | let _x: Box = box *"hello"; //~ ERROR E0161 - | ^^^^^^^^ cannot move out of borrowed content - -error[E0161]: cannot move a value of type str: the size of str cannot be statically determined - --> $DIR/E0161.rs:14:28 - | -LL | let _x: Box = box *"hello"; //~ ERROR E0161 - | ^^^^^^^^ - -error: aborting due to 2 previous errors - -Some errors occurred: E0161, E0507. -For more information about an error, try `rustc --explain E0161`. diff --git a/src/test/ui/issue-4335.nll.stderr b/src/test/ui/issue-4335.nll.stderr index 8eede34747827..eacd8b5e58047 100644 --- a/src/test/ui/issue-4335.nll.stderr +++ b/src/test/ui/issue-4335.nll.stderr @@ -1,8 +1,8 @@ -error[E0507]: cannot move out of borrowed content +error[E0507]: cannot move out of captured variable in an `FnMut` closure --> $DIR/issue-4335.rs:16:20 | LL | id(Box::new(|| *v)) - | ^^ cannot move out of borrowed content + | ^^ cannot move out of captured variable in an `FnMut` closure error[E0597]: `v` does not live long enough --> $DIR/issue-4335.rs:16:17 diff --git a/src/test/ui/span/borrowck-call-is-borrow-issue-12224.nll.stderr b/src/test/ui/span/borrowck-call-is-borrow-issue-12224.nll.stderr index f752015c650b9..b3563f1b6203c 100644 --- a/src/test/ui/span/borrowck-call-is-borrow-issue-12224.nll.stderr +++ b/src/test/ui/span/borrowck-call-is-borrow-issue-12224.nll.stderr @@ -28,11 +28,11 @@ LL | fn test4(f: &Test) { LL | f.f.call_mut(()) | ^^^ `f` is a `&` reference, so the data it refers to cannot be borrowed as mutable -error[E0507]: cannot move out of borrowed content +error[E0507]: cannot move out of captured variable in an `FnMut` closure --> $DIR/borrowck-call-is-borrow-issue-12224.rs:66:13 | LL | foo(f); - | ^ cannot move out of borrowed content + | ^ cannot move out of captured variable in an `FnMut` closure error[E0505]: cannot move out of `f` because it is borrowed --> $DIR/borrowck-call-is-borrow-issue-12224.rs:65:16 From 086c2d0b134ee3dcc330ab4bde64e4af80a55aeb Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Wed, 18 Jul 2018 21:03:07 +0100 Subject: [PATCH 3/4] Fix #52416 - ice for move errors in unsafe blocks --- src/librustc_mir/borrow_check/move_errors.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/librustc_mir/borrow_check/move_errors.rs b/src/librustc_mir/borrow_check/move_errors.rs index 0b49260f88a4e..b7ba5c855b996 100644 --- a/src/librustc_mir/borrow_check/move_errors.rs +++ b/src/librustc_mir/borrow_check/move_errors.rs @@ -96,19 +96,19 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { .map(|stmt| &stmt.kind) { let local_decl = &self.mir.local_decls[*local]; + // opt_match_place is the + // match_span is the span of the expression being matched on + // match *x.y { ... } match_place is Some(*x.y) + // ^^^^ match_span is the span of *x.y + // + // opt_match_place is None for let [mut] x = ... statements, + // whether or not the right-hand side is a place expression if let Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm { opt_match_place: Some((ref opt_match_place, match_span)), binding_mode: _, opt_ty_info: _, }))) = local_decl.is_user_variable { - // opt_match_place is the - // match_span is the span of the expression being matched on - // match *x.y { ... } match_place is Some(*x.y) - // ^^^^ match_span is the span of *x.y - // opt_match_place is None for let [mut] x = ... statements, - // whether or not the right-hand side is a place expression - // HACK use scopes to determine if this assignment is // the initialization of a variable. // FIXME(matthewjasper) This would probably be more @@ -127,8 +127,8 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { opt_match_place, match_span, ); + return; } - return; } } grouped_errors.push(GroupedMoveError::OtherIllegalMove { From d34924d8247e992cde5ac356440d1c9ccb4c5cdd Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Thu, 19 Jul 2018 21:15:10 +0100 Subject: [PATCH 4/4] update tests --- src/test/ui/issue-20801.nll.stderr | 25 ++++++++++++++++++++++--- src/test/ui/issue-20801.rs | 2 -- src/test/ui/issue-30355.nll.stderr | 12 ++++++------ 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/test/ui/issue-20801.nll.stderr b/src/test/ui/issue-20801.nll.stderr index 39b1405991a31..0394309a6e2bb 100644 --- a/src/test/ui/issue-20801.nll.stderr +++ b/src/test/ui/issue-20801.nll.stderr @@ -1,8 +1,27 @@ -error: internal compiler error: Accessing `(*_8)` with the kind `Write(Move)` shouldn't be possible +error[E0507]: cannot move out of borrowed content + --> $DIR/issue-20801.rs:36:22 + | +LL | let a = unsafe { *mut_ref() }; + | ^^^^^^^^^^ cannot move out of borrowed content + +error[E0507]: cannot move out of borrowed content + --> $DIR/issue-20801.rs:39:22 + | +LL | let b = unsafe { *imm_ref() }; + | ^^^^^^^^^^ cannot move out of borrowed content + +error[E0507]: cannot move out of borrowed content + --> $DIR/issue-20801.rs:42:22 + | +LL | let c = unsafe { *mut_ptr() }; + | ^^^^^^^^^^ cannot move out of borrowed content + +error[E0507]: cannot move out of borrowed content --> $DIR/issue-20801.rs:45:22 | LL | let d = unsafe { *const_ptr() }; - | ^^^^^^^^^^^^ + | ^^^^^^^^^^^^ cannot move out of borrowed content -error: aborting due to previous error +error: aborting due to 4 previous errors +For more information about this error, try `rustc --explain E0507`. diff --git a/src/test/ui/issue-20801.rs b/src/test/ui/issue-20801.rs index 0e8b7fb903769..d3b97a9c05863 100644 --- a/src/test/ui/issue-20801.rs +++ b/src/test/ui/issue-20801.rs @@ -8,8 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// ignore-test currently ICEs when using NLL (#52416) - // We used to ICE when moving out of a `*mut T` or `*const T`. struct T(u8); diff --git a/src/test/ui/issue-30355.nll.stderr b/src/test/ui/issue-30355.nll.stderr index e565ad59116e8..fdf8157dcf833 100644 --- a/src/test/ui/issue-30355.nll.stderr +++ b/src/test/ui/issue-30355.nll.stderr @@ -1,9 +1,3 @@ -error[E0508]: cannot move out of type `[u8]`, a non-copy slice - --> $DIR/issue-30355.rs:15:8 - | -LL | &X(*Y) - | ^^ cannot move out of here - error[E0161]: cannot move a value of type X: the size of X cannot be statically determined --> $DIR/issue-30355.rs:15:6 | @@ -16,6 +10,12 @@ error[E0161]: cannot move a value of type [u8]: the size of [u8] cannot be stati LL | &X(*Y) | ^^ +error[E0508]: cannot move out of type `[u8]`, a non-copy slice + --> $DIR/issue-30355.rs:15:8 + | +LL | &X(*Y) + | ^^ cannot move out of here + error: aborting due to 3 previous errors Some errors occurred: E0161, E0508.