Rollup merge of rust-lang#149072 - chenyukang:yukang-fix-unused-148960, r=davidtwco

Fix the issue of unused assignment from MIR liveness checking

Fixes rust-lang#148960
Fixes rust-lang#148418

r? ``@davidtwco``

cc ``@cjgillot``

My first try on MIR related code, so it may not be the best fix.

Author: matthiaskrgr

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<PlaceIndex>| {
                 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);
             }
         }
@@ -976,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();
@@ -1048,26 +1055,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;

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

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
+

tests/ui/lint/unused/unused-assign-148960.rs

@@ -0,0 +1,42 @@
+//@ 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
+    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);
+}
+
+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() {}

tests/ui/lint/unused/unused-assign-148960.stderr

@@ -0,0 +1,62 @@
+error: value assigned to `value` is never read
+  --> $DIR/unused-assign-148960.rs:6: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:12: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:13: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:24:17
+   |
+LL |     let mut p = Point { x: 1, y: 1 };
+   |                 ^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: maybe it is overwritten before being read?
+
+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
+

tests/ui/liveness/liveness-unused.rs

@@ -244,8 +244,8 @@ fn f10<T>(mut a: T, b: T) {
     //~^ ERROR value assigned to `a` is never read
 }
 
-fn f10b<T>(mut a: Box<T>, b: Box<T>) {
-    a = b;
+fn f10b<T>(mut a: Box<T>, b: Box<T>) { //~ 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

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<T>(mut a: Box<T>, b: Box<T>) {
+   |            ^^^^^
+   |
+   = 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