Group move errors before reporting, add suggestions

9 files changed, 502 insertions, 66 deletions

diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs
index ee6cb398acd..b0b88209989 100644
--- a/src/librustc/mir/mod.rs
+++ b/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,
diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs
index 6d77364aae0..a17e897d834 100644
--- a/src/librustc_mir/borrow_check/mod.rs
+++ b/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;
 
@@ -117,40 +117,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
         }
     };
diff --git a/src/librustc_mir/borrow_check/move_errors.rs b/src/librustc_mir/borrow_check/move_errors.rs
new file mode 100644
index 00000000000..bc68708decb
--- /dev/null
+++ b/src/librustc_mir/borrow_check/move_errors.rs
@@ -0,0 +1,388 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use rustc::hir;
+use rustc::mir::*;
+use rustc::ty::{self, TyCtxt};
+use rustc_errors::DiagnosticBuilder;
+use syntax_pos::Span;
+
+use dataflow::move_paths::{IllegalMoveOrigin, IllegalMoveOriginKind, MoveData};
+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:
+//
+// let (x, y) = foo()
+//
+// would move x from the 0 field of some temporary, and y from the 1 field. We
+// group such errors together for cleaner error reporting.
+//
+// Errors are kept separate if they are from places with different parent move
+// paths. For example, this generates two errors:
+//
+// let (&x, &y) = (&String::new(), &String::new());
+#[derive(Debug)]
+enum GroupedMoveError<'tcx> {
+    // Match place can't be moved from
+    // e.g. match x[0] { s => (), } where x: &[String]
+    MovesFromMatchPlace {
+        span: Span,
+        move_from: Place<'tcx>,
+        kind: IllegalMoveOriginKind<'tcx>,
+        binds_to: Vec<Local>,
+    },
+    // Part of a pattern can't be moved from,
+    // e.g. match &String::new() { &x => (), }
+    MovesFromPattern {
+        span: Span,
+        move_from: MovePathIndex,
+        kind: IllegalMoveOriginKind<'tcx>,
+        binds_to: Vec<Local>,
+    },
+    // Everything that isn't from pattern matching.
+    OtherIllegalMove {
+        span: Span,
+        kind: IllegalMoveOriginKind<'tcx>,
+    },
+}
+
+impl<'a, 'gcx, 'tcx> MoveErrorCtxt<'a, 'gcx, 'tcx> {
+    fn report_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>> {
+        let mut grouped_errors = Vec::new();
+        for error in errors {
+            self.append_to_grouped_errors(&mut grouped_errors, error);
+        }
+        grouped_errors
+    }
+
+    fn append_to_grouped_errors(
+        self,
+        grouped_errors: &mut Vec<GroupedMoveError<'tcx>>,
+        error: MoveError<'tcx>,
+    ) {
+        match error {
+            MoveError::UnionMove { .. } => {
+                unimplemented!("don't know how to report union move errors yet.")
+            }
+            MoveError::IllegalMove {
+                cannot_move_out_of: IllegalMoveOrigin { location, kind },
+            } => {
+                let stmt_source_info = self.mir.source_info(location);
+                if let Some(StatementKind::Assign(
+                    Place::Local(local),
+                    Rvalue::Use(Operand::Move(move_from)),
+                )) = self.mir.basic_blocks()[location.block]
+                    .statements
+                    .get(location.statement_index)
+                    .map(|stmt| &stmt.kind)
+                {
+                    let local_decl = &self.mir.local_decls[*local];
+                    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
+                        // reliable if it used the ever initialized dataflow
+                        // but move errors are currently reported before the
+                        // rest of borrowck has run.
+                        if self
+                            .mir
+                            .is_sub_scope(local_decl.source_info.scope, stmt_source_info.scope)
+                        {
+                            self.append_binding_error(
+                                grouped_errors,
+                                kind,
+                                move_from,
+                                *local,
+                                opt_match_place,
+                                match_span,
+                            );
+                        }
+                        return;
+                    }
+                }
+                grouped_errors.push(GroupedMoveError::OtherIllegalMove {
+                    span: stmt_source_info.span,
+                    kind,
+                });
+            }
+        }
+    }
+
+    fn append_binding_error(
+        self,
+        grouped_errors: &mut Vec<GroupedMoveError<'tcx>>,
+        kind: IllegalMoveOriginKind<'tcx>,
+        move_from: &Place<'tcx>,
+        bind_to: Local,
+        match_place: &Option<Place<'tcx>>,
+        match_span: Span,
+    ) {
+        debug!(
+            "append_to_grouped_errors(match_place={:?}, match_span={:?})",
+            match_place, match_span
+        );
+
+        let from_simple_let = match_place.is_none();
+        let match_place = match_place.as_ref().unwrap_or(move_from);
+
+        match self.move_data.rev_lookup.find(match_place) {
+            // Error with the match place
+            LookupResult::Parent(_) => {
+                for ge in &mut *grouped_errors {
+                    if let GroupedMoveError::MovesFromMatchPlace { span, binds_to, .. } = ge {
+                        if match_span == *span {
+                            debug!("appending local({:?}) to list", bind_to);
+                            if !binds_to.is_empty() {
+                                binds_to.push(bind_to);
+                            }
+                            return;
+                        }
+                    }
+                }
+                debug!("found a new move error location");
+
+                // Don't need to point to x in let x = ... .
+                let binds_to = if from_simple_let {
+                    vec![]
+                } else {
+                    vec![bind_to]
+                };
+                grouped_errors.push(GroupedMoveError::MovesFromMatchPlace {
+                    span: match_span,
+                    move_from: match_place.clone(),
+                    kind,
+                    binds_to,
+                });
+            }
+            // Error with the pattern
+            LookupResult::Exact(_) => {
+                let mpi = match self.move_data.rev_lookup.find(move_from) {
+                    LookupResult::Parent(Some(mpi)) => mpi,
+                    // move_from should be a projection from match_place.
+                    _ => unreachable!("Probably not unreachable..."),
+                };
+                for ge in &mut *grouped_errors {
+                    if let GroupedMoveError::MovesFromPattern {
+                        span,
+                        move_from: other_mpi,
+                        binds_to,
+                        ..
+                    } = ge
+                    {
+                        if match_span == *span && mpi == *other_mpi {
+                            debug!("appending local({:?}) to list", bind_to);
+                            binds_to.push(bind_to);
+                            return;
+                        }
+                    }
+                }
+                debug!("found a new move error location");
+                grouped_errors.push(GroupedMoveError::MovesFromPattern {
+                    span: match_span,
+                    move_from: mpi,
+                    kind,
+                    binds_to: vec![bind_to],
+                });
+            }
+        };
+    }
+
+    fn report(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 origin = Origin::Mir;
+            (
+                match kind {
+                    IllegalMoveOriginKind::Static => {
+                        self.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(..) => self
+                                .tcx
+                                .cannot_move_out_of_interior_noncopy(span, ty, None, origin),
+                            _ => self
+                                .tcx
+                                .cannot_move_out_of(span, "borrowed content", origin),
+                        }
+                    }
+                    IllegalMoveOriginKind::InteriorOfTypeWithDestructor { container_ty: ty } => {
+                        self.tcx
+                            .cannot_move_out_of_interior_of_drop(span, ty, origin)
+                    }
+                    IllegalMoveOriginKind::InteriorOfSliceOrArray { ty, is_index } => self
+                        .tcx
+                        .cannot_move_out_of_interior_noncopy(span, ty, Some(*is_index), origin),
+                },
+                span,
+            )
+        };
+
+        self.add_move_hints(error, &mut err, err_span);
+        err.emit();
+    }
+
+    fn add_move_hints(
+        self,
+        error: GroupedMoveError<'tcx>,
+        err: &mut DiagnosticBuilder<'a>,
+        span: Span,
+    ) {
+        match error {
+            GroupedMoveError::MovesFromMatchPlace {
+                mut binds_to,
+                move_from,
+                ..
+            } => {
+                // Ok to suggest a borrow, since the target can't be moved from
+                // anyway.
+                if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(span) {
+                    match move_from {
+                        Place::Projection(ref proj)
+                            if self.suitable_to_remove_deref(proj, &snippet) =>
+                        {
+                            err.span_suggestion(
+                                span,
+                                "consider removing this dereference operator",
+                                format!("{}", &snippet[1..]),
+                            );
+                        }
+                        _ => {
+                            err.span_suggestion(
+                                span,
+                                "consider using a reference instead",
+                                format!("&{}", snippet),
+                            );
+                        }
+                    }
+
+                    binds_to.sort();
+                    binds_to.dedup();
+                    for local in binds_to {
+                        let bind_to = &self.mir.local_decls[local];
+                        let binding_span = bind_to.source_info.span;
+                        err.span_label(
+                            binding_span,
+                            format!(
+                                "move occurs because {} has type `{}`, \
+                                 which does not implement the `Copy` trait",
+                                bind_to.name.unwrap(),
+                                bind_to.ty
+                            ),
+                        );
+                    }
+                }
+            }
+            GroupedMoveError::MovesFromPattern { mut binds_to, .. } => {
+                // Suggest ref, since there might be a move in
+                // another match arm
+                binds_to.sort();
+                binds_to.dedup();
+                for local in binds_to {
+                    let bind_to = &self.mir.local_decls[local];
+                    let binding_span = bind_to.source_info.span;
+
+                    // Suggest ref mut when the user has already written mut.
+                    let ref_kind = match bind_to.mutability {
+                        Mutability::Not => "ref",
+                        Mutability::Mut => "ref mut",
+                    };
+                    match bind_to.name {
+                        Some(name) => {
+                            err.span_suggestion(
+                                binding_span,
+                                "to prevent move, use ref or ref mut",
+                                format!("{} {:?}", ref_kind, name),
+                            );
+                        }
+                        None => {
+                            err.span_label(
+                                span,
+                                format!("Local {:?} is not suitable for ref", bind_to),
+                            );
+                        }
+                    }
+                }
+            }
+            // Nothing to suggest.
+            GroupedMoveError::OtherIllegalMove { .. } => (),
+        }
+    }
+
+    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,
+        };
+
+        proj.elem == ProjectionElem::Deref && snippet.starts_with('*') && match proj.base {
+            Place::Local(local) => {
+                let local_decl = &self.mir.local_decls[local];
+                // If this is a temporary, then this could be from an
+                // overloaded * operator.
+                local_decl.is_user_variable.is_some() && is_shared_ref(local_decl.ty)
+            }
+            Place::Static(ref st) => is_shared_ref(st.ty),
+            Place::Projection(ref proj) => match proj.elem {
+                ProjectionElem::Field(_, ty) => is_shared_ref(ty),
+                _ => false,
+            },
+        }
+    }
+}
diff --git a/src/librustc_mir/build/block.rs b/src/librustc_mir/build/block.rs
index 0fd55f752b8..bbbe757e96e 100644
--- a/src/librustc_mir/build/block.rs
+++ b/src/librustc_mir/build/block.rs
@@ -115,11 +115,21 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                     // Declare the bindings, which may create a source scope.
                     let remainder_span = remainder_scope.span(this.hir.tcx(),
                                                               &this.hir.region_scope_tree);
