Auto merge of #52359 - matthewjasper:combine-move-error-reporting, r=…

…pnkfelix

[NLL] Small move error reporting improvements

* Use a MirBorrowckContext when reporting errors to be more uniform with other error reporting
* Add a special message for the case of trying to move from capture variables in `Fn` and `FnMut` closures.

part of #51028

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<Vec<MoveError<'tcx>>>) =
+        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

src/librustc_mir/borrow_check/move_errors.rs

@@ -10,34 +10,16 @@
 
 use rustc::hir;
 use rustc::mir::*;
-use rustc::ty::{self, TyCtxt};
+use rustc::ty;
+use rustc_data_structures::indexed_vec::Idx;
 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<MoveError<'tcx>>,
-    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 +58,15 @@ enum GroupedMoveError<'tcx> {
     },
 }
 
-impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> {
-    fn report_errors(self, move_errors: Vec<MoveError<'tcx>>) {
+impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
+    pub(crate) fn report_move_errors(&self, move_errors: Vec<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<MoveError<'tcx>>) -> Vec<GroupedMoveError<'tcx>> {
+    fn group_move_errors(&self, errors: Vec<MoveError<'tcx>>) -> Vec<GroupedMoveError<'tcx>> {
         let mut grouped_errors = Vec::new();
         for error in errors {
             self.append_to_grouped_errors(&mut grouped_errors, error);
@@ -93,7 +75,7 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> {
     }
 
     fn append_to_grouped_errors(
-        self,
+        &self,
         grouped_errors: &mut Vec<GroupedMoveError<'tcx>>,
         error: MoveError<'tcx>,
     ) {
@@ -114,19 +96,19 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'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
@@ -145,8 +127,8 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> {
                                 opt_match_place,
                                 match_span,
                             );
+                            return;
                         }
-                        return;
                     }
                 }
                 grouped_errors.push(GroupedMoveError::OtherIllegalMove {
@@ -158,7 +140,7 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> {
     }
 
     fn append_binding_error(
-        self,
+        &self,
         grouped_errors: &mut Vec<GroupedMoveError<'tcx>>,
         kind: IllegalMoveOriginKind<'tcx>,
         move_from: &Place<'tcx>,
@@ -236,7 +218,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, .. }
@@ -249,14 +231,43 @@ impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'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),
@@ -279,7 +290,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 +376,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,

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 {

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

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
 

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
 

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`.

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);

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.

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

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