Skip to content

Commit

Permalink
Auto merge of #87129 - FabianWolff:issue-75356, r=varkor
Browse files Browse the repository at this point in the history
Warn about useless assignments of variables/fields to themselves

This PR fixes #75356. Following `@varkor's` suggestion in #75356 (comment), 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`
  • Loading branch information
bors committed Jul 17, 2021
2 parents c7331d6 + dc639c9 commit eb0b95b
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 0 deletions.
54 changes: 54 additions & 0 deletions compiler/rustc_passes/src/dead.rs
Expand Up @@ -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<'_>,
Expand Down Expand Up @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions 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;
Expand Down
50 changes: 50 additions & 0 deletions 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
}
44 changes: 44 additions & 0 deletions 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

1 change: 1 addition & 0 deletions 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
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/self/self-re-assign.rs
Expand Up @@ -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;

Expand Down

0 comments on commit eb0b95b

Please sign in to comment.