Rollup merge of #72465 - tmiasko:liveness-upvars, r=nikomatsakis

Warn about unused captured variables

Include captured variables in liveness analysis. Warn when captured variables
are unused (but possibly read or written to). Warn about dead assignments to
captured variables.

Fixes #37707.
Fixes #47128.
Fixes #63220.

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
 }

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
 }

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
 

src/test/ui/issues/issue-11958.rs

@@ -1,10 +1,11 @@
 // run-pass
-#![forbid(warnings)]
 
 // We shouldn't need to rebind a moved upvar as mut if it's already
 // marked as mut
 
 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`
 }

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
+

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() {}

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
+

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);
 }

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
+

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;