liveness: Warn about unused captured variables

12 files changed, 505 insertions, 64 deletions

diff --git a/src/librustc_passes/liveness.rs b/src/librustc_passes/liveness.rs
index 00dc8a41350..55978afc594 100644
--- a/src/librustc_passes/liveness.rs
+++ b/src/librustc_passes/liveness.rs
@@ -76,13 +76,10 @@
 //!   is not just used to generate a new value. For example, `x += 1` is
 //!   a read but not a use. This is used to generate better warnings.
 //!
-//! ## Special Variables
+//! ## Special nodes and variables
 //!
-//! We generate various special variables for various, well, special purposes.
-//! These are described in the `specials` struct:
-//!
-//! - `exit_ln`: a live node that is generated to represent every 'exit' from
-//!   the function, whether it be by explicit return, panic, or other means.
+//! We generate various special nodes for various, well, special purposes.
+//! These are described in the `Specials` struct.
 
 use self::LiveNodeKind::*;
 use self::VarKind::*;
@@ -131,6 +128,7 @@ enum LiveNodeKind {
     UpvarNode(Span),
     ExprNode(Span),
     VarDefNode(Span),
+    ClosureNode,
     ExitNode,
 }
 
@@ -140,6 +138,7 @@ fn live_node_kind_to_string(lnk: LiveNodeKind, tcx: TyCtxt<'_>) -> String {
         UpvarNode(s) => format!("Upvar node [{}]", sm.span_to_string(s)),
         ExprNode(s) => format!("Expr node [{}]", sm.span_to_string(s)),
         VarDefNode(s) => format!("Var def node [{}]", sm.span_to_string(s)),
+        ClosureNode => "Closure node".to_owned(),
         ExitNode => "Exit node".to_owned(),
     }
 }
@@ -396,10 +395,12 @@ fn visit_fn<'tcx>(
 
     // compute liveness
     let mut lsets = Liveness::new(&mut fn_maps, def_id);
-    let entry_ln = lsets.compute(&body.value);
+    let entry_ln = lsets.compute(fk, &body, sp, id);
+    lsets.log_liveness(entry_ln, id);
 
     // check for various error conditions
     lsets.visit_body(body);
+    lsets.warn_about_unused_upvars(entry_ln);
     lsets.warn_about_unused_args(body, entry_ln);
 }
 
