Auto merge of #48605 - KiChjang:unused-mut-warning, r=nikomatsakis

Allow MIR borrowck to catch unused mutable locals

Fixes #47279.

r? @nikomatsakis

8 files changed, 414 insertions, 119 deletions

diff --git a/src/librustc/ich/impls_mir.rs b/src/librustc/ich/impls_mir.rs
index c73f171806e..437626ff536 100644
--- a/src/librustc/ich/impls_mir.rs
+++ b/src/librustc/ich/impls_mir.rs
@@ -563,6 +563,11 @@ impl<'a, 'gcx> HashStable<StableHashingContext<'a>> for mir::Literal<'gcx> {
 
 impl_stable_hash_for!(struct mir::Location { block, statement_index });
 
+impl_stable_hash_for!(struct mir::BorrowCheckResult<'tcx> {
+    closure_requirements,
+    used_mut_upvars
+});
+
 impl_stable_hash_for!(struct mir::ClosureRegionRequirements<'tcx> {
     num_external_vids,
     outlives_requirements
diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs
index c525c4ed651..c26b3014e53 100644
--- a/src/librustc/mir/mod.rs
+++ b/src/librustc/mir/mod.rs
@@ -21,6 +21,7 @@ use rustc_data_structures::indexed_vec::{IndexVec, Idx};
 use rustc_data_structures::control_flow_graph::dominators::{Dominators, dominators};
 use rustc_data_structures::control_flow_graph::{GraphPredecessors, GraphSuccessors};
 use rustc_data_structures::control_flow_graph::ControlFlowGraph;
+use rustc_data_structures::small_vec::SmallVec;
 use rustc_serialize as serialize;
 use hir::def::CtorKind;
 use hir::def_id::DefId;
@@ -247,6 +248,22 @@ impl<'tcx> Mir<'tcx> {
         })
     }
 
+    /// Returns an iterator over all user-declared mutable arguments and locals.
+    #[inline]
+    pub fn mut_vars_and_args_iter<'a>(&'a self) -> impl Iterator<Item=Local> + 'a {
+        (1..self.local_decls.len()).filter_map(move |index| {
+            let local = Local::new(index);
+            let decl = &self.local_decls[local];
+            if (decl.is_user_variable || index < self.arg_count + 1)
+               && decl.mutability == Mutability::Mut
+            {
+                Some(local)
+            } else {
+                None
+            }
+        })
+    }
+
     /// Returns an iterator over all function arguments.
     #[inline]
     pub fn args_iter(&self) -> impl Iterator<Item=Local> {
@@ -2029,6 +2046,12 @@ pub struct GeneratorLayout<'tcx> {
     pub fields: Vec<LocalDecl<'tcx>>,
 }
 
+#[derive(Clone, Debug, RustcEncodable, RustcDecodable)]
+pub struct BorrowCheckResult<'gcx> {
+    pub closure_requirements: Option<ClosureRegionRequirements<'gcx>>,
+    pub used_mut_upvars: SmallVec<[Field; 8]>,
+}
+
 /// After we borrow check a closure, we are left with various
 /// requirements that we have inferred between the free regions that
 /// appear in the closure's signature or on its field types.  These
diff --git a/src/librustc/ty/maps/mod.rs b/src/librustc/ty/maps/mod.rs
index 57223a3c7b2..d89846a75ef 100644
--- a/src/librustc/ty/maps/mod.rs
+++ b/src/librustc/ty/maps/mod.rs
@@ -211,7 +211,7 @@ define_maps! { <'tcx>
 
     /// Borrow checks the function body. If this is a closure, returns
     /// additional requirements that the closure's creator must verify.
-    [] fn mir_borrowck: MirBorrowCheck(DefId) -> Option<mir::ClosureRegionRequirements<'tcx>>,
+    [] fn mir_borrowck: MirBorrowCheck(DefId) -> mir::BorrowCheckResult<'tcx>,
 
     /// Gets a complete map from all types to their inherent impls.
     /// Not meant to be used directly outside of coherence.
diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs
index 6d832d4060a..519dd574e5a 100644
--- a/src/librustc_borrowck/borrowck/mod.rs
+++ b/src/librustc_borrowck/borrowck/mod.rs
@@ -144,7 +144,10 @@ fn borrowck<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, owner_def_id: DefId)
     {
         check_loans::check_loans(&mut bccx, &loan_dfcx, &flowed_moves, &all_loans, body);
     }