-                    let scope = this.declare_bindings(None, remainder_span, lint_level, &pattern,
-                                                      ArmHasGuard(false));
+
+                    let scope;
 
                     // Evaluate the initializer, if present.
                     if let Some(init) = initializer {
+                        let initializer_span = init.span();
+
+                        scope = this.declare_bindings(
+                            None,
+                            remainder_span,
+                            lint_level,
+                            &pattern,
+                            ArmHasGuard(false),
+                            Some((None, initializer_span)),
+                        );
                         unpack!(block = this.in_opt_scope(
                             opt_destruction_scope.map(|de|(de, source_info)), block, |this| {
                                 let scope = (init_scope, source_info);
@@ -128,6 +138,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                                 })
                             }));
                     } else {
+                        scope = this.declare_bindings(None, remainder_span, lint_level, &pattern,
+                                                        ArmHasGuard(false), None);
+
                         // FIXME(#47184): We currently only insert `UserAssertTy` statements for
                         // patterns that are bindings, this is as we do not want to deconstruct
                         // the type being assertion to match the pattern.
diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs
index b6af0ed2a4a..79dbdfefeb8 100644
--- a/src/librustc_mir/build/matches/mod.rs
+++ b/src/librustc_mir/build/matches/mod.rs
@@ -44,6 +44,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                       arms: Vec<Arm<'tcx>>)
                       -> BlockAnd<()> {
         let tcx = self.hir.tcx();
+        let discriminant_span = discriminant.span();
         let discriminant_place = unpack!(block = self.as_place(block, discriminant));
 
         // Matching on a `discriminant_place` with an uninhabited type doesn't
@@ -96,7 +97,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
             let scope = self.declare_bindings(None, body.span,
                                               LintLevel::Inherited,
                                               &arm.patterns[0],
-                                              ArmHasGuard(arm.guard.is_some()));
+                                              ArmHasGuard(arm.guard.is_some()),
+                                              Some((Some(&discriminant_place), discriminant_span)));
             (body, scope.unwrap_or(self.source_scope))
         }).collect();
 