@@ -634,6 +635,12 @@ impl RWUTable {
 
 #[derive(Copy, Clone)]
 struct Specials {
+    /// A live node representing a point of execution before closure entry &
+    /// after closure exit. Used to calculate liveness of captured variables
+    /// through calls to the same closure. Used for Fn & FnMut closures only.
+    closure_ln: LiveNode,
+    /// A live node representing every 'exit' from the function, whether it be
+    /// by explicit return, panic, or other means.
     exit_ln: LiveNode,
 }
 
@@ -658,11 +665,8 @@ struct Liveness<'a, 'tcx> {
 
 impl<'a, 'tcx> Liveness<'a, 'tcx> {
     fn new(ir: &'a mut IrMaps<'tcx>, def_id: LocalDefId) -> Liveness<'a, 'tcx> {
-        // Special nodes and variables:
-        // - exit_ln represents the end of the fn, either by return or panic
-        // - implicit_ret_var is a pseudo-variable that represents
-        //   an implicit return
         let specials = Specials {
+            closure_ln: ir.add_live_node(ClosureNode),
             exit_ln: ir.add_live_node(ExitNode),
         };
 
@@ -789,6 +793,20 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
         String::from_utf8(wr).unwrap()
     }
 
+    fn log_liveness(&self, entry_ln: LiveNode, hir_id: hir::HirId) {
+        // hack to skip the loop unless debug! is enabled:
+        debug!(
+            "^^ liveness computation results for body {} (entry={:?})",
+            {
+                for ln_idx in 0..self.ir.num_live_nodes {
+                    debug!("{:?}", self.ln_str(LiveNode(ln_idx as u32)));
+                }
+                hir_id
+            },
+            entry_ln
+        );
+    }
+
     fn init_empty(&mut self, ln: LiveNode, succ_ln: LiveNode) {
         self.successors[ln.get()] = succ_ln;
 
@@ -889,33 +907,87 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
         self.rwu_table.assign_unpacked(idx, rwu);
     }
 
-    fn compute(&mut self, body: &hir::Expr<'_>) -> LiveNode {
-        debug!("compute: using id for body, {:?}", body);
+    fn compute(
+        &mut self,
+        fk: FnKind<'_>,
+        body: &hir::Body<'_>,
+        span: Span,
+        id: hir::HirId,
+    ) -> LiveNode {
+        debug!("compute: using id for body, {:?}", body.value);
 
-        let s = self.s;
+        // # Liveness of captured variables
+        //
+        // When computing the liveness for captured variables we take into
+        // account how variable is captured (ByRef vs ByValue) and what is the
+        // closure kind (Generator / FnOnce vs Fn / FnMut).
+        //
+        // Variables captured by reference are assumed to be used on the exit
+        // from the closure.
+        //
+        // In FnOnce closures, variables captured by value are known to be dead
+        // on exit since it is impossible to call the closure again.
+        //
+        // In Fn / FnMut closures, variables captured by value are live on exit
+        // if they are live on the entry to the closure, since only the closure
+        // itself can access them on subsequent calls.
 
         if let Some(upvars) = self.ir.tcx.upvars_mentioned(self.ir.body_owner) {
+            // Mark upvars captured by reference as used after closure exits.
             for (&var_hir_id, upvar) in upvars.iter().rev() {
-                let var = self.variable(var_hir_id, upvar.span);
-                self.acc(s.exit_ln, var, ACC_READ | ACC_USE);
+                let upvar_id = ty::UpvarId {
+                    var_path: ty::UpvarPath { hir_id: var_hir_id },
+                    closure_expr_id: self.ir.body_owner.expect_local(),
+                };
+                match self.tables.upvar_capture(upvar_id) {
+                    ty::UpvarCapture::ByRef(_) => {
+                        let var = self.variable(var_hir_id, upvar.span);
+                        self.acc(self.s.exit_ln, var, ACC_READ | ACC_USE);
+                    }
+                    ty::UpvarCapture::ByValue => {}
+                }
             }
         }
 
-        let entry_ln = self.propagate_through_expr(body, s.exit_ln);
+        let succ = self.propagate_through_expr(&body.value, self.s.exit_ln);
 
-        // hack to skip the loop unless debug! is enabled:
-        debug!(
-            "^^ liveness computation results for body {} (entry={:?})",
-            {
-                for ln_idx in 0..self.ir.num_live_nodes {
-                    debug!("{:?}", self.ln_str(LiveNode(ln_idx as u32)));
-                }
-                body.hir_id
+        match fk {
+            FnKind::Method(..) | FnKind::ItemFn(..) => return succ,
+            FnKind::Closure(..) => {}
+        }
+
+        let ty = self.tables.node_type(id);
+        match ty.kind {
+            ty::Closure(_def_id, substs) => match substs.as_closure().kind() {
+                ty::ClosureKind::Fn => {}
+                ty::ClosureKind::FnMut => {}
+                ty::ClosureKind::FnOnce => return succ,
             },
-            entry_ln
-        );
+            ty::Generator(..) => return succ,
+            _ => {
+                span_bug!(span, "type of closure expr {:?} is not a closure {:?}", id, ty,);
+            }
+        };
 
-        entry_ln
+        // Propagate through calls to the closure.
+        let mut first_merge = true;
+        loop {
+            self.init_from_succ(self.s.closure_ln, succ);
+            for param in body.params {
+                param.pat.each_binding(|_bm, hir_id, _x, ident| {
+                    let var = self.variable(hir_id, ident.span);
+                    self.define(self.s.closure_ln, var);
+                })
+            }
+
+            if !self.merge_from_succ(self.s.exit_ln, self.s.closure_ln, first_merge) {
+                break;
+            }
+            first_merge = false;
+            assert_eq!(succ, self.propagate_through_expr(&body.value, self.s.exit_ln));
+        }
+
+        succ
     }
 
     fn propagate_through_block(&mut self, blk: &hir::Block<'_>, succ: LiveNode) -> LiveNode {
@@ -1533,11 +1605,60 @@ impl<'tcx> Liveness<'_, 'tcx> {
         if name.is_empty() || name.as_bytes()[0] == b'_' { None } else { Some(name) }
     }
 
+    fn warn_about_unused_upvars(&self, entry_ln: LiveNode) {
+        let upvars = match self.ir.tcx.upvars_mentioned(self.ir.body_owner) {
+            None => return,
+            Some(upvars) => upvars,
+        };
+        for (&var_hir_id, upvar) in upvars.iter() {
+            let var = self.variable(var_hir_id, upvar.span);
+            let upvar_id = ty::UpvarId {
+                var_path: ty::UpvarPath { hir_id: var_hir_id },
+                closure_expr_id: self.ir.body_owner.expect_local(),
+            };
+            match self.tables.upvar_capture(upvar_id) {
+                ty::UpvarCapture::ByValue => {}
+                ty::UpvarCapture::ByRef(..) => continue,
+            };
+            if self.used_on_entry(entry_ln, var) {
+                if self.live_on_entry(entry_ln, var).is_none() {
+                    if let Some(name) = self.should_warn(var) {
+                        self.ir.tcx.struct_span_lint_hir(
+                            lint::builtin::UNUSED_ASSIGNMENTS,
+                            var_hir_id,
+                            vec![upvar.span],
+                            |lint| {
+                                lint.build(&format!("value captured by `{}` is never read", name))
+                                    .help("did you mean to capture by reference instead?")
+                                    .emit();
+                            },
+                        );
+                    }
+                }
+            } else {
+                if let Some(name) = self.should_warn(var) {
+                    self.ir.tcx.struct_span_lint_hir(
+                        lint::builtin::UNUSED_VARIABLES,
+                        var_hir_id,
+                        vec![upvar.span],
+                        |lint| {
+                            lint.build(&format!("unused variable: `{}`", name))
+                                .help("did you mean to capture by reference instead?")
+                                .emit();
+                        },
+                    );
+                }
+            }
+        }
+    }
+
     fn warn_about_unused_args(&self, body: &hir::Body<'_>, entry_ln: LiveNode) {
         for p in body.params {
             self.check_unused_vars_in_pat(&p.pat, Some(entry_ln), |spans, hir_id, ln, var| {
                 if self.live_on_entry(ln, var).is_none() {
-                    self.report_dead_assign(hir_id, spans, var, true);
+                    self.report_unsed_assign(hir_id, spans, var, |name| {
+                        format!("value passed to `{}` is never read", name)
+                    });
                 }
             });
         }
@@ -1651,35 +1772,30 @@ impl<'tcx> Liveness<'_, 'tcx> {
 
     fn warn_about_dead_assign(&self, spans: Vec<Span>, hir_id: HirId, ln: LiveNode, var: Variable) {
         if self.live_on_exit(ln, var).is_none() {
-            self.report_dead_assign(hir_id, spans, var, false);
+            self.report_unsed_assign(hir_id, spans, var, |name| {
+                format!("value assigned to `{}` is never read", name)
+            });
         }
     }
 
-    fn report_dead_assign(&self, hir_id: HirId, spans: Vec<Span>, var: Variable, is_param: bool) {
+    fn report_unsed_assign(
+        &self,
+        hir_id: HirId,
+        spans: Vec<Span>,
+        var: Variable,
+        message: impl Fn(&str) -> String,
+    ) {
         if let Some(name) = self.should_warn(var) {
-            if is_param {
-                self.ir.tcx.struct_span_lint_hir(
-                    lint::builtin::UNUSED_ASSIGNMENTS,
-                    hir_id,
-                    spans,
-                    |lint| {
-                        lint.build(&format!("value passed to `{}` is never read", name))
-                            .help("maybe it is overwritten before being read?")
-                            .emit();
-                    },
-                )
-            } else {
-                self.ir.tcx.struct_span_lint_hir(
-                    lint::builtin::UNUSED_ASSIGNMENTS,
-                    hir_id,
-                    spans,
-                    |lint| {
-                        lint.build(&format!("value assigned to `{}` is never read", name))
-                            .help("maybe it is overwritten before being read?")
-                            .emit();
-                    },
-                )
-            }
+            self.ir.tcx.struct_span_lint_hir(
+                lint::builtin::UNUSED_ASSIGNMENTS,
+                hir_id,
+                spans,
+                |lint| {
+                    lint.build(&message(&name))
+                        .help("maybe it is overwritten before being read?")
+                        .emit();
+                },
+            )
         }
     }
 }
diff --git a/src/test/ui/closures/closure-immutable-outer-variable.fixed b/src/test/ui/closures/closure-immutable-outer-variable.fixed
index 102f1f94a36..1b0feede34e 100644
--- a/src/test/ui/closures/closure-immutable-outer-variable.fixed
+++ b/src/test/ui/closures/closure-immutable-outer-variable.fixed
@@ -8,6 +8,6 @@ fn foo(mut f: Box<dyn FnMut()>) {
 
 fn main() {
     let mut y = true;
-    foo(Box::new(move || y = false) as Box<_>);
+    foo(Box::new(move || y = !y) as Box<_>);
     //~^ ERROR cannot assign to `y`, as it is not declared as mutable
 }
diff --git a/src/test/ui/closures/closure-immutable-outer-variable.rs b/src/test/ui/closures/closure-immutable-outer-variable.rs
index 6eb43b372c9..50ec1c6148a 100644
--- a/src/test/ui/closures/closure-immutable-outer-variable.rs
+++ b/src/test/ui/closures/closure-immutable-outer-variable.rs
@@ -8,6 +8,6 @@ fn foo(mut f: Box<dyn FnMut()>) {
 
 fn main() {
     let y = true;
-    foo(Box::new(move || y = false) as Box<_>);
+    foo(Box::new(move || y = !y) as Box<_>);
     //~^ ERROR cannot assign to `y`, as it is not declared as mutable
 }
diff --git a/src/test/ui/closures/closure-immutable-outer-variable.stderr b/src/test/ui/closures/closure-immutable-outer-variable.stderr
index 7e60f3cd8ff..799097889cd 100644
--- a/src/test/ui/closures/closure-immutable-outer-variable.stderr
+++ b/src/test/ui/closures/closure-immutable-outer-variable.stderr
@@ -3,8 +3,8 @@ error[E0594]: cannot assign to `y`, as it is not declared as mutable
    |
 LL |     let y = true;
    |         - help: consider changing this to be mutable: `mut y`
-LL |     foo(Box::new(move || y = false) as Box<_>);
-   |                          ^^^^^^^^^ cannot assign
+LL |     foo(Box::new(move || y = !y) as Box<_>);
+   |                          ^^^^^^ cannot assign
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/issues/issue-11958.rs b/src/test/ui/issues/issue-11958.rs
index 8fe8a8c6061..a7af01e25b4 100644
--- a/src/test/ui/issues/issue-11958.rs
+++ b/src/test/ui/issues/issue-11958.rs
@@ -1,5 +1,4 @@
 // run-pass
-#![forbid(warnings)]
 
 // We shouldn't need to rebind a moved upvar as mut if it's already
 // marked as mut
@@ -7,4 +6,6 @@
 pub fn main() {
     let mut x = 1;
     let _thunk = Box::new(move|| { x = 2; });
+    //~^ WARN value assigned to `x` is never read
+    //~| WARN unused variable: `x`
 }
diff --git a/src/test/ui/issues/issue-11958.stderr b/src/test/ui/issues/issue-11958.stderr
new file mode 100644
index 00000000000..25de6ff4c11
--- /dev/null
+++ b/src/test/ui/issues/issue-11958.stderr
@@ -0,0 +1,20 @@
+warning: value assigned to `x` is never read
+  --> $DIR/issue-11958.rs:8:36
+   |
+LL |     let _thunk = Box::new(move|| { x = 2; });
+   |                                    ^
+   |
+   = note: `#[warn(unused_assignments)]` on by default
+   = help: maybe it is overwritten before being read?
+
+warning: unused variable: `x`
+  --> $DIR/issue-11958.rs:8:36
+   |
+LL |     let _thunk = Box::new(move|| { x = 2; });
+   |                                    ^
+   |
+   = note: `#[warn(unused_variables)]` on by default
+   = help: did you mean to capture by reference instead?
+
+warning: 2 warnings emitted
+
diff --git a/src/test/ui/liveness/liveness-upvars.rs b/src/test/ui/liveness/liveness-upvars.rs
new file mode 100644
index 00000000000..b2837e74b8c
--- /dev/null
+++ b/src/test/ui/liveness/liveness-upvars.rs
@@ -0,0 +1,108 @@
+// edition:2018
+// check-pass
+#![warn(unused)]
+#![allow(unreachable_code)]
+
+pub fn unintentional_copy_one() {
+    let mut last = None;
+    let mut f = move |s| {
+        last = Some(s); //~  WARN value assigned to `last` is never read
+                        //~| WARN unused variable: `last`
+    };
+    f("a");
+    f("b");
+    f("c");
+    dbg!(last.unwrap());
+}
+
+pub fn unintentional_copy_two() {
+    let mut sum = 0;
+    (1..10).for_each(move |x| {
+        sum += x; //~ WARN unused variable: `sum`
+    });
+    dbg!(sum);
+}
+
+pub fn f() {
+    let mut c = 0;
+
+    // Captured by value, but variable is dead on entry.
+    move || {
+        c = 1; //~ WARN value captured by `c` is never read
+        println!("{}", c);
+    };
+    let _ = async move {
+        c = 1; //~ WARN value captured by `c` is never read
+        println!("{}", c);
+    };
+
+    // Read and written to, but never actually used.
+    move || {
+        c += 1; //~ WARN unused variable: `c`
+    };
+    let _ = async move {
+        c += 1; //~  WARN value assigned to `c` is never read
+                //~| WARN unused variable: `c`
+    };
+
+    move || {
+        println!("{}", c);
+        // Value is read by closure itself on later invocations.
+        c += 1;
+    };
+    let b = Box::new(42);
+    move || {
+        println!("{}", c);
+        // Never read because this is FnOnce closure.
+        c += 1; //~  WARN value assigned to `c` is never read
+        drop(b);
+    };
+    let _ = async move {
+        println!("{}", c);
+        // Never read because this is a generator.
+        c += 1; //~  WARN value assigned to `c` is never read
+    };
+}
+
+pub fn nested() {
+    let mut d = None;
+    let mut e = None;
+    || {
+        || {
+            d = Some("d1"); //~ WARN value assigned to `d` is never read
+            d = Some("d2");
+        };
+        move || {
+            e = Some("e1"); //~  WARN value assigned to `e` is never read
+                            //~| WARN unused variable: `e`
+            e = Some("e2"); //~  WARN value assigned to `e` is never read
+        };
+    };
+}
+
+pub fn g<T: Default>(mut v: T) {
+    |r| {
+        if r {
+            v = T::default(); //~ WARN value assigned to `v` is never read
+        } else {
+            drop(v);
+        }
+    };
+}
+
+pub fn h<T: Copy + Default + std::fmt::Debug>() {
+    let mut z = T::default();
+    move |b| {
+        loop {
+            if b {
+                z = T::default(); //~  WARN value assigned to `z` is never read
+                                  //~| WARN unused variable: `z`
+            } else {
+                return;
+            }
+        }
+        dbg!(z);
+    };
+}
+
+fn main() {}
diff --git a/src/test/ui/liveness/liveness-upvars.stderr b/src/test/ui/liveness/liveness-upvars.stderr
new file mode 100644
index 00000000000..14fed917864
--- /dev/null
+++ b/src/test/ui/liveness/liveness-upvars.stderr
@@ -0,0 +1,150 @@
+warning: value assigned to `last` is never read
+  --> $DIR/liveness-upvars.rs:9:9
+   |
+LL |         last = Some(s);
+   |         ^^^^
+   |
+note: the lint level is defined here
+  --> $DIR/liveness-upvars.rs:3:9
+   |
+LL | #![warn(unused)]
+   |         ^^^^^^
+   = note: `#[warn(unused_assignments)]` implied by `#[warn(unused)]`
+   = help: maybe it is overwritten before being read?
+
+warning: unused variable: `last`
+  --> $DIR/liveness-upvars.rs:9:9
+   |
+LL |         last = Some(s);
+   |         ^^^^
+   |
+note: the lint level is defined here
+  --> $DIR/liveness-upvars.rs:3:9
+   |
+LL | #![warn(unused)]
+   |         ^^^^^^
+   = note: `#[warn(unused_variables)]` implied by `#[warn(unused)]`
+   = help: did you mean to capture by reference instead?
+
+warning: unused variable: `sum`
+  --> $DIR/liveness-upvars.rs:21:9
+   |
+LL |         sum += x;
+   |         ^^^
+   |
+   = help: did you mean to capture by reference instead?
+
+warning: value captured by `c` is never read
+  --> $DIR/liveness-upvars.rs:31:9
+   |
+LL |         c = 1;
+   |         ^
+   |
+   = help: did you mean to capture by reference instead?
+
+warning: value captured by `c` is never read
+  --> $DIR/liveness-upvars.rs:35:9
+   |
+LL |         c = 1;
+   |         ^
+   |
+   = help: did you mean to capture by reference instead?
+
+warning: unused variable: `c`
+  --> $DIR/liveness-upvars.rs:41:9
+   |
+LL |         c += 1;
+   |         ^
+   |
+   = help: did you mean to capture by reference instead?
+
+warning: value assigned to `c` is never read
+  --> $DIR/liveness-upvars.rs:44:9
+   |
+LL |         c += 1;
+   |         ^
+   |
+   = help: maybe it is overwritten before being read?
+
+warning: unused variable: `c`
+  --> $DIR/liveness-upvars.rs:44:9
+   |
+LL |         c += 1;
+   |         ^
+   |
+   = help: did you mean to capture by reference instead?
+
+warning: value assigned to `c` is never read
+  --> $DIR/liveness-upvars.rs:57:9
+   |
+LL |         c += 1;
+   |         ^
+   |
+   = help: maybe it is overwritten before being read?
+
+warning: value assigned to `c` is never read
+  --> $DIR/liveness-upvars.rs:63:9
+   |
+LL |         c += 1;
+   |         ^
+   |
+   = help: maybe it is overwritten before being read?
+
+warning: value assigned to `d` is never read
+  --> $DIR/liveness-upvars.rs:72:13
+   |
+LL |             d = Some("d1");
+   |             ^
+   |
+   = help: maybe it is overwritten before being read?
+
+warning: value assigned to `e` is never read
+  --> $DIR/liveness-upvars.rs:76:13
+   |
+LL |             e = Some("e1");
+   |             ^
+   |
+   = help: maybe it is overwritten before being read?
+
+warning: value assigned to `e` is never read
+  --> $DIR/liveness-upvars.rs:78:13
+   |
+LL |             e = Some("e2");
+   |             ^
+   |
+   = help: maybe it is overwritten before being read?
+
+warning: unused variable: `e`
+  --> $DIR/liveness-upvars.rs:76:13
+   |
+LL |             e = Some("e1");
+   |             ^
+   |
+   = help: did you mean to capture by reference instead?
+
+warning: value assigned to `v` is never read
+  --> $DIR/liveness-upvars.rs:86:13
+   |
+LL |             v = T::default();
+   |             ^
+   |
+   = help: maybe it is overwritten before being read?
+
+warning: value assigned to `z` is never read
+  --> $DIR/liveness-upvars.rs:98:17
+   |
+LL |                 z = T::default();
+   |                 ^
+   |
+   = help: maybe it is overwritten before being read?
+
+warning: unused variable: `z`
+  --> $DIR/liveness-upvars.rs:98:17
+   |
+LL |                 z = T::default();
+   |                 ^
+   |
+   = help: did you mean to capture by reference instead?
+
+warning: 17 warnings emitted
+
diff --git a/src/test/ui/unboxed-closures/unboxed-closures-counter-not-moved.rs b/src/test/ui/unboxed-closures/unboxed-closures-counter-not-moved.rs
index fb24df3c24e..390386e57fa 100644
--- a/src/test/ui/unboxed-closures/unboxed-closures-counter-not-moved.rs
+++ b/src/test/ui/unboxed-closures/unboxed-closures-counter-not-moved.rs
@@ -1,5 +1,4 @@
 // run-pass
-#![allow(unused_variables)]
 // Test that we mutate a counter on the stack only when we expect to.
 
 fn call<F>(f: F) where F : FnOnce() {
@@ -13,7 +12,7 @@ fn main() {
     call(|| {
         // Move `y`, but do not move `counter`, even though it is read
         // by value (note that it is also mutated).
-        for item in y {
+        for item in y { //~ WARN unused variable: `item`
             let v = counter;
             counter += v;
         }
@@ -22,7 +21,8 @@ fn main() {
 
     call(move || {
         // this mutates a moved copy, and hence doesn't affect original
-        counter += 1;
+        counter += 1; //~  WARN value assigned to `counter` is never read
+                      //~| WARN unused variable: `counter`
     });
     assert_eq!(counter, 88);
 }
diff --git a/src/test/ui/unboxed-closures/unboxed-closures-counter-not-moved.stderr b/src/test/ui/unboxed-closures/unboxed-closures-counter-not-moved.stderr
new file mode 100644
index 00000000000..ba4b3dac670
--- /dev/null
+++ b/src/test/ui/unboxed-closures/unboxed-closures-counter-not-moved.stderr
@@ -0,0 +1,27 @@
+warning: unused variable: `item`
+  --> $DIR/unboxed-closures-counter-not-moved.rs:15:13
+   |
+LL |         for item in y {
+   |             ^^^^ help: if this is intentional, prefix it with an underscore: `_item`
+   |
+   = note: `#[warn(unused_variables)]` on by default
+
+warning: value assigned to `counter` is never read
+  --> $DIR/unboxed-closures-counter-not-moved.rs:24:9
+   |
+LL |         counter += 1;
+   |         ^^^^^^^
+   |
+   = note: `#[warn(unused_assignments)]` on by default
+   = help: maybe it is overwritten before being read?
+
+warning: unused variable: `counter`
+  --> $DIR/unboxed-closures-counter-not-moved.rs:24:9
+   |
+LL |         counter += 1;
+   |         ^^^^^^^
+   |
+   = help: did you mean to capture by reference instead?
+
+warning: 3 warnings emitted
+
diff --git a/src/test/ui/unboxed-closures/unboxed-closures-move-mutable.rs b/src/test/ui/unboxed-closures/unboxed-closures-move-mutable.rs
index 9b519e63a95..e5b19db7822 100644
--- a/src/test/ui/unboxed-closures/unboxed-closures-move-mutable.rs
+++ b/src/test/ui/unboxed-closures/unboxed-closures-move-mutable.rs
@@ -13,11 +13,11 @@ fn set(x: &mut usize) { *x = 42; }
 fn main() {
     {
         let mut x = 0_usize;
-        move || x += 1;
+        move || x += 1; //~ WARN unused variable: `x`
     }
     {
         let mut x = 0_usize;
-        move || x += 1;
+        move || x += 1; //~ WARN unused variable: `x`
     }
     {
         let mut x = 0_usize;
diff --git a/src/test/ui/unboxed-closures/unboxed-closures-move-mutable.stderr b/src/test/ui/unboxed-closures/unboxed-closures-move-mutable.stderr
new file mode 100644
index 00000000000..4dfd1bb3075
--- /dev/null
+++ b/src/test/ui/unboxed-closures/unboxed-closures-move-mutable.stderr
@@ -0,0 +1,19 @@
+warning: unused variable: `x`
+  --> $DIR/unboxed-closures-move-mutable.rs:16:17
+   |
+LL |         move || x += 1;
+   |                 ^
+   |
+   = note: `#[warn(unused_variables)]` on by default
+   = help: did you mean to capture by reference instead?
+
+warning: unused variable: `x`
+  --> $DIR/unboxed-closures-move-mutable.rs:20:17
+   |
+LL |         move || x += 1;
+   |                 ^
+   |
+   = help: did you mean to capture by reference instead?
+
+warning: 2 warnings emitted
+