-    unused::check(&mut bccx, body);
+
+    if !tcx.use_mir_borrowck() {
+        unused::check(&mut bccx, body);
+    }
 
     Lrc::new(BorrowCheckResult {
         used_mut_nodes: bccx.used_mut_nodes.into_inner(),
diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs
index 5c7061abbb6..3145be7df85 100644
--- a/src/librustc_mir/borrow_check/mod.rs
+++ b/src/librustc_mir/borrow_check/mod.rs
@@ -17,15 +17,17 @@ use rustc::hir::map::definitions::DefPathData;
 use rustc::infer::InferCtxt;
 use rustc::ty::{self, ParamEnv, TyCtxt};
 use rustc::ty::maps::Providers;
-use rustc::mir::{AssertMessage, BasicBlock, BorrowKind, Location, Place};
-use rustc::mir::{Mir, Mutability, Operand, Projection, ProjectionElem, Rvalue};
-use rustc::mir::{Field, Statement, StatementKind, Terminator, TerminatorKind};
-use rustc::mir::{ClosureRegionRequirements, Local};
+use rustc::lint::builtin::UNUSED_MUT;
+use rustc::mir::{AssertMessage, AggregateKind, BasicBlock, BorrowCheckResult, BorrowKind};
+use rustc::mir::{ClearCrossCrate, Local, Location, Place, Mir, Mutability, Operand};
+use rustc::mir::{Projection, ProjectionElem, Rvalue, Field, Statement, StatementKind};
+use rustc::mir::{Terminator, TerminatorKind};
 
 use rustc_data_structures::control_flow_graph::dominators::Dominators;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::indexed_set::IdxSetBuf;
 use rustc_data_structures::indexed_vec::Idx;
+use rustc_data_structures::small_vec::SmallVec;
 
 use std::rc::Rc;
 
@@ -69,12 +71,15 @@ pub fn provide(providers: &mut Providers) {
 fn mir_borrowck<'a, 'tcx>(
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
     def_id: DefId,
-) -> Option<ClosureRegionRequirements<'tcx>> {
+) -> BorrowCheckResult<'tcx> {
     let input_mir = tcx.mir_validated(def_id);
     debug!("run query mir_borrowck: {}", tcx.item_path_str(def_id));
 
     if !tcx.has_attr(def_id, "rustc_mir_borrowck") && !tcx.use_mir_borrowck() {
-        return None;
+        return BorrowCheckResult {
+            closure_requirements: None,
+            used_mut_upvars: SmallVec::new(),
+        };
     }
 
     let opt_closure_req = tcx.infer_ctxt().enter(|infcx| {
@@ -90,7 +95,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
     infcx: &InferCtxt<'a, 'gcx, 'tcx>,
     input_mir: &Mir<'gcx>,
     def_id: DefId,
-) -> Option<ClosureRegionRequirements<'gcx>> {
+) -> BorrowCheckResult<'gcx> {
     let tcx = infcx.tcx;
     let attributes = tcx.get_attrs(def_id);
     let param_env = tcx.param_env(def_id);
@@ -237,6 +242,8 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
         reservation_error_reported: FxHashSet(),
         moved_error_reported: FxHashSet(),
         nonlexical_regioncx: regioncx,
+        used_mut: FxHashSet(),
+        used_mut_upvars: SmallVec::new(),
         nonlexical_cause_info: None,
         borrow_set,
         dominators,
@@ -252,7 +259,66 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
 
     mbcx.analyze_results(&mut state); // entry point for DataflowResultsConsumer
 
-    opt_closure_req
+    // For each non-user used mutable variable, check if it's been assigned from
+    // a user-declared local. If so, then put that local into the used_mut set.
+    // Note that this set is expected to be small - only upvars from closures
+    // would have a chance of erroneously adding non-user-defined mutable vars
+    // to the set.
+    let temporary_used_locals: FxHashSet<Local> =
+        mbcx.used_mut.iter()
+            .filter(|&local| !mbcx.mir.local_decls[*local].is_user_variable)
+            .cloned()
+            .collect();
+
+    for local in temporary_used_locals {
+        for location in mbcx.mir.find_assignments(local) {
+            for moi in &mbcx.move_data.loc_map[location] {
+                let mpi = &mbcx.move_data.moves[*moi].path;
+                let path = &mbcx.move_data.move_paths[*mpi];
+                debug!("assignment of {:?} to {:?}, adding {:?} to used mutable set",
+                       path.place, local, path.place);
+                if let Place::Local(user_local) = path.place {
+                    mbcx.used_mut.insert(user_local);
+                }
+            }
+        }
+    }
+
+    debug!("mbcx.used_mut: {:?}", mbcx.used_mut);
+
+    for local in mbcx.mir.mut_vars_and_args_iter().filter(|local| !mbcx.used_mut.contains(local)) {
+        if let ClearCrossCrate::Set(ref vsi) = mbcx.mir.visibility_scope_info {
+            let local_decl = &mbcx.mir.local_decls[local];
+
+            // Skip implicit `self` argument for closures
+            if local.index() == 1 && tcx.is_closure(mbcx.mir_def_id) {
+                continue;
+            }
+
+            // Skip over locals that begin with an underscore
+            match local_decl.name {
+                Some(name) if name.as_str().starts_with("_") => continue,
+                _ => {},
+            }
+
+            let source_info = local_decl.source_info;
+            let mut_span = tcx.sess.codemap().span_until_non_whitespace(source_info.span);
+
+            tcx.struct_span_lint_node(
+                UNUSED_MUT,
+                vsi[local_decl.syntactic_scope].lint_root,
+                source_info.span,
+                "variable does not need to be mutable"
+            )
+            .span_suggestion_short(mut_span, "remove this `mut`", "".to_owned())
+            .emit();
+        }
+    }
+
+    BorrowCheckResult {
+        closure_requirements: opt_closure_req,
+        used_mut_upvars: mbcx.used_mut_upvars,
+    }
 }
 
 #[allow(dead_code)]
@@ -287,6 +353,12 @@ pub struct MirBorrowckCtxt<'cx, 'gcx: 'tcx, 'tcx: 'cx> {
     /// This field keeps track of errors reported in the checking of moved variables,
     /// so that we don't report report seemingly duplicate errors.
     moved_error_reported: FxHashSet<Place<'tcx>>,
+    /// This field keeps track of all the local variables that are declared mut and are mutated.
+    /// Used for the warning issued by an unused mutable local variable.
+    used_mut: FxHashSet<Local>,
+    /// If the function we're checking is a closure, then we'll need to report back the list of
+    /// mutable upvars that have been used. This field keeps track of them.
+    used_mut_upvars: SmallVec<[Field; 8]>,
     /// Non-lexical region inference context, if NLL is enabled.  This
     /// contains the results from region inference and lets us e.g.
     /// find out which CFG points are contained in each borrow region.
@@ -684,6 +756,11 @@ enum InitializationRequiringAction {
     Assignment,
 }
 
+struct RootPlace<'d, 'tcx: 'd> {
+    place: &'d Place<'tcx>,
+    is_local_mutation_allowed: LocalMutationIsAllowed,
+}
+
 impl InitializationRequiringAction {
     fn as_noun(self) -> &'static str {
         match self {
@@ -830,7 +907,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         }
 
         let mutability_error =
-            self.check_access_permissions(place_span, rw, is_local_mutation_allowed);
+            self.check_access_permissions(place_span, rw, is_local_mutation_allowed, flow_state);
         let conflict_error =
             self.check_access_for_conflict(context, place_span, sd, rw, flow_state);
 
@@ -1097,9 +1174,34 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                 // `NullOp::Box`?
             }
 
