From 4b0963653ef2479333d7b37843633fb5d5902f03 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Esteban=20K=C3=BCber?= Date: Fri, 24 May 2019 19:10:00 -0700 Subject: [PATCH] When encountering move error on an `Option`, suggest using `as_ref` --- src/librustc_mir/borrow_check/move_errors.rs | 161 +++++++++--------- .../ui/suggestions/option-content-move.fixed | 21 +++ .../ui/suggestions/option-content-move.rs | 21 +++ .../ui/suggestions/option-content-move.stderr | 12 ++ 4 files changed, 138 insertions(+), 77 deletions(-) create mode 100644 src/test/ui/suggestions/option-content-move.fixed create mode 100644 src/test/ui/suggestions/option-content-move.rs create mode 100644 src/test/ui/suggestions/option-content-move.stderr diff --git a/src/librustc_mir/borrow_check/move_errors.rs b/src/librustc_mir/borrow_check/move_errors.rs index a7bad44c42c..19ad92a8275 100644 --- a/src/librustc_mir/borrow_check/move_errors.rs +++ b/src/librustc_mir/borrow_check/move_errors.rs @@ -242,12 +242,7 @@ fn report(&mut self, error: GroupedMoveError<'tcx>) { let (mut err, err_span) = { let (span, original_path, kind): (Span, &Place<'tcx>, &IllegalMoveOriginKind<'_>) = match error { - GroupedMoveError::MovesFromPlace { - span, - ref original_path, - ref kind, - .. - } | + GroupedMoveError::MovesFromPlace { span, ref original_path, ref kind, .. } | GroupedMoveError::MovesFromValue { span, ref original_path, ref kind, .. } | GroupedMoveError::OtherIllegalMove { span, ref original_path, ref kind } => { (span, original_path, kind) @@ -257,81 +252,93 @@ fn report(&mut self, error: GroupedMoveError<'tcx>) { debug!("report: original_path={:?} span={:?}, kind={:?} \ original_path.is_upvar_field_projection={:?}", original_path, span, kind, self.is_upvar_field_projection(original_path)); - ( - match kind { - IllegalMoveOriginKind::Static => { - self.infcx.tcx.cannot_move_out_of(span, "static item", origin) - } - 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.infcx.tcx).ty; - let is_upvar_field_projection = - self.prefixes(&original_path, PrefixSet::All) - .any(|p| self.is_upvar_field_projection(p).is_some()); - debug!("report: ty={:?}", ty); - match ty.sty { - ty::Array(..) | ty::Slice(..) => - self.infcx.tcx.cannot_move_out_of_interior_noncopy( - span, ty, None, origin - ), - ty::Closure(def_id, closure_substs) - if def_id == self.mir_def_id && is_upvar_field_projection - => { - let closure_kind_ty = - closure_substs.closure_kind_ty(def_id, self.infcx.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"), - }; - debug!("report: closure_kind_ty={:?} closure_kind={:?} \ - place_description={:?}", closure_kind_ty, closure_kind, - place_description); - - let mut diag = self.infcx.tcx.cannot_move_out_of( - span, place_description, origin); - - for prefix in self.prefixes(&original_path, PrefixSet::All) { - if let Some(field) = self.is_upvar_field_projection(prefix) { - let upvar_hir_id = self.upvars[field.index()].var_hir_id; - let upvar_span = self.infcx.tcx.hir().span_by_hir_id( - upvar_hir_id); - diag.span_label(upvar_span, "captured outer variable"); - break; - } + let err = match kind { + IllegalMoveOriginKind::Static => { + self.infcx.tcx.cannot_move_out_of(span, "static item", origin) + } + 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.infcx.tcx).ty; + let is_upvar_field_projection = + self.prefixes(&original_path, PrefixSet::All) + .any(|p| self.is_upvar_field_projection(p).is_some()); + debug!("report: ty={:?}", ty); + let mut err = match ty.sty { + ty::Array(..) | ty::Slice(..) => + self.infcx.tcx.cannot_move_out_of_interior_noncopy( + span, ty, None, origin + ), + ty::Closure(def_id, closure_substs) + if def_id == self.mir_def_id && is_upvar_field_projection + => { + let closure_kind_ty = + closure_substs.closure_kind_ty(def_id, self.infcx.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"), + }; + debug!("report: closure_kind_ty={:?} closure_kind={:?} \ + place_description={:?}", closure_kind_ty, closure_kind, + place_description); + + let mut diag = self.infcx.tcx.cannot_move_out_of( + span, place_description, origin); + + for prefix in self.prefixes(&original_path, PrefixSet::All) { + if let Some(field) = self.is_upvar_field_projection(prefix) { + let upvar_hir_id = self.upvars[field.index()].var_hir_id; + let upvar_span = self.infcx.tcx.hir().span_by_hir_id( + upvar_hir_id); + diag.span_label(upvar_span, "captured outer variable"); + break; } - - diag } - _ => { - let source = self.borrowed_content_source(place); - self.infcx.tcx.cannot_move_out_of( - span, &source.to_string(), origin - ) - }, + + diag } + _ => { + let source = self.borrowed_content_source(place); + self.infcx.tcx.cannot_move_out_of( + span, &source.to_string(), origin + ) + }, + }; + let orig_path_ty = format!( + "{:?}", + original_path.ty(self.mir, self.infcx.tcx).ty, + ); + let snippet = self.infcx.tcx.sess.source_map().span_to_snippet(span).unwrap(); + if orig_path_ty.starts_with("std::option::Option") { + err.span_suggestion( + span, + "consider borrowing the `Option`'s content", + format!("{}.as_ref()", snippet), + Applicability::MaybeIncorrect, + ); } - IllegalMoveOriginKind::InteriorOfTypeWithDestructor { container_ty: ty } => { - self.infcx.tcx - .cannot_move_out_of_interior_of_drop(span, ty, origin) - } - IllegalMoveOriginKind::InteriorOfSliceOrArray { ty, is_index } => - self.infcx.tcx.cannot_move_out_of_interior_noncopy( - span, ty, Some(*is_index), origin - ), - }, - span, - ) + err + } + IllegalMoveOriginKind::InteriorOfTypeWithDestructor { container_ty: ty } => { + self.infcx.tcx + .cannot_move_out_of_interior_of_drop(span, ty, origin) + } + IllegalMoveOriginKind::InteriorOfSliceOrArray { ty, is_index } => + self.infcx.tcx.cannot_move_out_of_interior_noncopy( + span, ty, Some(*is_index), origin + ), + }; + (err, span) }; self.add_move_hints(error, &mut err, err_span); diff --git a/src/test/ui/suggestions/option-content-move.fixed b/src/test/ui/suggestions/option-content-move.fixed new file mode 100644 index 00000000000..7113de9f414 --- /dev/null +++ b/src/test/ui/suggestions/option-content-move.fixed @@ -0,0 +1,21 @@ +//run-rustfix + +pub struct LipogramCorpora { + selections: Vec<(char, Option)>, +} + +impl LipogramCorpora { + pub fn validate_all(&mut self) -> Result<(), char> { + for selection in &self.selections { + if selection.1.is_some() { + if selection.1.as_ref().unwrap().contains(selection.0) { + //~^ ERROR cannot move out of borrowed content + return Err(selection.0); + } + } + } + Ok(()) + } +} + +fn main() {} diff --git a/src/test/ui/suggestions/option-content-move.rs b/src/test/ui/suggestions/option-content-move.rs new file mode 100644 index 00000000000..5a38376ffa6 --- /dev/null +++ b/src/test/ui/suggestions/option-content-move.rs @@ -0,0 +1,21 @@ +//run-rustfix + +pub struct LipogramCorpora { + selections: Vec<(char, Option)>, +} + +impl LipogramCorpora { + pub fn validate_all(&mut self) -> Result<(), char> { + for selection in &self.selections { + if selection.1.is_some() { + if selection.1.unwrap().contains(selection.0) { + //~^ ERROR cannot move out of borrowed content + return Err(selection.0); + } + } + } + Ok(()) + } +} + +fn main() {} diff --git a/src/test/ui/suggestions/option-content-move.stderr b/src/test/ui/suggestions/option-content-move.stderr new file mode 100644 index 00000000000..7f019c3b5b9 --- /dev/null +++ b/src/test/ui/suggestions/option-content-move.stderr @@ -0,0 +1,12 @@ +error[E0507]: cannot move out of borrowed content + --> $DIR/option-content-move.rs:11:20 + | +LL | if selection.1.unwrap().contains(selection.0) { + | ^^^^^^^^^^^ + | | + | cannot move out of borrowed content + | help: consider borrowing the `Option`'s content: `selection.1.as_ref()` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0507`. -- 2.44.0