Label definition of captured variables in errors.

9 files changed, 95 insertions, 63 deletions

diff --git a/src/librustc/mir/tcx.rs b/src/librustc/mir/tcx.rs
index b55843ac527..e972b02b581 100644
--- a/src/librustc/mir/tcx.rs
+++ b/src/librustc/mir/tcx.rs
@@ -127,14 +127,14 @@ impl<'tcx> Place<'tcx> {
     /// of a closure type.
     pub fn is_upvar_field_projection<'cx, 'gcx>(&self, mir: &'cx Mir<'tcx>,
                                                 tcx: &TyCtxt<'cx, 'gcx, 'tcx>) -> Option<Field> {
-        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 @@ impl<'tcx> Place<'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 @@ impl<'tcx> Place<'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 967a8d90330..f96b9b8082f 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<Vec<MoveError<'tcx>>>) =
+    let (move_data, move_errors): (MoveData<'tcx>, Option<Vec<(Place<'tcx>, 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 86bbec795b0..2c3bf83fc2a 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<MoveError<'tcx>>) {
+    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<MoveError<'tcx>>) -> Vec<GroupedMoveError<'tcx>> {
+    fn group_move_errors(
+        &self,
+        errors: Vec<(Place<'tcx>, MoveError<'tcx>)>
+    ) -> Vec<GroupedMoveError<'tcx>> {
         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 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
     fn append_to_grouped_errors(
         &self,
         grouped_errors: &mut Vec<GroupedMoveError<'tcx>>,
+        original_path: Place<'tcx>,
         error: MoveError<'tcx>,
     ) {
         match error {
@@ -116,6 +123,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
                         self.append_binding_error(
                             grouped_errors,
                             kind,
+                            original_path,
                             move_from,
                             *local,
                             opt_match_place,
@@ -127,6 +135,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
                 }
                 grouped_errors.push(GroupedMoveError::OtherIllegalMove {
                     span: stmt_source_info.span,
+                    original_path,
                     kind,
                 });
             }
@@ -137,6 +146,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
         &self,
         grouped_errors: &mut Vec<GroupedMoveError<'tcx>>,
         kind: IllegalMoveOriginKind<'tcx>,
+        original_path: Place<'tcx>,
         move_from: &Place<'tcx>,
         bind_to: Local,
         match_place: &Option<Place<'tcx>>,
@@ -176,6 +186,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
                 grouped_errors.push(GroupedMoveError::MovesFromMatchPlace {
                     span,
                     move_from: match_place.clone(),
+                    original_path,
                     kind,
                     binds_to,
                 });
@@ -206,6 +217,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
                 grouped_errors.push(GroupedMoveError::MovesFromPattern {
                     span: match_span,
                     move_from: mpi,
+                    original_path,
                     kind,
                     binds_to: vec![bind_to],
                 });
@@ -215,13 +227,23 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
 
     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 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, '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 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, '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 25428d2c424..283cccd5117 100644
--- a/src/librustc_mir/borrow_check/mutability_errors.rs
+++ b/src/librustc_mir/borrow_check/mutability_errors.rs
@@ -71,7 +71,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
                 ));
 
                 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 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
                         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 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
 
         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 44e46e90549..60030f27020 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<MoveError<'tcx>>,
+    errors: Vec<(Place<'tcx>, MoveError<'tcx>)>,
 }
 
 impl<'a, 'gcx, 'tcx> MoveDataBuilder<'a, 'gcx, 'tcx> {
@@ -186,7 +186,9 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, 'tcx> {
 }
 
 impl<'a, 'gcx, 'tcx> MoveDataBuilder<'a, 'gcx, 'tcx> {
-    fn finalize(self) -> Result<MoveData<'tcx>, (MoveData<'tcx>, Vec<MoveError<'tcx>>)> {
+    fn finalize(
+        self
+    ) -> Result<MoveData<'tcx>, (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 @@ impl<'a, 'gcx, 'tcx> MoveDataBuilder<'a, 'gcx, 'tcx> {
     }
 }
 
-pub(super) fn gather_moves<'a, 'gcx, 'tcx>(mir: &Mir<'tcx>, tcx: TyCtxt<'a, 'gcx, 'tcx>)
-                                           -> Result<MoveData<'tcx>,
-                                                     (MoveData<'tcx>, Vec<MoveError<'tcx>>)> {
+pub(super) fn gather_moves<'a, 'gcx, 'tcx>(
+    mir: &Mir<'tcx>,
+    tcx: TyCtxt<'a, 'gcx, 'tcx>
+) -> Result<MoveData<'tcx>, (MoveData<'tcx>, Vec<(Place<'tcx>, MoveError<'tcx>)>)> {
     let mut builder = MoveDataBuilder::new(mir, tcx);
 
     builder.gather_args();
@@ -407,7 +410,7 @@ impl<'b, 'a, 'gcx, 'tcx> Gatherer<'b, 'a, 'gcx, '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 64bfd36b7ee..7b4cbdf7131 100644
--- a/src/librustc_mir/dataflow/move_paths/mod.rs
+++ b/src/librustc_mir/dataflow/move_paths/mod.rs
@@ -313,7 +313,7 @@ impl<'tcx> MoveError<'tcx> {
 
 impl<'a, 'gcx, 'tcx> MoveData<'tcx> {
     pub fn gather_moves(mir: &Mir<'tcx>, tcx: TyCtxt<'a, 'gcx, 'tcx>)
-                        -> Result<Self, (Self, Vec<MoveError<'tcx>>)> {
+                        -> Result<Self, (Self, Vec<(Place<'tcx>, 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 05a022a726c..45fa1764f70 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 07a9f374b2c..0eb5fc8c324 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 c7dbc043cda..a05a3911aa7 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<FnMut(isize)>, b: isize| {
+   |         ----- captured outer variable
+...
 LL |         foo(f);
    |             ^ cannot move out of captured variable in an `FnMut` closure