rust-clippy icon indicating copy to clipboard operation
rust-clippy copied to clipboard

`manual_flatten` false positives with reference slices

Open awused opened this issue 3 years ago • 3 comments

Lint name: manual_flatten

I tried this code:

struct Test {
    a: Option<usize>,
    b: Option<usize>,
}

fn main() {
    let t = Test {
        a: Some(1),
        b: Some(2),
    };

    for x in [&t.a, &t.b] {
        if let Some(y) = x {
            println!("{}", y);     
        }
    }
} 

This also applies to mutable references.

Clippy suggests this, which is both incomplete as a refactor and does not compile. Try as I might I could not get into_iter().flatten() to compile.

for x in [&t.a, &t.b].into_iter().flatten() {
    if let Some(y) = x {
        println!("{}", y);
    }
}

Meta

I saw https://github.com/rust-lang/rust-clippy/issues/6893 which appears to be the same issue, but it was claimed to be fixed there and that fix should have landed in 1.53. I'm on 1.54 now and still seeing it with both mutable and immutable reference slices.

  • cargo clippy -V: clippy 0.1.54 (a178d03 2021-07-26)
  • rustc -Vv:
rustc 1.54.0 (a178d0322 2021-07-26)
binary: rustc
commit-hash: a178d0322ce20e33eac124758e837cbd80a6f633
commit-date: 2021-07-26
host: x86_64-unknown-linux-gnu
release: 1.54.0
LLVM version: 12.0.1

awused avatar Jul 29 '21 23:07 awused

In addition to the original issue, manual_flatten also seems to give false suggestion on

fn main() {
    for n in [&&Some(1)] {
        if let Some(x) = n {
            println!("{}", x);
        }
    }
}

Running clippy suggests this which does not compile:

for n in [&&Some(1)].into_iter().copied().flatten() {
    println!("{}", n);
}

Problem lies on not checking nested reference on the lint here.

Using flatten() on nested references is a pain, since we would call copied() a lot of times. Would this lint makes more sense if it exempts reference altogether rather than suggesting into_iter().copied().flatten()?

Rust Playground

dswij avatar Jul 30 '21 11:07 dswij

@rustbot claim

dswij avatar Aug 09 '21 11:08 dswij

I was going through some of my old allowed lints recently. Whatever was preventing manually edited code using flatten() from building and running with borrows seems fixed upstream somewhere between 1.54 and 1.62. The clippy suggested fix is still incomplete and doesn't build, but at least the finding itself is no longer wrong even if it requires manual intervention.

awused avatar Jul 21 '22 18:07 awused