diff --git a/src/librustc/mir/tcx.rs b/src/librustc/mir/tcx.rs index b55843ac527dfd9a90907a602bd492c48360dcb2..e972b02b581f78ba6dc1241e83206da1a1ad5314 100644 --- a/src/librustc/mir/tcx.rs +++ b/src/librustc/mir/tcx.rs @@ -127,14 +127,14 @@ pub fn ty<'a, 'gcx, D>(&self, local_decls: &D, tcx: TyCtxt<'a, 'gcx, 'tcx>) -> P /// of a closure type. pub fn is_upvar_field_projection<'cx, 'gcx>(&self, mir: &'cx Mir<'tcx>, tcx: &TyCtxt<'cx, 'gcx, 'tcx>) -> Option { - let place = if let Place::Projection(ref proj) = self { + let (place, by_ref) = if let Place::Projection(ref proj) = self { if let ProjectionElem::Deref = proj.elem { - &proj.base + (&proj.base, true) } else { - self + (self, false) } } else { - self + (self, false) }; match place { @@ -142,7 +142,9 @@ pub fn is_upvar_field_projection<'cx, 'gcx>(&self, mir: &'cx Mir<'tcx>, ProjectionElem::Field(field, _ty) => { let base_ty = proj.base.ty(mir, *tcx).to_ty(*tcx); - if base_ty.is_closure() || base_ty.is_generator() { + if (base_ty.is_closure() || base_ty.is_generator()) && + (!by_ref || mir.upvar_decls[field.index()].by_ref) + { Some(field) } else { None @@ -153,6 +155,21 @@ pub fn is_upvar_field_projection<'cx, 'gcx>(&self, mir: &'cx Mir<'tcx>, _ => None, } } + + /// Strip the deref projections from a `Place`. For example, given `(*(*((*_1).0: &&T)))`, this + /// will return `((*_1).0)`. Once stripped of any deref projections, places can then be + /// checked as upvar field projections using `is_upvar_field_projection`. + pub fn strip_deref_projections(&self) -> &Place<'tcx> { + let mut current = self; + while let Place::Projection(ref proj) = current { + if let ProjectionElem::Deref = proj.elem { + current = &proj.base; + } else { + break; + } + } + current + } } pub enum RvalueInitializationState { diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 967a8d90330968acdeb4106eb0931aa6b4a88a4f..f96b9b8082fa799d2aee174bd92565d7b2cf0c1b 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -151,7 +151,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( let location_table = &LocationTable::new(mir); let mut errors_buffer = Vec::new(); - let (move_data, move_errors): (MoveData<'tcx>, Option>>) = + let (move_data, move_errors): (MoveData<'tcx>, Option, MoveError<'tcx>)>>) = match MoveData::gather_moves(mir, tcx) { Ok(move_data) => (move_data, None), Err((move_data, move_errors)) => (move_data, Some(move_errors)), diff --git a/src/librustc_mir/borrow_check/move_errors.rs b/src/librustc_mir/borrow_check/move_errors.rs index 86bbec795b0f4fc00e2d50684c5b1ec0c3f99e14..2c3bf83fc2a403e4685c4c831b9102ece10661bc 100644 --- a/src/librustc_mir/borrow_check/move_errors.rs +++ b/src/librustc_mir/borrow_check/move_errors.rs @@ -11,8 +11,8 @@ use rustc::hir; use rustc::mir::*; use rustc::ty; -use rustc_data_structures::indexed_vec::Idx; use rustc_errors::DiagnosticBuilder; +use rustc_data_structures::indexed_vec::Idx; use syntax_pos::Span; use borrow_check::MirBorrowckCtxt; @@ -38,6 +38,7 @@ enum GroupedMoveError<'tcx> { // Match place can't be moved from // e.g. match x[0] { s => (), } where x: &[String] MovesFromMatchPlace { + original_path: Place<'tcx>, span: Span, move_from: Place<'tcx>, kind: IllegalMoveOriginKind<'tcx>, @@ -46,6 +47,7 @@ enum GroupedMoveError<'tcx> { // Part of a pattern can't be moved from, // e.g. match &String::new() { &x => (), } MovesFromPattern { + original_path: Place<'tcx>, span: Span, move_from: MovePathIndex, kind: IllegalMoveOriginKind<'tcx>, @@ -53,23 +55,27 @@ enum GroupedMoveError<'tcx> { }, // Everything that isn't from pattern matching. OtherIllegalMove { + original_path: Place<'tcx>, span: Span, kind: IllegalMoveOriginKind<'tcx>, }, } impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> { - pub(crate) fn report_move_errors(&mut self, move_errors: Vec>) { + pub(crate) fn report_move_errors(&mut self, move_errors: Vec<(Place<'tcx>, MoveError<'tcx>)>) { 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<(Place<'tcx>, MoveError<'tcx>)> + ) -> Vec> { let mut grouped_errors = Vec::new(); - for error in errors { - self.append_to_grouped_errors(&mut grouped_errors, error); + for (original_path, error) in errors { + self.append_to_grouped_errors(&mut grouped_errors, original_path, error); } grouped_errors } @@ -77,6 +83,7 @@ fn group_move_errors(&self, errors: Vec>) -> Vec>, + original_path: Place<'tcx>, error: MoveError<'tcx>, ) { match error { @@ -116,6 +123,7 @@ fn append_to_grouped_errors( self.append_binding_error( grouped_errors, kind, + original_path, move_from, *local, opt_match_place, @@ -127,6 +135,7 @@ fn append_to_grouped_errors( } grouped_errors.push(GroupedMoveError::OtherIllegalMove { span: stmt_source_info.span, + original_path, kind, }); } @@ -137,6 +146,7 @@ fn append_binding_error( &self, grouped_errors: &mut Vec>, kind: IllegalMoveOriginKind<'tcx>, + original_path: Place<'tcx>, move_from: &Place<'tcx>, bind_to: Local, match_place: &Option>, @@ -176,6 +186,7 @@ fn append_binding_error( grouped_errors.push(GroupedMoveError::MovesFromMatchPlace { span, move_from: match_place.clone(), + original_path, kind, binds_to, }); @@ -206,6 +217,7 @@ fn append_binding_error( grouped_errors.push(GroupedMoveError::MovesFromPattern { span: match_span, move_from: mpi, + original_path, kind, binds_to: vec![bind_to], }); @@ -215,13 +227,23 @@ fn append_binding_error( fn report(&mut self, error: GroupedMoveError<'tcx>) { let (mut err, err_span) = { - let (span, kind): (Span, &IllegalMoveOriginKind) = match error { - GroupedMoveError::MovesFromMatchPlace { span, ref kind, .. } - | GroupedMoveError::MovesFromPattern { span, ref kind, .. } - | GroupedMoveError::OtherIllegalMove { span, ref kind } => (span, kind), - }; + let (span, original_path, kind): (Span, &Place<'tcx>, &IllegalMoveOriginKind) = + match error { + GroupedMoveError::MovesFromMatchPlace { + span, + ref original_path, + ref kind, + .. + } | + GroupedMoveError::MovesFromPattern { span, ref original_path, ref kind, .. } | + GroupedMoveError::OtherIllegalMove { span, ref original_path, ref kind } => { + (span, original_path, kind) + }, + }; let origin = Origin::Mir; - debug!("report: span={:?}, kind={:?}", span, kind); + debug!("report: original_path={:?} span={:?}, kind={:?} \ + original_path.is_upvar_field_projection={:?}", original_path, span, kind, + original_path.is_upvar_field_projection(self.mir, &self.tcx)); ( match kind { IllegalMoveOriginKind::Static => { @@ -237,17 +259,11 @@ fn report(&mut self, error: GroupedMoveError<'tcx>) { .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::Promoted(_) | - Place::Local(_) | Place::Static(_) => unreachable!(), - } - } => - { + if !self.mir.upvar_decls.is_empty() && + original_path.strip_deref_projections() + .is_upvar_field_projection(self.mir, &self.tcx) + .is_some() + => { let closure_kind_ty = closure_substs.closure_kind_ty(def_id, self.tcx); let closure_kind = closure_kind_ty.to_opt_closure_kind(); @@ -267,7 +283,19 @@ fn report(&mut self, error: GroupedMoveError<'tcx>) { place_description={:?}", closure_kind_ty, closure_kind, place_description); - self.tcx.cannot_move_out_of(span, place_description, origin) + let mut diag = self.tcx.cannot_move_out_of( + span, place_description, origin); + + if let Some(field) = original_path.is_upvar_field_projection( + self.mir, &self.tcx) { + let upvar_decl = &self.mir.upvar_decls[field.index()]; + let upvar_hir_id = upvar_decl.var_hir_id.assert_crate_local(); + let upvar_node_id = self.tcx.hir.hir_to_node_id(upvar_hir_id); + let upvar_span = self.tcx.hir.span(upvar_node_id); + diag.span_label(upvar_span, "captured outer variable"); + } + + diag } _ => self .tcx diff --git a/src/librustc_mir/borrow_check/mutability_errors.rs b/src/librustc_mir/borrow_check/mutability_errors.rs index 25428d2c4241f57b273037fe43c23ff199e5ea89..283cccd51174a1bbe81e32f8c8def62fb5f3805f 100644 --- a/src/librustc_mir/borrow_check/mutability_errors.rs +++ b/src/librustc_mir/borrow_check/mutability_errors.rs @@ -71,7 +71,7 @@ pub(super) fn report_mutability_error( )); item_msg = format!("`{}`", access_place_desc.unwrap()); - if self.is_upvar(access_place) { + if access_place.is_upvar_field_projection(self.mir, &self.tcx).is_some() { reason = ", as it is not declared as mutable".to_string(); } else { let name = self.mir.upvar_decls[upvar_index.index()].debug_name; @@ -90,7 +90,8 @@ pub(super) fn report_mutability_error( the_place_err.ty(self.mir, self.tcx).to_ty(self.tcx) )); - reason = if self.is_upvar(access_place) { + reason = if access_place.is_upvar_field_projection(self.mir, + &self.tcx).is_some() { ", as it is a captured variable in a `Fn` closure".to_string() } else { ", as `Fn` closures cannot mutate their captured variables".to_string() @@ -394,31 +395,6 @@ pub(super) fn report_mutability_error( err.buffer(&mut self.errors_buffer); } - - // Does this place refer to what the user sees as an upvar - fn is_upvar(&self, place: &Place<'tcx>) -> bool { - match *place { - Place::Projection(box Projection { - ref base, - elem: ProjectionElem::Field(_, _), - }) => { - let base_ty = base.ty(self.mir, self.tcx).to_ty(self.tcx); - is_closure_or_generator(base_ty) - } - Place::Projection(box Projection { - base: - Place::Projection(box Projection { - ref base, - elem: ProjectionElem::Field(upvar_index, _), - }), - elem: ProjectionElem::Deref, - }) => { - let base_ty = base.ty(self.mir, self.tcx).to_ty(self.tcx); - is_closure_or_generator(base_ty) && self.mir.upvar_decls[upvar_index.index()].by_ref - } - _ => false, - } - } } fn suggest_ampmut_self<'cx, 'gcx, 'tcx>( diff --git a/src/librustc_mir/dataflow/move_paths/builder.rs b/src/librustc_mir/dataflow/move_paths/builder.rs index 44e46e90549a706950d97da0750bd124f62035df..60030f270205523937958bbb7f78ab341002015c 100644 --- a/src/librustc_mir/dataflow/move_paths/builder.rs +++ b/src/librustc_mir/dataflow/move_paths/builder.rs @@ -27,7 +27,7 @@ struct MoveDataBuilder<'a, 'gcx: 'tcx, 'tcx: 'a> { mir: &'a Mir<'tcx>, tcx: TyCtxt<'a, 'gcx, 'tcx>, data: MoveData<'tcx>, - errors: Vec>, + errors: Vec<(Place<'tcx>, MoveError<'tcx>)>, } impl<'a, 'gcx, 'tcx> MoveDataBuilder<'a, 'gcx, 'tcx> { @@ -186,7 +186,9 @@ fn move_path_for_projection(&mut self, } impl<'a, 'gcx, 'tcx> MoveDataBuilder<'a, 'gcx, 'tcx> { - fn finalize(self) -> Result, (MoveData<'tcx>, Vec>)> { + fn finalize( + self + ) -> Result, (MoveData<'tcx>, Vec<(Place<'tcx>, MoveError<'tcx>)>)> { debug!("{}", { debug!("moves for {:?}:", self.mir.span); for (j, mo) in self.data.moves.iter_enumerated() { @@ -207,9 +209,10 @@ fn finalize(self) -> Result, (MoveData<'tcx>, Vec } } -pub(super) fn gather_moves<'a, 'gcx, 'tcx>(mir: &Mir<'tcx>, tcx: TyCtxt<'a, 'gcx, 'tcx>) - -> Result, - (MoveData<'tcx>, Vec>)> { +pub(super) fn gather_moves<'a, 'gcx, 'tcx>( + mir: &Mir<'tcx>, + tcx: TyCtxt<'a, 'gcx, 'tcx> +) -> Result, (MoveData<'tcx>, Vec<(Place<'tcx>, MoveError<'tcx>)>)> { let mut builder = MoveDataBuilder::new(mir, tcx); builder.gather_args(); @@ -407,7 +410,7 @@ fn gather_move(&mut self, place: &Place<'tcx>) { let path = match self.move_path_for(place) { Ok(path) | Err(MoveError::UnionMove { path }) => path, Err(error @ MoveError::IllegalMove { .. }) => { - self.builder.errors.push(error); + self.builder.errors.push((place.clone(), error)); return; } }; diff --git a/src/librustc_mir/dataflow/move_paths/mod.rs b/src/librustc_mir/dataflow/move_paths/mod.rs index 64bfd36b7eeb6d92faaeb0a19b77c1dcf5c46fb6..7b4cbdf7131b0e67b2c3b78b5c2a3a29210be39a 100644 --- a/src/librustc_mir/dataflow/move_paths/mod.rs +++ b/src/librustc_mir/dataflow/move_paths/mod.rs @@ -313,7 +313,7 @@ fn cannot_move_out_of(location: Location, kind: IllegalMoveOriginKind<'tcx>) -> impl<'a, 'gcx, 'tcx> MoveData<'tcx> { pub fn gather_moves(mir: &Mir<'tcx>, tcx: TyCtxt<'a, 'gcx, 'tcx>) - -> Result>)> { + -> Result, MoveError<'tcx>)>)> { builder::gather_moves(mir, tcx) } } diff --git a/src/test/ui/borrowck/borrowck-in-static.nll.stderr b/src/test/ui/borrowck/borrowck-in-static.nll.stderr index 05a022a726cd977b8013c8bfb45b07db329944cd..45fa1764f7027230af918979f7648d00e8a418cb 100644 --- a/src/test/ui/borrowck/borrowck-in-static.nll.stderr +++ b/src/test/ui/borrowck/borrowck-in-static.nll.stderr @@ -1,6 +1,8 @@ error[E0507]: cannot move out of captured variable in an `Fn` closure --> $DIR/borrowck-in-static.rs:15:17 | +LL | let x = Box::new(0); + | - captured outer variable LL | Box::new(|| x) //~ ERROR cannot move out of captured outer variable | ^ cannot move out of captured variable in an `Fn` closure 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 07a9f374b2c3935fd2368bd4bdee6ab7c960082d..0eb5fc8c324356ad358fa2438ac9bb1c992834a0 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,6 +1,9 @@ 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 | let y = vec![format!("World")]; + | - captured outer variable +LL | call(|| { LL | y.into_iter(); | ^ cannot move out of captured variable in an `Fn` closure 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 c7dbc043cdaee3fa78e23f7525645f50516e5a44..a05a3911aa771144eb177e4b291e274e5b49bc29 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 @@ -31,6 +31,9 @@ LL | f.f.call_mut(()) error[E0507]: cannot move out of captured variable in an `FnMut` closure --> $DIR/borrowck-call-is-borrow-issue-12224.rs:66:13 | +LL | let mut f = move |g: Box, b: isize| { + | ----- captured outer variable +... LL | foo(f); | ^ cannot move out of captured variable in an `FnMut` closure