@@ -254,7 +256,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
             }
             _ => {
                 let place = unpack!(block = self.as_place(block, initializer));
-                self.place_into_pattern(block, irrefutable_pat, &place)
+                self.place_into_pattern(block, irrefutable_pat, &place, true)
             }
         }
     }
@@ -262,7 +264,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
     pub fn place_into_pattern(&mut self,
                                mut block: BasicBlock,
                                irrefutable_pat: Pattern<'tcx>,
-                               initializer: &Place<'tcx>)
+                               initializer: &Place<'tcx>,
+                               set_match_place: bool)
                                -> BlockAnd<()> {
         // create a dummy candidate
         let mut candidate = Candidate {
@@ -288,6 +291,25 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                       candidate.match_pairs);
         }
 
+        // for matches and function arguments, the place that is being matched
+        // can be set when creating the variables. But the place for
+        // let PATTERN = ... might not even exist until we do the assignment.
+        // so we set it here instead
+        if set_match_place {
+            for binding in &candidate.bindings {
+                let local = self.var_local_id(binding.var_id, OutsideGuard);
+
+                if let Some(ClearCrossCrate::Set(BindingForm::Var(
+                    VarBindingForm {opt_match_place: Some((ref mut match_place, _)), .. }
+                ))) = self.local_decls[local].is_user_variable
+                {
+                    *match_place = Some(initializer.clone());
+                } else {
+                    bug!("Let binding to non-user variable.")
+                }
+            }
+        }
+
         // now apply the bindings, which will also declare the variables
         self.bind_matched_candidate_for_arm_body(block, &candidate.bindings);
 
