about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-07-17 22:51:07 +0000
committerbors <bors@rust-lang.org>2021-07-17 22:51:07 +0000
commiteb0b95b55a0b38d91e834dd30902b67627ed2eb0 (patch)
tree79654f38a443096137846f8e7f0a781b738ced16
parentc7331d65bdbab1187f5a9b8f5b918248678ebdb9 (diff)
parentdc639c944af8a47c5017317eebe2764087c8f776 (diff)
downloadrust-eb0b95b55a0b38d91e834dd30902b67627ed2eb0.tar.gz
rust-eb0b95b55a0b38d91e834dd30902b67627ed2eb0.zip
Auto merge of #87129 - FabianWolff:issue-75356, r=varkor
Warn about useless assignments of variables/fields to themselves

This PR fixes #75356. Following `@varkor's` suggestion in https://github.com/rust-lang/rust/issues/75356#issuecomment-700339154, I have implemented this warning as part of the `dead_code` lint. Unlike the `-Wself-assign` implementation in [Clang](https://github.com/llvm/llvm-project/blob/56e6d4742e6909bd7d2db201cc5e0e3e77c6f282/clang/lib/Sema/SemaExpr.cpp#L13875-L13909), my implementation also warns about self-assignments of struct fields (`s.x = s.x`).

r? `@varkor`
-rw-r--r--compiler/rustc_passes/src/dead.rs54
-rw-r--r--src/test/ui/issues/issue-3290.rs1
-rw-r--r--src/test/ui/lint/dead-code/self-assign.rs50
-rw-r--r--src/test/ui/lint/dead-code/self-assign.stderr44
-rw-r--r--src/test/ui/nll/issue-50461-used-mut-from-moves.rs1
-rw-r--r--src/test/ui/self/self-re-assign.rs1
6 files changed, 151 insertions, 0 deletions
diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs
index bef56bbc287..713d572b93a 100644
--- a/compiler/rustc_passes/src/dead.rs
+++ b/compiler/rustc_passes/src/dead.rs
@@ -150,6 +150,59 @@ impl<'tcx> MarkSymbolVisitor<'tcx> {
         }
     }
 
+    fn check_for_self_assign(&mut self, assign: &'tcx hir::Expr<'tcx>) {
+        fn check_for_self_assign_helper(
+            tcx: TyCtxt<'tcx>,
+            typeck_results: &'tcx ty::TypeckResults<'tcx>,
+            lhs: &'tcx hir::Expr<'tcx>,
+            rhs: &'tcx hir::Expr<'tcx>,
+        ) -> bool {
+            match (&lhs.kind, &rhs.kind) {
+                (hir::ExprKind::Path(ref qpath_l), hir::ExprKind::Path(ref qpath_r)) => {
+                    if let (Res::Local(id_l), Res::Local(id_r)) = (
+                        typeck_results.qpath_res(qpath_l, lhs.hir_id),
+                        typeck_results.qpath_res(qpath_r, rhs.hir_id),
+                    ) {
+                        if id_l == id_r {
+                            return true;
+                        }
+                    }
+                    return false;
+                }
+                (hir::ExprKind::Field(lhs_l, ident_l), hir::ExprKind::Field(lhs_r, ident_r)) => {
+                    if ident_l == ident_r {
+                        return check_for_self_assign_helper(tcx, typeck_results, lhs_l, lhs_r);
+                    }
+                    return false;
+                }
+                _ => {
+                    return false;
+                }
+            }
+        }
+
+        if let hir::ExprKind::Assign(lhs, rhs, _) = assign.kind {
+            if check_for_self_assign_helper(self.tcx, self.typeck_results(), lhs, rhs)
+                && !assign.span.from_expansion()
+            {
+                let is_field_assign = matches!(lhs.kind, hir::ExprKind::Field(..));
+                self.tcx.struct_span_lint_hir(
+                    lint::builtin::DEAD_CODE,
+                    assign.hir_id,
+                    assign.span,
+                    |lint| {
+                        lint.build(&format!(
+                            "useless assignment of {} of type `{}` to itself",
+                            if is_field_assign { "field" } else { "variable" },
+                            self.typeck_results().expr_ty(lhs),
+                        ))
+                        .emit();
+                    },
+                )
+            }
+        }
+    }
+
     fn handle_field_pattern_match(
         &mut self,
         lhs: &hir::Pat<'_>,
@@ -287,6 +340,7 @@ impl<'tcx> Visitor<'tcx> for MarkSymbolVisitor<'tcx> {
             }
             hir::ExprKind::Assign(ref left, ref right, ..) => {
                 self.handle_assign(left);
+                self.check_for_self_assign(expr);
                 self.visit_expr(right);
                 return;
             }
diff --git a/src/test/ui/issues/issue-3290.rs b/src/test/ui/issues/issue-3290.rs
index 63e1b28a60a..840c7ff8355 100644
--- a/src/test/ui/issues/issue-3290.rs
+++ b/src/test/ui/issues/issue-3290.rs
@@ -1,5 +1,6 @@
 // run-pass
 #![feature(box_syntax)]
