Spurious "redundant redefinition of a binding" error from redundant_locals

[emarsden]

Summary

Nightly clippy is showing what I believe to be a spurious "redundant redefinition of a binding" error for lexically scoped mutable bindings.

Lint Name

redundant_locals

Reproducer

I tried this code:

fn do_shadow_let() {
    let bazzle = 42;
    {
        let bazzle = 69;
        println!("shadow_let: internal bazzle: {bazzle}");
    }
    println!("shadow_let: external bazzle: {bazzle}");
}

fn do_shadow_letmut() {
    let mut bazzle = 0;
    bazzle += 42;
    {
        let mut bazzle = bazzle;
        bazzle = 69;
        println!("shadow_letmut: internal bazzle: {bazzle}");
    }
    println!("shadow_letmut: external bazzle: {bazzle}");
}

fn main () {
    do_shadow_let();
    do_shadow_letmut();
}

I saw this happen:

error: redundant redefinition of a binding
  --> src/main.rs:14:9
   |
14 |     let mut bazzle = 0;
   |         ^^^^^^^^^^
...
17 |         let mut bazzle = bazzle;
   |         ^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: remove the redefinition of `bazzle`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_locals
   = note: `#[deny(clippy::redundant_locals)]` on by default

I expected to see no error for the shadowed mutable binding case, as for the shadowed immutable binding.

Version

rustc 1.73.0-nightly (8131b9774 2023-08-02)
binary: rustc
commit-hash: 8131b9774ebcb6c162fcac71545a13543ec369e7
commit-date: 2023-08-02
host: x86_64-unknown-linux-gnu
release: 1.73.0-nightly
LLVM version: 16.0.5

Additional Labels

No response

[Centri3]

We'll need to be more careful if they're in differing blocks, by making sure they're never mutated or explicitly used.

[max-niederman]

Well... this is embarassing.

I think the best way to handle this would be to just not emit redundant_locals on mutable bindings in different blocks,
since the case where the binding is needlessly mutable is already covered by rustc's unused_mut.

[Centri3]

That won't catch it if it's immutable and dropped, though. It'll still lint in those cases

[Centri3]

We could use rustc_hir_typeck's ExprUseVisitor and not lint if it's consumed or mutated (or a mutable borrow), if it's Copy

For Copy types, a redundant redefinition isn't usually redundant if they're in differing blocks so in those cases we need to be more careful

[max-niederman]

That won't catch it if it's immutable and dropped, though. It'll still lint in those cases

fn main() {
    let copy = 0;
    let not_copy = Box::new(0);
    {
        let copy = copy;
        let not_copy = not_copy;
        drop(copy);
        drop(not_copy);
    }
}

Isn't this still always redundant?

[Centri3]

Hmm... I actually think so. For copy it'll be, of course, copied but not_copy can't be used afterwards anyway. So that should be ok then.

(Also, you can't drop a Copy type. My bad 😅)