@@ -302,7 +324,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                             scope_span: Span,
                             lint_level: LintLevel,
                             pattern: &Pattern<'tcx>,
-                            has_guard: ArmHasGuard)
+                            has_guard: ArmHasGuard,
+                            opt_match_place: Option<(Option<&Place<'tcx>>, Span)>)
                             -> Option<SourceScope> {
         assert!(!(visibility_scope.is_some() && lint_level.is_explicit()),
                 "can't have both a visibility and a lint scope at the same time");
@@ -326,7 +349,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
             };
             let visibility_scope = visibility_scope.unwrap();
             this.declare_binding(source_info, visibility_scope, mutability, name, mode, var,
-                                 ty, has_guard);
+                                 ty, has_guard, opt_match_place.map(|(x, y)| (x.cloned(), y)));
         });
         visibility_scope
     }
@@ -1121,7 +1144,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                        mode: BindingMode,
                        var_id: NodeId,
                        var_ty: Ty<'tcx>,
-                       has_guard: ArmHasGuard)
+                       has_guard: ArmHasGuard,
+                       opt_match_place: Option<(Option<Place<'tcx>>, Span)>)
     {
         debug!("declare_binding(var_id={:?}, name={:?}, mode={:?}, var_ty={:?}, \
                 visibility_scope={:?}, source_info={:?})",
@@ -1146,6 +1170,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                 // idents in pat; but complex w/ unclear UI payoff.
                 // Instead, just abandon providing diagnostic info.
                 opt_ty_info: None,
+                opt_match_place,
             }))),
         };
         let for_arm_body = self.local_decls.push(local.clone());
diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs
index 85671414618..cdbb2a13e0e 100644
--- a/src/librustc_mir/build/mod.rs
+++ b/src/librustc_mir/build/mod.rs
@@ -705,6 +705,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
 
             if let Some(pattern) = pattern {
                 let pattern = self.hir.pattern_from_hir(pattern);
+                let span = pattern.span;
 
                 match *pattern.kind {
                     // Don't introduce extra copies for simple bindings
@@ -716,15 +717,19 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                             } else {
                                 let binding_mode = ty::BindingMode::BindByValue(mutability.into());
                                 Some(ClearCrossCrate::Set(BindingForm::Var(VarBindingForm {
-                                    binding_mode, opt_ty_info })))
+                                    binding_mode,
+                                    opt_ty_info,
+                                    opt_match_place: Some((Some(place.clone()), span)),
+                                })))
                             };
                         self.var_indices.insert(var, LocalsForNode::One(local));
                     }
                     _ => {
                         scope = self.declare_bindings(scope, ast_body.span,
                                                       LintLevel::Inherited, &pattern,
-                                                      matches::ArmHasGuard(false));
-                        unpack!(block = self.place_into_pattern(block, pattern, &place));
+                                                      matches::ArmHasGuard(false),
+                                                      Some((Some(&place), span)));
+                        unpack!(block = self.place_into_pattern(block, pattern, &place, false));
                     }
                 }
             }