+#![allow(dead_code)]
 
 pub fn main() {
    let mut x: Box<_> = box 3;
diff --git a/src/test/ui/lint/dead-code/self-assign.rs b/src/test/ui/lint/dead-code/self-assign.rs
new file mode 100644
index 00000000000..b8bf7d860c4
--- /dev/null
+++ b/src/test/ui/lint/dead-code/self-assign.rs
@@ -0,0 +1,50 @@
+// Test that dead code warnings are issued for superfluous assignments of
+// fields or variables to themselves (issue #75356).
+
+// check-pass
+#![allow(unused_assignments)]
+#![warn(dead_code)]
+
+fn main() {
+    let mut x = 0;
+    x = x;
+    //~^ WARNING: useless assignment of variable of type `i32` to itself
+
+    x = (x);
+    //~^ WARNING: useless assignment of variable of type `i32` to itself
+
+    x = {x};
+    // block expressions don't count as self-assignments
+
+
+    struct S<'a> { f: &'a str }
+    let mut s = S { f: "abc" };
+    s = s;
+    //~^ WARNING: useless assignment of variable of type `S` to itself
+
+    s.f = s.f;
+    //~^ WARNING: useless assignment of field of type `&str` to itself
+
+
+    struct N0 { x: Box<i32> }
+    struct N1 { n: N0 }
+    struct N2(N1);
+    struct N3 { n: N2 };
+    let mut n3 = N3 { n: N2(N1 { n: N0 { x: Box::new(42) } }) };
+    n3.n.0.n.x = n3.n.0.n.x;
+    //~^ WARNING: useless assignment of field of type `Box<i32>` to itself
+
+    let mut t = (1, ((2, 3, (4, 5)),));
+    t.1.0.2.1 = t.1.0.2.1;
+    //~^ WARNING: useless assignment of field of type `i32` to itself
+
+
+    let mut y = 0;
+    macro_rules! assign_to_y {
+        ($cur:expr) => {{
+            y = $cur;
+        }};
+    }
+    assign_to_y!(y);
+    // self-assignments in macro expansions are not reported either
+}
diff --git a/src/test/ui/lint/dead-code/self-assign.stderr b/src/test/ui/lint/dead-code/self-assign.stderr
new file mode 100644
index 00000000000..bb79c0ec72a
--- /dev/null
+++ b/src/test/ui/lint/dead-code/self-assign.stderr
@@ -0,0 +1,44 @@
+warning: useless assignment of variable of type `i32` to itself
+  --> $DIR/self-assign.rs:10:5
+   |
+LL |     x = x;
+   |     ^^^^^
+   |
+note: the lint level is defined here
+  --> $DIR/self-assign.rs:6:9
+   |
+LL | #![warn(dead_code)]
+   |         ^^^^^^^^^
+
+warning: useless assignment of variable of type `i32` to itself
+  --> $DIR/self-assign.rs:13:5
+   |
+LL |     x = (x);
+   |     ^^^^^^^
+
+warning: useless assignment of variable of type `S` to itself
+  --> $DIR/self-assign.rs:22:5
+   |
+LL |     s = s;
+   |     ^^^^^
+
+warning: useless assignment of field of type `&str` to itself
+  --> $DIR/self-assign.rs:25:5
+   |
+LL |     s.f = s.f;
+   |     ^^^^^^^^^
+
+warning: useless assignment of field of type `Box<i32>` to itself
+  --> $DIR/self-assign.rs:34:5
+   |
+LL |     n3.n.0.n.x = n3.n.0.n.x;
+   |     ^^^^^^^^^^^^^^^^^^^^^^^
+
+warning: useless assignment of field of type `i32` to itself
+  --> $DIR/self-assign.rs:38:5
+   |
+LL |     t.1.0.2.1 = t.1.0.2.1;
+   |     ^^^^^^^^^^^^^^^^^^^^^
+
+warning: 6 warnings emitted
+
diff --git a/src/test/ui/nll/issue-50461-used-mut-from-moves.rs b/src/test/ui/nll/issue-50461-used-mut-from-moves.rs
index 69d7cdd83a6..2458b171e64 100644
--- a/src/test/ui/nll/issue-50461-used-mut-from-moves.rs
+++ b/src/test/ui/nll/issue-50461-used-mut-from-moves.rs
@@ -1,6 +1,7 @@
 // run-pass
 
 #![deny(unused_mut)]
+#![allow(dead_code)]
 
 struct Foo {
     pub value: i32
diff --git a/src/test/ui/self/self-re-assign.rs b/src/test/ui/self/self-re-assign.rs
index a7b089ebff4..88e86146832 100644
--- a/src/test/ui/self/self-re-assign.rs
+++ b/src/test/ui/self/self-re-assign.rs
@@ -3,6 +3,7 @@
 // that we do not glue_drop before we glue_take (#3290).
 
 #![feature(box_syntax)]
+#![allow(dead_code)]
 
 use std::rc::Rc;