Auto merge of #51729 - matthewjasper:move-errors, r=nikomatsakis

[NLL] Better move errors

Make a number of changes to improve the quality of NLL cannot move errors.

* Group errors that occur in the same `match` with the same cause.
* Suggest `ref`, `&` or removing `*` to avoid the move.
* Show the place being matched on.

Differences from AST borrowck:

* `&` is suggested over `ref` when matching on a place that can't be moved from.
* Removing `*` is suggested instead of adding `&` when applicable.
* Sub-pattern spans aren't used, this would probably need Spans on Places.

Closes #45699
Closes #46627
Closes #51187
Closes #51189

r? @pnkfelix

Author: bors

src/librustc/mir/mod.rs

@@ -497,8 +497,8 @@ pub enum LocalKind {
     ReturnPointer,
 }
 
-#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug, RustcEncodable, RustcDecodable)]
-pub struct VarBindingForm {
+#[derive(Clone, PartialEq, Eq, Hash, Debug, RustcEncodable, RustcDecodable)]
+pub struct VarBindingForm<'tcx> {
     /// Is variable bound via `x`, `mut x`, `ref x`, or `ref mut x`?
     pub binding_mode: ty::BindingMode,
     /// If an explicit type was provided for this variable binding,
@@ -508,21 +508,49 @@ pub struct VarBindingForm {
     /// doing so breaks incremental compilation (as of this writing),
     /// while a `Span` does not cause our tests to fail.
     pub opt_ty_info: Option<Span>,
+    /// Place of the RHS of the =, or the subject of the `match` where this
+    /// variable is initialized. None in the case of `let PATTERN;`.
+    /// Some((None, ..)) in the case of and `let [mut] x = ...` because
+    /// (a) the right-hand side isn't evaluated as a place expression.
+    /// (b) it gives a way to separate this case from the remaining cases
+    ///     for diagnostics.
+    pub opt_match_place: Option<(Option<Place<'tcx>>, Span)>,
 }
 
-#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug, RustcEncodable, RustcDecodable)]
-pub enum BindingForm {
+#[derive(Clone, PartialEq, Eq, Hash, Debug, RustcEncodable, RustcDecodable)]
+pub enum BindingForm<'tcx> {
     /// This is a binding for a non-`self` binding, or a `self` that has an explicit type.
-    Var(VarBindingForm),
+    Var(VarBindingForm<'tcx>),
     /// Binding for a `self`/`&self`/`&mut self` binding where the type is implicit.
     ImplicitSelf,
 }
 
-CloneTypeFoldableAndLiftImpls! { BindingForm, }
+CloneTypeFoldableAndLiftImpls! { BindingForm<'tcx>, }
 
-impl_stable_hash_for!(struct self::VarBindingForm { binding_mode, opt_ty_info });
+impl_stable_hash_for!(struct self::VarBindingForm<'tcx> {
+    binding_mode,
+    opt_ty_info,
+    opt_match_place
+});
+
+mod binding_form_impl {
+    use rustc_data_structures::stable_hasher::{HashStable, StableHasher, StableHasherResult};
+    use ich::StableHashingContext;
 
-impl_stable_hash_for!(enum self::BindingForm { Var(binding), ImplicitSelf, });
+    impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for super::BindingForm<'tcx> {
+        fn hash_stable<W: StableHasherResult>(&self,
+                                            hcx: &mut StableHashingContext<'a>,
+                                            hasher: &mut StableHasher<W>) {
+            use super::BindingForm::*;
+            ::std::mem::discriminant(self).hash_stable(hcx, hasher);
+
+            match self {
+                Var(binding) => binding.hash_stable(hcx, hasher),
+                ImplicitSelf => (),
+            }
+        }
+    }
+}
 
 /// A MIR local.
 ///
@@ -542,7 +570,7 @@ pub struct LocalDecl<'tcx> {
     /// therefore it need not be visible across crates. pnkfelix
     /// currently hypothesized we *need* to wrap this in a
     /// `ClearCrossCrate` as long as it carries as `HirId`.
-    pub is_user_variable: Option<ClearCrossCrate<BindingForm>>,
+    pub is_user_variable: Option<ClearCrossCrate<BindingForm<'tcx>>>,
 
     /// True if this is an internal local
     ///
@@ -670,6 +698,7 @@ impl<'tcx> LocalDecl<'tcx> {
             Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm {
                 binding_mode: ty::BindingMode::BindByValue(_),
                 opt_ty_info: _,
+                opt_match_place: _,
             }))) => true,
 
             // FIXME: might be able to thread the distinction between
@@ -688,6 +717,7 @@ impl<'tcx> LocalDecl<'tcx> {
             Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm {
                 binding_mode: ty::BindingMode::BindByValue(_),
                 opt_ty_info: _,
+                opt_match_place: _,
             }))) => true,
 
             Some(ClearCrossCrate::Set(BindingForm::ImplicitSelf)) => true,

src/librustc_mir/borrow_check/mod.rs

@@ -35,7 +35,6 @@ use syntax_pos::Span;
 
 use dataflow::indexes::BorrowIndex;
 use dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MovePathIndex};
-use dataflow::move_paths::{IllegalMoveOriginKind, MoveError};
 use dataflow::Borrows;
 use dataflow::DataflowResultsConsumer;
 use dataflow::FlowAtLocation;
@@ -62,6 +61,7 @@ mod path_utils;
 crate mod place_ext;
 mod prefixes;
 mod used_muts;
+mod move_errors;
 
 pub(crate) mod nll;
 
@@ -147,40 +147,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
     let move_data: MoveData<'tcx> = match MoveData::gather_moves(mir, tcx) {
         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!("don't 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 { target_ty: ty } => {
-                        // Inspect the type of the content behind the
-                        // borrow to provide feedback about why this
-                        // was a move rather than a copy.
-                        match ty.sty {
-                            ty::TyArray(..) | ty::TySlice(..) => {
-                                tcx.cannot_move_out_of_interior_noncopy(span, ty, None, origin)
-                            }
-                            _ => 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::InteriorOfSliceOrArray { ty, is_index } => {
-                        tcx.cannot_move_out_of_interior_noncopy(span, ty, Some(is_index), origin)
-                    }
-                };
-                err.emit();
-            }
+            move_errors::report_move_errors(&mir, tcx, move_errors, &move_data);
             move_data
         }
     };