Closures should always implement the all applicable Fn* traits.

[Stebalien]

If a closure is passed into a function call as temporary, it only implements the Fn* traits needed to satisfy the constraints specified by the function. However, if it is declared first and then moved into the function, it correctly implements all applicable Fn* traits.

fn strip<F: FnOnce()>(f: F) -> F { f }
fn fail<F: Fn()>(f: F) { f() }

// Works.
fn test_works() {
    let f = || {};
    let f = strip(f);
    fail(f);
}

// Doesn't
fn test_broken() {
    // f only implements FnOnce().
    let f = strip(|| {});
    fail(f);
}

fn main() {}

[bluss]

This bug has an observable effect with specialization too. Fn(X) is accepted as a more specific impl than FnMut(X), but it will not actually be used in practice. cc @aturon

Edited: I pasted the wrong code in my first attempt.

// This prints "default apply"
// rustc 1.9.0-nightly (c66d2380a 2016-03-15)
#![feature(specialization)]

pub fn apply<F>(f: F)
    where F: FnMut()
{
    <()>::apply(f);
}

trait Apply<F> {
    fn apply(f: F);
}

impl<F> Apply<F> for ()
    where F: FnMut()
{
    default fn apply(f: F) {
        println!("default apply")
    }
}

impl<F> Apply<F> for ()
    where F: Fn()
{
    fn apply(f: F) {
        println!("thread safe apply")
    }
}


fn main() {
    apply(|| {});
}

[pnkfelix]

I don't know off-hand if this is a "mere compiler bug" or a deep language specification issue.

I'm going to throw it onto the compiler teams plate for the short term, trusting that the team can move it to lang if that's what makes sense.

[pnkfelix]

triage: I-nominated T-compiler

[nikomatsakis]

So this was intentional, but not necessarily wise. It helped us to get closures up and going, and I left it in as a useful "shorcut" to the right answer for closure inference. It is also the only way to influence closure trait selection, should you want to do so (but that last part is bad; we should probably have some explicit syntax at some point).

So my feeling is we should try commenting the "expected type inference" out and see what breaks. That is probably fairly easy to do. Hopefully, things will "just work" -- but there may be corner cases that will start to fail, I'm not sure.

[nikomatsakis]

triage: P-medium

[brson]

Could be P-low?

[steveklabnik]

Triage: not aware of any changes here

[varkor]

So my feeling is we should try commenting the "expected type inference" out and see what breaks. That is probably fairly easy to do.

This might make a good E-mentor or E-medium issue: as a first step, change the type inference and see what goes wrong and report back after that. I imagine src/librustc_typeck/check/closure.rs is the right place to start looking, but I haven't looked properly.

[noctune]

I've done a bit of experimenting with this, and it seems like just dropping the expected kind here works without issues for FnMut and Fn. It does mess up the ui tests though, as it changes the displayed error message.

FnOnce has some issues with closures like these where it incorrectly infers it to be a FnMut instead. Error in full:

error: captured variable cannot escape `FnMut` closure body            ] 21/36: core                                                                                    
  --> src/libcore/fmt/builders.rs:17:13
   |
12 |         Self::wrap_buf_test(fmt, move |buf| {
   |                                           - inferred to be a `FnMut` closure
...
17 |             slot.as_mut().unwrap()
   |             ^^^^^^^^^^^^^^^^^^^^^^ returns a reference to a captured variable which escapes the closure body
   |
   = note: `FnMut` closures only have access to their captured variables while they are executing...
   = note: ...therefore, they cannot allow references to captured variables to escape

It seems like the closure kind inference does not account for mutable references being returned from the closure, so it ends up inferring a less strict kind than necessary.