False positive for redundant_closure lint

[samestep]

I wrote a simple thunk implementation:

struct Thunk<T>(Box<FnMut() -> T>);

impl<T> Thunk<T> {
    fn new<F: 'static + FnOnce() -> T>(f: F) -> Thunk<T> {
        let mut option = Some(f);
        Thunk(Box::new(move || option.take().unwrap()()))
    }

    fn unwrap(self) -> T {
        let Thunk(mut f) = self;
        f()
    }
}

fn main() {
    let thunk = Thunk::new(|| println!("Hello, world!"));
    thunk.unwrap()
}

However, clippy complains that the closure in Thunk::new is redundant:

warning: redundant closure found, #[warn(redundant_closure)] on by default
 --> src/main.rs:6:24
  |
6 |         Thunk(Box::new(move || option.take().unwrap()()))
  |                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
help: remove closure as shown:
  |         Thunk(Box::new(option.take().unwrap()))
  = help: for further information visit https://github.com/Manishearth/rust-clippy/wiki#redundant_closure

But if I use clippy's suggestion, I get this error:

error[E0277]: the trait bound `F: std::ops::FnMut<()>` is not satisfied
 --> src/main.rs:6:15
  |
6 |         Thunk(Box::new(option.take().unwrap()))
  |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::ops::FnMut<()>` is not implemented for `F`
  |
  = help: consider adding a `where F: std::ops::FnMut<()>` bound
  = note: required for the cast to the object type `std::ops::FnMut() -> T + 'static`

This seems like a bug in the redundant_closure lint.

[oli-obk]

Ugh... Box<FnOnce> ... We should just bail when encountering it...

[mtak-]

This is also triggering on my code which does not have a box, but is similarly lazy. Here is a minimal repro for v0.0.191.

struct Wrapper<T>(Option<T>);

impl<T> Wrapper<T> {
    fn map_it<U, V>(&mut self, other: Wrapper<U>) -> Wrapper<V>
    where
        T: FnOnce(U) -> V,
    {
        Wrapper(other.0.map(|u| self.0.take().expect("")(u)))
    }
}

warning: redundant closure found
  --> src/private/map.rs:44:29
   |
44 |         Wrapper(other.0.map(|u| self.0.take().expect("")(u)))
   |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove closure as shown: `self.0.take().expect("")`
   |
   = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.191/index.html#redundant_closure

EDIT: Too minimal, i think clippy is correct here.

[phansch]

Going to take a look at this over the next couple of days

[Nemo157]

Here is a pretty minimal repro:

#[allow(unknown_lints, blacklisted_name)]
fn main() {
    let mut foo = Some(|x| x * x);
    let bar = [Ok(1), Err(2)];
    let baz: Vec<_> = bar.iter().map(|x| x.map_err(|e| foo.take().unwrap()(e))).collect();
    // let baz: Vec<_> = bar.iter().map(|x| x.map_err(foo.take().unwrap())).collect();
    println!("{:?}", baz);
}

[phansch]

I didn't have enough time to dig into this, unfortunately. If someone wants to pick this up, feel free!