From 4930d3e6129b17538e687f97c3b3f3e0dcaea76f Mon Sep 17 00:00:00 2001 From: yukang Date: Wed, 19 Nov 2025 08:16:11 +0800 Subject: [PATCH 1/2] Fix the issue of unused assignment from MIR liveness checking --- compiler/rustc_mir_transform/src/liveness.rs | 25 +++++++----- tests/ui/lint/unused/unused-assign-148960.rs | 33 +++++++++++++++ .../lint/unused/unused-assign-148960.stderr | 40 +++++++++++++++++++ 3 files changed, 89 insertions(+), 9 deletions(-) create mode 100644 tests/ui/lint/unused/unused-assign-148960.rs create mode 100644 tests/ui/lint/unused/unused-assign-148960.stderr diff --git a/compiler/rustc_mir_transform/src/liveness.rs b/compiler/rustc_mir_transform/src/liveness.rs index f7dc703560245..d0134689fec06 100644 --- a/compiler/rustc_mir_transform/src/liveness.rs +++ b/compiler/rustc_mir_transform/src/liveness.rs @@ -45,6 +45,9 @@ struct Access { /// When we encounter multiple statements at the same location, we only increase the liveness, /// in order to avoid false positives. live: bool, + /// Is this a direct access to the place itself, no projections, or to a field? + /// This helps distinguish `x = ...` from `x.field = ...` + is_direct: bool, } #[tracing::instrument(level = "debug", skip(tcx), ret)] @@ -650,15 +653,17 @@ impl<'a, 'tcx> AssignmentResult<'a, 'tcx> { |place: Place<'tcx>, kind, source_info: SourceInfo, live: &DenseBitSet| { if let Some((index, extra_projections)) = checked_places.get(place.as_ref()) { if !is_indirect(extra_projections) { + let is_direct = extra_projections.is_empty(); match assignments[index].entry(source_info) { IndexEntry::Vacant(v) => { - let access = Access { kind, live: live.contains(index) }; + let access = Access { kind, live: live.contains(index), is_direct }; v.insert(access); } IndexEntry::Occupied(mut o) => { // There were already a sighting. Mark this statement as live if it // was, to avoid false positives. o.get_mut().live |= live.contains(index); + o.get_mut().is_direct &= is_direct; } } } @@ -742,7 +747,7 @@ impl<'a, 'tcx> AssignmentResult<'a, 'tcx> { continue; }; let source_info = body.local_decls[place.local].source_info; - let access = Access { kind, live: live.contains(index) }; + let access = Access { kind, live: live.contains(index), is_direct: true }; assignments[index].insert(source_info, access); } } @@ -1048,26 +1053,28 @@ impl<'a, 'tcx> AssignmentResult<'a, 'tcx> { let Some((name, decl_span)) = self.checked_places.names[index] else { continue }; - // We have outstanding assignments and with non-trivial drop. - // This is probably a drop-guard, so we do not issue a warning there. - if maybe_drop_guard( + let is_maybe_drop_guard = maybe_drop_guard( tcx, self.typing_env, index, &self.ever_dropped, self.checked_places, self.body, - ) { - continue; - } + ); // We probed MIR in reverse order for dataflow. // We revert the vector to give a consistent order to the user. - for (source_info, Access { live, kind }) in statements.into_iter().rev() { + for (source_info, Access { live, kind, is_direct }) in statements.into_iter().rev() { if live { continue; } + // If this place was dropped and has non-trivial drop, + // skip reporting field assignments. + if !is_direct && is_maybe_drop_guard { + continue; + } + // Report the dead assignment. let Some(hir_id) = source_info.scope.lint_root(&self.body.source_scopes) else { continue; diff --git a/tests/ui/lint/unused/unused-assign-148960.rs b/tests/ui/lint/unused/unused-assign-148960.rs new file mode 100644 index 0000000000000..5d1810ad04a68 --- /dev/null +++ b/tests/ui/lint/unused/unused-assign-148960.rs @@ -0,0 +1,33 @@ +//@ check-fail +#![deny(unused)] + +fn test_one_extra_assign() { + let mut value = b"0".to_vec(); //~ ERROR value assigned to `value` is never read + value = b"1".to_vec(); + println!("{:?}", value); +} + +fn test_two_extra_assign() { + let mut x = 1; //~ ERROR value assigned to `x` is never read + x = 2; //~ ERROR value assigned to `x` is never read + x = 3; + println!("{}", x); +} + +struct Point { + x: i32, + y: i32, +} + +fn test_indirect_assign() { + let mut p = Point { x: 1, y: 1 }; //~ ERROR value assigned to `p` is never read + p = Point { x: 2, y: 2 }; + p.x = 3; + println!("{}", p.y); +} + +fn main() { + test_one_extra_assign(); + test_two_extra_assign(); + test_indirect_assign(); +} diff --git a/tests/ui/lint/unused/unused-assign-148960.stderr b/tests/ui/lint/unused/unused-assign-148960.stderr new file mode 100644 index 0000000000000..3f135264d8690 --- /dev/null +++ b/tests/ui/lint/unused/unused-assign-148960.stderr @@ -0,0 +1,40 @@ +error: value assigned to `value` is never read + --> $DIR/unused-assign-148960.rs:5:21 + | +LL | let mut value = b"0".to_vec(); + | ^^^^^^^^^^^^^ + | + = help: maybe it is overwritten before being read? +note: the lint level is defined here + --> $DIR/unused-assign-148960.rs:2:9 + | +LL | #![deny(unused)] + | ^^^^^^ + = note: `#[deny(unused_assignments)]` implied by `#[deny(unused)]` + +error: value assigned to `x` is never read + --> $DIR/unused-assign-148960.rs:11:17 + | +LL | let mut x = 1; + | ^ + | + = help: maybe it is overwritten before being read? + +error: value assigned to `x` is never read + --> $DIR/unused-assign-148960.rs:12:5 + | +LL | x = 2; + | ^^^^^ + | + = help: maybe it is overwritten before being read? + +error: value assigned to `p` is never read + --> $DIR/unused-assign-148960.rs:23:17 + | +LL | let mut p = Point { x: 1, y: 1 }; + | ^^^^^^^^^^^^^^^^^^^^ + | + = help: maybe it is overwritten before being read? + +error: aborting due to 4 previous errors + From 00f3155794f40030249a36b4abb09906b6e8fa5d Mon Sep 17 00:00:00 2001 From: yukang Date: Thu, 20 Nov 2025 19:47:21 +0800 Subject: [PATCH 2/2] fix unused assigment issue for variable with drop, issue 148418 --- compiler/rustc_mir_transform/src/liveness.rs | 6 ++- tests/ui/drop/or-pattern-drop-order.rs | 22 ++++++---- tests/ui/drop/or-pattern-drop-order.stderr | 43 +++++++++++++++++++ tests/ui/lint/unused/unused-assign-148960.rs | 17 ++++++-- .../lint/unused/unused-assign-148960.stderr | 32 +++++++++++--- tests/ui/liveness/liveness-unused.rs | 4 +- tests/ui/liveness/liveness-unused.stderr | 18 +++++++- 7 files changed, 120 insertions(+), 22 deletions(-) create mode 100644 tests/ui/drop/or-pattern-drop-order.stderr diff --git a/compiler/rustc_mir_transform/src/liveness.rs b/compiler/rustc_mir_transform/src/liveness.rs index d0134689fec06..e66fb9d2d2a35 100644 --- a/compiler/rustc_mir_transform/src/liveness.rs +++ b/compiler/rustc_mir_transform/src/liveness.rs @@ -981,8 +981,10 @@ impl<'a, 'tcx> AssignmentResult<'a, 'tcx> { self.checked_places, self.body, ) { - statements.clear(); - continue; + statements.retain(|_, access| access.is_direct); + if statements.is_empty() { + continue; + } } let typo = maybe_suggest_typo(); diff --git a/tests/ui/drop/or-pattern-drop-order.rs b/tests/ui/drop/or-pattern-drop-order.rs index 88355a4937e4f..c7913be032eab 100644 --- a/tests/ui/drop/or-pattern-drop-order.rs +++ b/tests/ui/drop/or-pattern-drop-order.rs @@ -33,15 +33,15 @@ fn main() { // Drops are right-to-left: `z`, `y`, `x`. let (x, Ok(y) | Err(y), z); // Assignment order doesn't matter. - z = LogDrop(o, 1); - y = LogDrop(o, 2); - x = LogDrop(o, 3); + z = LogDrop(o, 1); //~ WARN value assigned to `z` is never read + y = LogDrop(o, 2); //~ WARN value assigned to `y` is never read + x = LogDrop(o, 3); //~ WARN value assigned to `x` is never read }); assert_drop_order(1..=2, |o| { // The first or-pattern alternative determines the bindings' drop order: `y`, `x`. let ((true, x, y) | (false, y, x)); - x = LogDrop(o, 2); - y = LogDrop(o, 1); + x = LogDrop(o, 2); //~ WARN value assigned to `x` is never read + y = LogDrop(o, 1); //~ WARN value assigned to `y` is never read }); // `let pat = expr;` should have the same drop order. @@ -61,15 +61,21 @@ fn main() { // `match` should have the same drop order. assert_drop_order(1..=3, |o| { // Drops are right-to-left: `z`, `y` `x`. - match (LogDrop(o, 3), Ok(LogDrop(o, 2)), LogDrop(o, 1)) { (x, Ok(y) | Err(y), z) => {} } + match (LogDrop(o, 3), Ok(LogDrop(o, 2)), LogDrop(o, 1)) { + (x, Ok(y) | Err(y), z) => {} + } }); assert_drop_order(1..=2, |o| { // The first or-pattern alternative determines the bindings' drop order: `y`, `x`. - match (true, LogDrop(o, 2), LogDrop(o, 1)) { (true, x, y) | (false, y, x) => {} } + match (true, LogDrop(o, 2), LogDrop(o, 1)) { + (true, x, y) | (false, y, x) => {} + } }); assert_drop_order(1..=2, |o| { // That drop order is used regardless of which or-pattern alternative matches: `y`, `x`. - match (false, LogDrop(o, 1), LogDrop(o, 2)) { (true, x, y) | (false, y, x) => {} } + match (false, LogDrop(o, 1), LogDrop(o, 2)) { + (true, x, y) | (false, y, x) => {} + } }); // Function params are visited one-by-one, and the order of bindings within a param's pattern is diff --git a/tests/ui/drop/or-pattern-drop-order.stderr b/tests/ui/drop/or-pattern-drop-order.stderr new file mode 100644 index 0000000000000..3d1aa69fb91a8 --- /dev/null +++ b/tests/ui/drop/or-pattern-drop-order.stderr @@ -0,0 +1,43 @@ +warning: value assigned to `x` is never read + --> $DIR/or-pattern-drop-order.rs:43:9 + | +LL | x = LogDrop(o, 2); + | ^ + | + = help: maybe it is overwritten before being read? + = note: `#[warn(unused_assignments)]` (part of `#[warn(unused)]`) on by default + +warning: value assigned to `y` is never read + --> $DIR/or-pattern-drop-order.rs:44:9 + | +LL | y = LogDrop(o, 1); + | ^ + | + = help: maybe it is overwritten before being read? + +warning: value assigned to `x` is never read + --> $DIR/or-pattern-drop-order.rs:38:9 + | +LL | x = LogDrop(o, 3); + | ^ + | + = help: maybe it is overwritten before being read? + +warning: value assigned to `y` is never read + --> $DIR/or-pattern-drop-order.rs:37:9 + | +LL | y = LogDrop(o, 2); + | ^ + | + = help: maybe it is overwritten before being read? + +warning: value assigned to `z` is never read + --> $DIR/or-pattern-drop-order.rs:36:9 + | +LL | z = LogDrop(o, 1); + | ^ + | + = help: maybe it is overwritten before being read? + +warning: 5 warnings emitted + diff --git a/tests/ui/lint/unused/unused-assign-148960.rs b/tests/ui/lint/unused/unused-assign-148960.rs index 5d1810ad04a68..2e51c01398a37 100644 --- a/tests/ui/lint/unused/unused-assign-148960.rs +++ b/tests/ui/lint/unused/unused-assign-148960.rs @@ -1,5 +1,6 @@ //@ check-fail #![deny(unused)] +#![allow(dead_code)] fn test_one_extra_assign() { let mut value = b"0".to_vec(); //~ ERROR value assigned to `value` is never read @@ -26,8 +27,16 @@ fn test_indirect_assign() { println!("{}", p.y); } -fn main() { - test_one_extra_assign(); - test_two_extra_assign(); - test_indirect_assign(); +struct Foo; + +impl Drop for Foo { + fn drop(&mut self) {} +} + +// testcase for issue #148418 +fn test_unused_variable() { + let mut foo = Foo; //~ ERROR variable `foo` is assigned to, but never used + foo = Foo; //~ ERROR value assigned to `foo` is never read } + +fn main() {} diff --git a/tests/ui/lint/unused/unused-assign-148960.stderr b/tests/ui/lint/unused/unused-assign-148960.stderr index 3f135264d8690..0e23fabd5f561 100644 --- a/tests/ui/lint/unused/unused-assign-148960.stderr +++ b/tests/ui/lint/unused/unused-assign-148960.stderr @@ -1,5 +1,5 @@ error: value assigned to `value` is never read - --> $DIR/unused-assign-148960.rs:5:21 + --> $DIR/unused-assign-148960.rs:6:21 | LL | let mut value = b"0".to_vec(); | ^^^^^^^^^^^^^ @@ -13,7 +13,7 @@ LL | #![deny(unused)] = note: `#[deny(unused_assignments)]` implied by `#[deny(unused)]` error: value assigned to `x` is never read - --> $DIR/unused-assign-148960.rs:11:17 + --> $DIR/unused-assign-148960.rs:12:17 | LL | let mut x = 1; | ^ @@ -21,7 +21,7 @@ LL | let mut x = 1; = help: maybe it is overwritten before being read? error: value assigned to `x` is never read - --> $DIR/unused-assign-148960.rs:12:5 + --> $DIR/unused-assign-148960.rs:13:5 | LL | x = 2; | ^^^^^ @@ -29,12 +29,34 @@ LL | x = 2; = help: maybe it is overwritten before being read? error: value assigned to `p` is never read - --> $DIR/unused-assign-148960.rs:23:17 + --> $DIR/unused-assign-148960.rs:24:17 | LL | let mut p = Point { x: 1, y: 1 }; | ^^^^^^^^^^^^^^^^^^^^ | = help: maybe it is overwritten before being read? -error: aborting due to 4 previous errors +error: variable `foo` is assigned to, but never used + --> $DIR/unused-assign-148960.rs:38:9 + | +LL | let mut foo = Foo; + | ^^^^^^^ + | + = note: consider using `_foo` instead + = note: `#[deny(unused_variables)]` implied by `#[deny(unused)]` +help: you might have meant to pattern match on the similarly named struct `Foo` + | +LL - let mut foo = Foo; +LL + let Foo = Foo; + | + +error: value assigned to `foo` is never read + --> $DIR/unused-assign-148960.rs:39:5 + | +LL | foo = Foo; + | ^^^ + | + = help: maybe it is overwritten before being read? + +error: aborting due to 6 previous errors diff --git a/tests/ui/liveness/liveness-unused.rs b/tests/ui/liveness/liveness-unused.rs index 639d7c2776dee..a291d2489695f 100644 --- a/tests/ui/liveness/liveness-unused.rs +++ b/tests/ui/liveness/liveness-unused.rs @@ -244,8 +244,8 @@ fn f10(mut a: T, b: T) { //~^ ERROR value assigned to `a` is never read } -fn f10b(mut a: Box, b: Box) { - a = b; +fn f10b(mut a: Box, b: Box) { //~ ERROR variable `a` is assigned to, but never used + a = b; //~ ERROR value assigned to `a` is never read } // unused params warnings are not needed for intrinsic functions without bodies diff --git a/tests/ui/liveness/liveness-unused.stderr b/tests/ui/liveness/liveness-unused.stderr index 23a26841be2fc..f56ca7823e165 100644 --- a/tests/ui/liveness/liveness-unused.stderr +++ b/tests/ui/liveness/liveness-unused.stderr @@ -283,5 +283,21 @@ LL | a = b; | = help: maybe it is overwritten before being read? -error: aborting due to 34 previous errors; 1 warning emitted +error: variable `a` is assigned to, but never used + --> $DIR/liveness-unused.rs:247:12 + | +LL | fn f10b(mut a: Box, b: Box) { + | ^^^^^ + | + = note: consider using `_a` instead + +error: value assigned to `a` is never read + --> $DIR/liveness-unused.rs:248:5 + | +LL | a = b; + | ^ + | + = help: maybe it is overwritten before being read? + +error: aborting due to 36 previous errors; 1 warning emitted