Skip to content

Commit

Permalink
Auto merge of #11320 - max-niederman:redundant_locals_shadow_mutated,…
Browse files Browse the repository at this point in the history
… r=Alexendoo

redundant_locals: fix FPs on mutated shadows

Fixes #11290.

When a mutable binding is shadowed by
a mutable binding of the same name in a different scope, mutations in that scope have different meaning.
This PR fixes spurious `redundant_locals` emissions on such locals.

cc `@Centri3,` `@flip1995`

changelog: [`redundant_locals`]: fix false positives on mutated shadows
  • Loading branch information
bors committed Aug 11, 2023
2 parents 8703661 + a5f62bd commit 1e8fdf4
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 11 deletions.
11 changes: 11 additions & 0 deletions clippy_lints/src/redundant_locals.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::is_from_proc_macro;
use clippy_utils::ty::needs_ordered_drop;
use rustc_ast::Mutability;
use rustc_hir::def::Res;
use rustc_hir::{BindingAnnotation, ByRef, Expr, ExprKind, HirId, Local, Node, Pat, PatKind, QPath};
use rustc_lint::{LateContext, LateLintPass, LintContext};
Expand Down Expand Up @@ -62,6 +63,8 @@ impl<'tcx> LateLintPass<'tcx> for RedundantLocals {
if let Node::Pat(binding_pat) = cx.tcx.hir().get(binding_id);
// the previous binding has the same mutability
if find_binding(binding_pat, ident).unwrap().1 == mutability;
// the local does not change the effect of assignments to the binding. see #11290
if !affects_assignments(cx, mutability, binding_id, local.hir_id);
// the local does not affect the code's drop behavior
if !affects_drop_behavior(cx, binding_id, local.hir_id, expr);
// the local is user-controlled
Expand Down Expand Up @@ -94,6 +97,14 @@ fn find_binding(pat: &Pat<'_>, name: Ident) -> Option<BindingAnnotation> {
ret
}

/// Check if a rebinding of a local changes the effect of assignments to the binding.
fn affects_assignments(cx: &LateContext<'_>, mutability: Mutability, bind: HirId, rebind: HirId) -> bool {
let hir = cx.tcx.hir();

// the binding is mutable and the rebinding is in a different scope than the original binding
mutability == Mutability::Mut && hir.get_enclosing_scope(bind) != hir.get_enclosing_scope(rebind)
}

/// Check if a rebinding of a local affects the code's drop behavior.
fn affects_drop_behavior<'tcx>(cx: &LateContext<'tcx>, bind: HirId, rebind: HirId, rebind_expr: &Expr<'tcx>) -> bool {
let hir = cx.tcx.hir();
Expand Down
9 changes: 9 additions & 0 deletions tests/ui/redundant_locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ fn downgraded_mutability() {
let x = x;
}

// see #11290
fn shadow_mutation() {
let mut x = 1;
{
let mut x = x;
x = 2;
}
}

fn coercion(par: &mut i32) {
let par: &i32 = par;

Expand Down
22 changes: 11 additions & 11 deletions tests/ui/redundant_locals.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ LL | let mut x = x;
= help: remove the redefinition of `x`

error: redundant redefinition of a binding
--> $DIR/redundant_locals.rs:37:14
--> $DIR/redundant_locals.rs:46:14
|
LL | fn parameter(x: i32) {
| ^
Expand All @@ -30,7 +30,7 @@ LL | let x = x;
= help: remove the redefinition of `x`

error: redundant redefinition of a binding
--> $DIR/redundant_locals.rs:42:9
--> $DIR/redundant_locals.rs:51:9
|
LL | let x = 1;
| ^
Expand All @@ -40,7 +40,7 @@ LL | let x = x;
= help: remove the redefinition of `x`

error: redundant redefinition of a binding
--> $DIR/redundant_locals.rs:43:9
--> $DIR/redundant_locals.rs:52:9
|
LL | let x = x;
| ^
Expand All @@ -50,7 +50,7 @@ LL | let x = x;
= help: remove the redefinition of `x`

error: redundant redefinition of a binding
--> $DIR/redundant_locals.rs:44:9
--> $DIR/redundant_locals.rs:53:9
|
LL | let x = x;
| ^
Expand All @@ -60,7 +60,7 @@ LL | let x = x;
= help: remove the redefinition of `x`

error: redundant redefinition of a binding
--> $DIR/redundant_locals.rs:45:9
--> $DIR/redundant_locals.rs:54:9
|
LL | let x = x;
| ^
Expand All @@ -70,7 +70,7 @@ LL | let x = x;
= help: remove the redefinition of `x`

error: redundant redefinition of a binding
--> $DIR/redundant_locals.rs:50:9
--> $DIR/redundant_locals.rs:59:9
|
LL | let a = 1;
| ^
Expand All @@ -81,7 +81,7 @@ LL | let a = a;
= help: remove the redefinition of `a`

error: redundant redefinition of a binding
--> $DIR/redundant_locals.rs:51:9
--> $DIR/redundant_locals.rs:60:9
|
LL | let b = 2;
| ^
Expand All @@ -92,7 +92,7 @@ LL | let b = b;
= help: remove the redefinition of `b`

error: redundant redefinition of a binding
--> $DIR/redundant_locals.rs:58:13
--> $DIR/redundant_locals.rs:67:13
|
LL | let x = 1;
| ^
Expand All @@ -102,7 +102,7 @@ LL | let x = x;
= help: remove the redefinition of `x`

error: redundant redefinition of a binding
--> $DIR/redundant_locals.rs:65:13
--> $DIR/redundant_locals.rs:74:13
|
LL | let x = 1;
| ^
Expand All @@ -112,7 +112,7 @@ LL | let x = x;
= help: remove the redefinition of `x`

error: redundant redefinition of a binding
--> $DIR/redundant_locals.rs:68:6
--> $DIR/redundant_locals.rs:77:6
|
LL | |x: i32| {
| ^
Expand All @@ -122,7 +122,7 @@ LL | let x = x;
= help: remove the redefinition of `x`

error: redundant redefinition of a binding
--> $DIR/redundant_locals.rs:85:9
--> $DIR/redundant_locals.rs:94:9
|
LL | let x = 1;
| ^
Expand Down

0 comments on commit 1e8fdf4

Please sign in to comment.