-            Rvalue::Aggregate(ref _aggregate_kind, ref operands) => for operand in operands {
-                self.consume_operand(context, (operand, span), flow_state);
-            },
+            Rvalue::Aggregate(ref aggregate_kind, ref operands) => {
+                // We need to report back the list of mutable upvars that were
+                // moved into the closure and subsequently used by the closure,
+                // in order to populate our used_mut set.
+                if let AggregateKind::Closure(def_id, _) = &**aggregate_kind {
+                    let BorrowCheckResult { used_mut_upvars, .. } = self.tcx.mir_borrowck(*def_id);
+                    debug!("{:?} used_mut_upvars={:?}", def_id, used_mut_upvars);
+                    for field in used_mut_upvars {
+                        match operands[field.index()] {
+                            Operand::Move(Place::Local(local)) => {
+                                self.used_mut.insert(local);
+                            }
+                            Operand::Move(ref place @ Place::Projection(_)) => {
+                                if let Some(field) = self.is_upvar_field_projection(place) {
+                                    self.used_mut_upvars.push(field);
+                                }
+                            }
+                            Operand::Move(Place::Static(..)) |
+                            Operand::Copy(..) |
+                            Operand::Constant(..) => {}
+                        }
+                    }
+                }
+
+                for operand in operands {
+                    self.consume_operand(context, (operand, span), flow_state);
+                }
+            }
         }
     }
 
