nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2024-11-26 08:44:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

liveness: Warn about unused captured variables

Browse Source
This commit is contained in:
Tomasz Miąsko 2020-05-22 00:00:00 +00:00
parent 74fcbfb491
commit 4dc56614b2
12 changed files with 505 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

224
src/librustc_passes/liveness.rs
View File

@ -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();
},
)
}
}
}

2
src/test/ui/closures/closure-immutable-outer-variable.fixed
View File

@ -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
}

2
src/test/ui/closures/closure-immutable-outer-variable.rs
View File

@ -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
}

4
src/test/ui/closures/closure-immutable-outer-variable.stderr
View File

@ -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

3
src/test/ui/issues/issue-11958.rs
View File

@ -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`
}

20
src/test/ui/issues/issue-11958.stderr Normal file
View File

@ -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

108
src/test/ui/liveness/liveness-upvars.rs Normal file
View File

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

150
src/test/ui/liveness/liveness-upvars.stderr Normal file
View File

@ -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

6
src/test/ui/unboxed-closures/unboxed-closures-counter-not-moved.rs
View File

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

27
src/test/ui/unboxed-closures/unboxed-closures-counter-not-moved.stderr Normal file
View File

@ -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

4
src/test/ui/unboxed-closures/unboxed-closures-move-mutable.rs
View File

@ -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;

19
src/test/ui/unboxed-closures/unboxed-closures-move-mutable.stderr Normal file
View File

@ -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