Redundant_closure false positive with moved FnMut

[glittershark]

Lint name: redundant_closure

I tried this code (reduced to a minimal example):

fn fp(x: Vec<i32>, y: Vec<i32>) -> Vec<i32> {
    let mut res = Vec::new();
    let mut add_to_res = |n| res.push(n);
    x.into_iter().for_each(|x| add_to_res(x));
    y.into_iter().for_each(add_to_res);
    res
}

If I apply the suggestion (redundant_closure on the fourth line), resulting in the following code:

fn fp(x: Vec<i32>, y: Vec<i32>) -> Vec<i32> {
    let mut res = Vec::new();
    let mut add_to_res = |n| res.push(n);
    x.into_iter().for_each(add_to_res);
    y.into_iter().for_each(add_to_res);
    res
}

I get the following error:

error[E0382]: use of moved value: `add_to_res`
 --> src/lib.rs:7:28
  |
6 |     x.into_iter().for_each(add_to_res);
  |                            ---------- value moved here
7 |     y.into_iter().for_each(add_to_res);
  |                            ^^^^^^^^^^ value used here after move
  |
note: closure cannot be moved more than once as it is not `Copy` due to moving the variable `res` out of its environment
 --> src/lib.rs:5:30
  |
5 |     let mut add_to_res = |n| res.push(n);
  |                              ^^^

Meta

• cargo clippy -V: e.g. clippy 0.0.212 (98d6634 2020-11-14)
• rustc -Vv:

  rustc 1.50.0-nightly (98d66340d 2020-11-14)
  binary: rustc
  commmit-hash: 98d66340d6e63eda115afc8b0da1d87965881936
  commit-date: 2020-11-14
  host: x86_64-unknown-linux-gnu
  release: 1.50.0-nightly
  

[camsteffen]

It should suggest for_each(&mut add_to_res).

[ebobrow]

I'd like to try this. Should the lint always suggest &mut for FnMut closures, or only if the closure is later reused? I believe that || some_mut_func() is identical to &mut some_mut_func, so there's no harm in adding &mut except that it's potentially unnecessary code.

[camsteffen]

Should the lint always suggest &mut for FnMut closures, or only if the closure is later reused?

Yes. clippy_utils::path_to_local and LocalUsedVisitor should come in handy.