@@ -1300,7 +1402,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
     ) {
         debug!("check_if_reassignment_to_immutable_state({:?})", place);
         // determine if this path has a non-mut owner (and thus needs checking).
-        if let Ok(()) = self.is_mutable(place, LocalMutationIsAllowed::No) {
+        if let Ok(..) = self.is_mutable(place, LocalMutationIsAllowed::No) {
             return;
         }
         debug!(
@@ -1594,10 +1696,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
     ///
     /// Returns true if an error is reported, false otherwise.
     fn check_access_permissions(
-        &self,
+        &mut self,
         (place, span): (&Place<'tcx>, Span),
         kind: ReadOrWrite,
         is_local_mutation_allowed: LocalMutationIsAllowed,
+        flow_state: &Flows<'cx, 'gcx, 'tcx>,
     ) -> bool {
         debug!(
             "check_access_permissions({:?}, {:?}, {:?})",
@@ -1612,90 +1715,93 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                 }
             }
             Reservation(WriteKind::MutableBorrow(BorrowKind::Mut { .. }))
-            | Write(WriteKind::MutableBorrow(BorrowKind::Mut { .. })) => if let Err(place_err) =
-                self.is_mutable(place, is_local_mutation_allowed)
-            {
-                error_reported = true;
-                let item_msg = self.get_default_err_msg(place);
-                let mut err = self.tcx
-                    .cannot_borrow_path_as_mutable(span, &item_msg, Origin::Mir);
-                err.span_label(span, "cannot borrow as mutable");
-
-                if place != place_err {
-                    if let Some(name) = self.describe_place(place_err) {
-                        err.note(&format!("the value which is causing this path not to be mutable \
-                                           is...: `{}`", name));
-                    }
-                }
-
-                err.emit();
-            },
-            Reservation(WriteKind::Mutate) | Write(WriteKind::Mutate) => {
-
-                if let Err(place_err) = self.is_mutable(place, is_local_mutation_allowed) {
-                    error_reported = true;
-                    let mut err_info = None;
-                    match *place_err {
-
-                        Place::Projection(box Projection {
-                        ref base, elem:ProjectionElem::Deref}) => {
-                            match *base {
-                                Place::Local(local) => {
-                                    let locations = self.mir.find_assignments(local);
-                                        if locations.len() > 0 {
-                                            let item_msg = if error_reported {
-                                                self.get_secondary_err_msg(base)
-                                            } else {
-                                                self.get_default_err_msg(place)
-                                            };
-                                            let sp = self.mir.source_info(locations[0]).span;
-                                            let mut to_suggest_span = String::new();
-                                            if let Ok(src) =
-                                                self.tcx.sess.codemap().span_to_snippet(sp) {
-                                                    to_suggest_span = src[1..].to_string();
-                                            };
-                                            err_info = Some((
-                                                    sp,
-                                                    "consider changing this to be a \
-                                                    mutable reference",
-                                                    to_suggest_span,
-                                                    item_msg,
-                                                    self.get_primary_err_msg(base)));
-                                        }
-                                },
-                            _ => {},
-                            }
-                        },
-                        _ => {},
-                    }
+            | Write(WriteKind::MutableBorrow(BorrowKind::Mut { .. })) => {
+                match self.is_mutable(place, is_local_mutation_allowed) {
+                    Ok(root_place) => self.add_used_mut(root_place, flow_state),
+                    Err(place_err) => {
+                        error_reported = true;
+                        let item_msg = self.get_default_err_msg(place);
+                        let mut err = self.tcx
+                            .cannot_borrow_path_as_mutable(span, &item_msg, Origin::Mir);
+                        err.span_label(span, "cannot borrow as mutable");
 
-                    if let Some((err_help_span,
-                                 err_help_stmt,
-                                 to_suggest_span,
-                                 item_msg,
-                                 sec_span)) = err_info {
-                        let mut err = self.tcx.cannot_assign(span, &item_msg, Origin::Mir);
-                        err.span_suggestion(err_help_span,
-                                            err_help_stmt,
-                                            format!("&mut {}", to_suggest_span));
-                        if place != place_err {
-                            err.span_label(span, sec_span);
-                        }
-                        err.emit()
-                    } else {
-                        let item_msg_ = self.get_default_err_msg(place);
-                        let mut err = self.tcx.cannot_assign(span, &item_msg_, Origin::Mir);
-                        err.span_label(span, "cannot mutate");
                         if place != place_err {
                             if let Some(name) = self.describe_place(place_err) {
                                 err.note(&format!("the value which is causing this path not to be \
-                                                   mutable is...: `{}`", name));
+                                    mutable is...: `{}`", name));
                             }
                         }
+
                         err.emit();
                     }
                 }
             }
+            Reservation(WriteKind::Mutate) | Write(WriteKind::Mutate) => {
+                match self.is_mutable(place, is_local_mutation_allowed) {
+                    Ok(root_place) => self.add_used_mut(root_place, flow_state),
+                    Err(place_err) => {
+                        error_reported = true;
+
+                        let err_info = if let Place::Projection(
+                            box Projection {
+                                base: Place::Local(local),
+                                elem: ProjectionElem::Deref
+                            }
+                        ) = *place_err {
+                            let locations = self.mir.find_assignments(local);
+                            if locations.len() > 0 {
+                                let item_msg = if error_reported {
+                                    self.get_secondary_err_msg(&Place::Local(local))
+                                } else {
+                                    self.get_default_err_msg(place)
+                                };
+                                let sp = self.mir.source_info(locations[0]).span;
+                                let mut to_suggest_span = String::new();
+                                if let Ok(src) =
+                                    self.tcx.sess.codemap().span_to_snippet(sp) {
+                                        to_suggest_span = src[1..].to_string();
+                                };
+                                Some((sp,
+                                      "consider changing this to be a \
+                                      mutable reference",
+                                      to_suggest_span,
+                                      item_msg,
+                                      self.get_primary_err_msg(&Place::Local(local))))
+                            } else {
+                                None
+                            }
+                        } else {
+                            None
+                        };
+
+                        if let Some((err_help_span,
+                                     err_help_stmt,
+                                     to_suggest_span,
+                                     item_msg,
+                                     sec_span)) = err_info {
+                            let mut err = self.tcx.cannot_assign(span, &item_msg, Origin::Mir);
+                            err.span_suggestion(err_help_span,
+                                                err_help_stmt,
+                                                format!("&mut {}", to_suggest_span));
+                            if place != place_err {
+                                err.span_label(span, sec_span);
+                            }
+                            err.emit()
+                        } else {
+                            let item_msg = self.get_default_err_msg(place);
+                            let mut err = self.tcx.cannot_assign(span, &item_msg, Origin::Mir);
+                            err.span_label(span, "cannot mutate");
+                            if place != place_err {
+                                if let Some(name) = self.describe_place(place_err) {
+                                    err.note(&format!("the value which is causing this path not \
+                                                       to be mutable is...: `{}`", name));
+                                }
+                            }
+                            err.emit();
+                        }
+                    }
+                }
+            }
             Reservation(WriteKind::Move)
             | Reservation(WriteKind::StorageDeadOrDrop)
             | Reservation(WriteKind::MutableBorrow(BorrowKind::Shared))
@@ -1722,30 +1828,76 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         error_reported
     }
 
-    /// Can this value be written or borrowed mutably
+    /// Adds the place into the used mutable variables set
+    fn add_used_mut<'d>(
+        &mut self,
+        root_place: RootPlace<'d, 'tcx>,
+        flow_state: &Flows<'cx, 'gcx, 'tcx>
+    ) {
+        match root_place {
+            RootPlace {
+                place: Place::Local(local),
+                is_local_mutation_allowed,
+            } => {
+                if is_local_mutation_allowed != LocalMutationIsAllowed::Yes {
+                    // If the local may be initialized, and it is now currently being
+                    // mutated, then it is justified to be annotated with the `mut`
+                    // keyword, since the mutation may be a possible reassignment.
+                    let mpi = self.move_data.rev_lookup.find_local(*local);
+                    if flow_state.inits.contains(&mpi) {
+                        self.used_mut.insert(*local);
+                    }
+                }
+            }
+            RootPlace {
+                place: place @ Place::Projection(_),
+                is_local_mutation_allowed: _,
+            } => {
+                if let Some(field) = self.is_upvar_field_projection(&place) {
+                    self.used_mut_upvars.push(field);
+                }
+            }
+            RootPlace {
+                place: Place::Static(..),
+                is_local_mutation_allowed: _,
+            } => {}
+        }
+    }
+
+    /// Whether this value be written or borrowed mutably.
+    /// Returns the root place if the place passed in is a projection.
     fn is_mutable<'d>(
         &self,
         place: &'d Place<'tcx>,
         is_local_mutation_allowed: LocalMutationIsAllowed,
-    ) -> Result<(), &'d Place<'tcx>> {
+    ) -> Result<RootPlace<'d, 'tcx>, &'d Place<'tcx>> {
         match *place {
             Place::Local(local) => {
                 let local = &self.mir.local_decls[local];
                 match local.mutability {
                     Mutability::Not => match is_local_mutation_allowed {
-                        LocalMutationIsAllowed::Yes | LocalMutationIsAllowed::ExceptUpvars => {
-                            Ok(())
+                        LocalMutationIsAllowed::Yes => {
+                            Ok(RootPlace {
+                                place,
+                                is_local_mutation_allowed: LocalMutationIsAllowed::Yes
+                            })
+                        }
+                        LocalMutationIsAllowed::ExceptUpvars => {
+                            Ok(RootPlace {
+                                place,
+                                is_local_mutation_allowed: LocalMutationIsAllowed::ExceptUpvars
+                            })
                         }
                         LocalMutationIsAllowed::No => Err(place),
                     },
-                    Mutability::Mut => Ok(()),
+                    Mutability::Mut => Ok(RootPlace { place, is_local_mutation_allowed }),
                 }
             }
             Place::Static(ref static_) =>
                 if self.tcx.is_static(static_.def_id) != Some(hir::Mutability::MutMutable) {
                     Err(place)
                 } else {
-                    Ok(())
+                    Ok(RootPlace { place, is_local_mutation_allowed })
                 },
             Place::Projection(ref proj) => {
                 match proj.elem {
@@ -1781,9 +1933,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                                 match tnm.mutbl {
                                     // `*const` raw pointers are not mutable
                                     hir::MutImmutable => return Err(place),
-                                    // `*mut` raw pointers are always mutable, regardless of context
-                                    // The users have to check by themselve.
-                                    hir::MutMutable => return Ok(()),
+                                    // `*mut` raw pointers are always mutable, regardless of
+                                    // context. The users have to check by themselves.
+                                    hir::MutMutable => {
+                                        return Ok(RootPlace { place, is_local_mutation_allowed });
+                                    }
                                 }
                             }
                             // `Box<T>` owns its content, so mutable if its location is mutable
@@ -1814,7 +1968,34 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                                 }
                                 (Mutability::Not, LocalMutationIsAllowed::Yes)
                                 | (Mutability::Mut, _) => {
-                                    self.is_mutable(&proj.base, is_local_mutation_allowed)
+                                    // Subtle: this is an upvar
+                                    // reference, so it looks like
+                                    // `self.foo` -- we want to double
+                                    // check that the context `*self`
+                                    // is mutable (i.e., this is not a
+                                    // `Fn` closure).  But if that
+                                    // check succeeds, we want to
+                                    // *blame* the mutability on
+                                    // `place` (that is,
+                                    // `self.foo`). This is used to
+                                    // propagate the info about
+                                    // whether mutability declarations
+                                    // are used outwards, so that we register
+                                    // the outer variable as mutable. Otherwise a
+                                    // test like this fails to record the `mut`
+                                    // as needed:
+                                    //
+                                    // ```
+                                    // fn foo<F: FnOnce()>(_f: F) { }
+                                    // fn main() {
+                                    //     let var = Vec::new();
+                                    //     foo(move || {
+                                    //         var.push(1);
+                                    //     });
+                                    // }
+                                    // ```
+                                    let _ = self.is_mutable(&proj.base, is_local_mutation_allowed)?;
+                                    Ok(RootPlace { place, is_local_mutation_allowed })
                                 }
                             }
                         } else {
diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs
index 0ed95a319f7..a811b2c147e 100644
--- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs
+++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs
@@ -1426,7 +1426,9 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
             // these extra requirements are basically like where
             // clauses on the struct.
             AggregateKind::Closure(def_id, substs) => {
-                if let Some(closure_region_requirements) = tcx.mir_borrowck(*def_id) {
+                if let Some(closure_region_requirements) =
+                    tcx.mir_borrowck(*def_id).closure_requirements
+                {
                     closure_region_requirements.apply_requirements(
                         self.infcx,
                         self.body_id,
diff --git a/src/test/compile-fail/lint-unused-mut-variables.rs b/src/test/compile-fail/lint-unused-mut-variables.rs
index 3c76740d2b5..14d836074dc 100644
--- a/src/test/compile-fail/lint-unused-mut-variables.rs
+++ b/src/test/compile-fail/lint-unused-mut-variables.rs
@@ -8,6 +8,9 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// revisions: lexical nll
+#![cfg_attr(nll, feature(nll))]
+
 // Exercise the unused_mut attribute in some positive and negative cases
 
 #![allow(unused_assignments)]
@@ -18,15 +21,22 @@
 
 fn main() {
     // negative cases
-    let mut a = 3; //~ ERROR: variable does not need to be mutable
-    let mut a = 2; //~ ERROR: variable does not need to be mutable
-    let mut b = 3; //~ ERROR: variable does not need to be mutable
-    let mut a = vec![3]; //~ ERROR: variable does not need to be mutable
-    let (mut a, b) = (1, 2); //~ ERROR: variable does not need to be mutable
-    let mut a; //~ ERROR: variable does not need to be mutable
+    let mut a = 3; //[lexical]~ ERROR: variable does not need to be mutable
+                   //[nll]~^ ERROR: variable does not need to be mutable
+    let mut a = 2; //[lexical]~ ERROR: variable does not need to be mutable
+                   //[nll]~^ ERROR: variable does not need to be mutable
+    let mut b = 3; //[lexical]~ ERROR: variable does not need to be mutable
+                   //[nll]~^ ERROR: variable does not need to be mutable
+    let mut a = vec![3]; //[lexical]~ ERROR: variable does not need to be mutable
+                         //[nll]~^ ERROR: variable does not need to be mutable
+    let (mut a, b) = (1, 2); //[lexical]~ ERROR: variable does not need to be mutable
+                             //[nll]~^ ERROR: variable does not need to be mutable
+    let mut a; //[lexical]~ ERROR: variable does not need to be mutable
+               //[nll]~^ ERROR: variable does not need to be mutable
     a = 3;
 
-    let mut b; //~ ERROR: variable does not need to be mutable
+    let mut b; //[lexical]~ ERROR: variable does not need to be mutable
+               //[nll]~^ ERROR: variable does not need to be mutable
     if true {
         b = 3;
     } else {
@@ -34,37 +44,45 @@ fn main() {
     }
 
     match 30 {
-        mut x => {} //~ ERROR: variable does not need to be mutable
+        mut x => {} //[lexical]~ ERROR: variable does not need to be mutable
+                    //[nll]~^ ERROR: variable does not need to be mutable
     }
     match (30, 2) {
-      (mut x, 1) | //~ ERROR: variable does not need to be mutable
+      (mut x, 1) | //[lexical]~ ERROR: variable does not need to be mutable
+                   //[nll]~^ ERROR: variable does not need to be mutable
       (mut x, 2) |
       (mut x, 3) => {
       }
       _ => {}
     }
 
-    let x = |mut y: isize| 10; //~ ERROR: variable does not need to be mutable
-    fn what(mut foo: isize) {} //~ ERROR: variable does not need to be mutable
+    let x = |mut y: isize| 10; //[lexical]~ ERROR: variable does not need to be mutable
+                               //[nll]~^ ERROR: variable does not need to be mutable
+    fn what(mut foo: isize) {} //[lexical]~ ERROR: variable does not need to be mutable
+                               //[nll]~^ ERROR: variable does not need to be mutable
 
-    let mut a = &mut 5; //~ ERROR: variable does not need to be mutable
+    let mut a = &mut 5; //[lexical]~ ERROR: variable does not need to be mutable
+                        //[nll]~^ ERROR: variable does not need to be mutable
     *a = 4;
 
     let mut a = 5;
-    let mut b = (&mut a,);
-    *b.0 = 4; //~^ ERROR: variable does not need to be mutable
+    let mut b = (&mut a,); //[lexical]~ ERROR: variable does not need to be mutable
+    *b.0 = 4;              //[nll]~^ ERROR: variable does not need to be mutable
 
-    let mut x = &mut 1; //~ ERROR: variable does not need to be mutable
+    let mut x = &mut 1; //[lexical]~ ERROR: variable does not need to be mutable
+                        //[nll]~^ ERROR: variable does not need to be mutable
     let mut f = || {
       *x += 1;
     };
     f();
 
     fn mut_ref_arg(mut arg : &mut [u8]) -> &mut [u8] {
-        &mut arg[..] //~^ ERROR: variable does not need to be mutable
+        &mut arg[..] //[lexical]~^ ERROR: variable does not need to be mutable
+                     //[nll]~^^ ERROR: variable does not need to be mutable
     }
 
-    let mut v : &mut Vec<()> = &mut vec![]; //~ ERROR: variable does not need to be mutable
+    let mut v : &mut Vec<()> = &mut vec![]; //[lexical]~ ERROR: variable does not need to be mutable
+                                            //[nll]~^ ERROR: variable does not need to be mutable
     v.push(());
 
     // positive cases
@@ -76,6 +94,12 @@ fn main() {
     callback(|| {
         a.push(3);
     });
+    let mut a = Vec::new();
+    callback(|| {
+        callback(|| {
+            a.push(3);
+        });
+    });
     let (mut a, b) = (1, 2);
     a = 34;
 
@@ -116,5 +140,6 @@ fn foo(mut a: isize) {
 fn bar() {
     #[allow(unused_mut)]
     let mut a = 3;
-    let mut b = vec![2]; //~ ERROR: variable does not need to be mutable
+    let mut b = vec![2]; //[lexical]~ ERROR: variable does not need to be mutable
+                         //[nll]~^ ERROR: variable does not need to be mutable
 }
diff --git a/src/test/run-pass/borrowck/borrowck-unused-mut-locals.rs b/src/test/run-pass/borrowck/borrowck-unused-mut-locals.rs
new file mode 100644
index 00000000000..7f1b6ed1701
--- /dev/null
+++ b/src/test/run-pass/borrowck/borrowck-unused-mut-locals.rs
@@ -0,0 +1,56 @@
+// Copyright 2015 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.
+
+#![feature(nll)]
+#![deny(unused_mut)]
+
+#[derive(Debug)]
+struct A {}
+
+fn init_a() -> A {
+    A {}
+}
+
+#[derive(Debug)]
+struct B<'a> {
+    ed: &'a mut A,
+}
+
+fn init_b<'a>(ed: &'a mut A) -> B<'a> {
+    B { ed }
+}
+
+#[derive(Debug)]
+struct C<'a> {
+    pd: &'a mut B<'a>,
+}
+
+fn init_c<'a>(pd: &'a mut B<'a>) -> C<'a> {
+    C { pd }
+}
+
+#[derive(Debug)]
+struct D<'a> {
+    sd: &'a mut C<'a>,
+}
+
+fn init_d<'a>(sd: &'a mut C<'a>) -> D<'a> {
+    D { sd }
+}
+
+fn main() {
+    let mut a = init_a();
+    let mut b = init_b(&mut a);
+    let mut c = init_c(&mut b);
+
+    let d = init_d(&mut c);
+
+    println!("{:?}", d)
+}