diff --git a/src/librustc_mir/dataflow/move_paths/builder.rs b/src/librustc_mir/dataflow/move_paths/builder.rs
index 590f9917015..9ffbe21e1e2 100644
--- a/src/librustc_mir/dataflow/move_paths/builder.rs
+++ b/src/librustc_mir/dataflow/move_paths/builder.rs
@@ -109,8 +109,7 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> {
         match *place {
             Place::Local(local) => Ok(self.builder.data.rev_lookup.locals[local]),
             Place::Static(..) => {
-                let span = self.builder.mir.source_info(self.loc).span;
-                Err(MoveError::cannot_move_out_of(span, Static))
+                Err(MoveError::cannot_move_out_of(self.loc, Static))
             }
             Place::Projection(ref proj) => {
                 self.move_path_for_projection(place, proj)
@@ -133,13 +132,13 @@ 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(
-                    mir.source_info(self.loc).span,
+                    self.loc,
                     BorrowedContent { target_ty: place.ty(mir, tcx).to_ty(tcx) })),
             ty::TyAdt(adt, _) if adt.has_dtor(tcx) && !adt.is_box() =>
-                return Err(MoveError::cannot_move_out_of(mir.source_info(self.loc).span,
+                return Err(MoveError::cannot_move_out_of(self.loc,
                                                          InteriorOfTypeWithDestructor {
                     container_ty: place_ty
                 })),
@@ -148,7 +147,7 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> {
                 return Err(MoveError::UnionMove { path: base }),
             ty::TySlice(_) =>
                 return Err(MoveError::cannot_move_out_of(
-                    mir.source_info(self.loc).span,
+                    self.loc,
                     InteriorOfSliceOrArray {
                         ty: place_ty, is_index: match proj.elem {
                             ProjectionElem::Index(..) => true,
@@ -158,7 +157,7 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> {
             ty::TyArray(..) => match proj.elem {
                 ProjectionElem::Index(..) =>
                     return Err(MoveError::cannot_move_out_of(
-                        mir.source_info(self.loc).span,
+                        self.loc,
                         InteriorOfSliceOrArray {
                             ty: place_ty, is_index: true
                         })),
diff --git a/src/librustc_mir/dataflow/move_paths/mod.rs b/src/librustc_mir/dataflow/move_paths/mod.rs
index a73e47bc16a..3051a687eac 100644
--- a/src/librustc_mir/dataflow/move_paths/mod.rs
+++ b/src/librustc_mir/dataflow/move_paths/mod.rs
@@ -209,7 +209,7 @@ pub enum InitKind {
     Deep,
     /// Only does a shallow init
     Shallow,
-    /// This doesn't initialize the variabe on panic (and a panic is possible).
+    /// This doesn't initialize the variable on panic (and a panic is possible).
     NonPanicPathOnly,
 }
 
@@ -271,7 +271,7 @@ impl<'tcx> MovePathLookup<'tcx> {
 
 #[derive(Debug)]
 pub struct IllegalMoveOrigin<'tcx> {
-    pub(crate) span: Span,
+    pub(crate) location: Location,
     pub(crate) kind: IllegalMoveOriginKind<'tcx>,
 }
 
@@ -304,8 +304,8 @@ pub enum MoveError<'tcx> {
 }
 
 impl<'tcx> MoveError<'tcx> {
-    fn cannot_move_out_of(span: Span, kind: IllegalMoveOriginKind<'tcx>) -> Self {
-        let origin = IllegalMoveOrigin { span, kind };
+    fn cannot_move_out_of(location: Location, kind: IllegalMoveOriginKind<'tcx>) -> Self {
+        let origin = IllegalMoveOrigin { location, kind };
         MoveError::IllegalMove { cannot_move_out_of: origin }
     }
 }
diff --git a/src/librustc_mir/hair/mod.rs b/src/librustc_mir/hair/mod.rs
index ab9acdc4950..a8d29df8690 100644
--- a/src/librustc_mir/hair/mod.rs
+++ b/src/librustc_mir/hair/mod.rs
@@ -315,6 +315,15 @@ pub enum LogicalOp {
     Or,
 }
 
+impl<'tcx> ExprRef<'tcx> {
+    pub fn span(&self) -> Span {
+        match self {
+            ExprRef::Hair(expr) => expr.span,
+            ExprRef::Mirror(expr) => expr.span,
+        }
+    }
+}
+
 ///////////////////////////////////////////////////////////////////////////
 // The Mirror trait