Opaque error message when typechecking closure

[rozbb]

UPDATE: Mentoring instructions found in this comment below.

---

The following code fails because the closure moves out of vec, and anything implementing Fn only gets a reference to its environment (i.e can't move self)

fn get_closure() -> Box<Fn() -> Vec<u8>> {
    let vec = vec![1u8, 2u8];

    let closure = move || {
        vec
    };

    Box::new(closure)
}

A simple fix that would allow the closure to properly implement Fn is to replace vec with vec.clone() inside the closure. The error message, however, is of no help:

error: the trait `core::ops::Fn<()>` is not implemented for the type `[closure <anon>:6:24: 8:6]` [E0277]
<line #> Box::new(closure)
         ^~~~~~~~~~~~~~~~~

In cases where the closure is more complex than this trivial example, the error message is no more descriptive than above, making it much more difficult to debug.

I would like to see something more along the lines of:

error: the trait `core::ops::Fn<()>` is not implemented for the type `[closure <anon>:6:24: 8:6]`
<line #> note: closure moves out of environment: vec
                                                 ^~~

[abonander]

Here's a user on Reddit who was confused by these error messages.

This is a real problem.

[steveklabnik]

This error message has gotten better:

error[E0525]: expected a closure that implements the `Fn` trait, but this closure only implements `FnOnce`
 --> <anon>:4:19
  |
4 |     let closure = move || {
  |                   ^
  |
note: the requirement to implement `Fn` derives from here
 --> <anon>:8:5
  |
8 |     Box::new(closure)
  |     ^^^^^^^^^^^^^^^^^

Is it good enough? I'm not sure.

[Mark-Simulacrum]

I think it would be nice if the "derives from here" message pointed at the type or printed the type of the expression. This is certainly not necessary, though.

[Mark-Simulacrum]

I believe this will be fixed by #42196.

[Mark-Simulacrum]

Hm, looks like it wasn't -- @nikomatsakis you provided mentoring instructions for #42065; I expected that in the case of the code in the description of this issue it'd point at the vec in the closure body and state that it is the reason the closure is FnOnce. Could you clarify why that isn't the case, and perhaps provide some instructions for how to make it so? The current error is below:

error[E0525]: expected a closure that implements the `Fn` trait, but this closure only implements `FnOnce`
  --> test.rs:17:19
   |
17 |       let closure = move || {
   |  ___________________^
18 | |         vec
19 | |     };
   | |_____^
   |
note: the requirement to implement `Fn` derives from here
  --> test.rs:21:5
   |
21 |     Box::new(closure)
   |     ^^^^^^^^^^^^^^^^^

error: aborting due to previous error(s)

[nikomatsakis]

Yeah. Perhaps @tommyip would like to extend their previous work to cover this case! The short version is that, in #42196, we added the infrastructure to track why which variable we need to print, but we only print that information in one particular case (the case where the closure is being called twice). There are more errors where it'd be useful context (like this one).

The relevant code for printing the error is here. We would need to add similar calls to the error-reporting code here.

The tricky bit is that we have to find the right "tables" to get the data from. When this code runs, the self is an inference context, which will typically have a tables associated with it. I'd probably code this in a defensive way. That is, I would match on self.tables to see if there is a valid tables associated with this inference context, but if not, just don't emit the extra data into the error (the matching code might look rather like this function, but without the panic!). If there is a tables present, then check to see if the data for this closure is found in there (it may not be, maybe ti's the wrong one). If all of those conditions are met, we can emit the extra info (I expect they always will be in practice).

[tommyip]

I will have a look at this, are there any other places that would benefit from the tracked data?

[tommyip]

@nikomatsakis
Ok I got this working, how should the error message look? Maybe something like:

error[E0525]: expected a closure that implements the `Fn` trait, but this closure only implements `FnOnce`
 --> ../rust-prog/issue_26046.rs:4:19
  |
4 |       let closure = move || {
  |  ___________________^
5 | |         vec
6 | |     };
  | |_____^
  |
note: closure is `FnOnce` because it moves the variable `vec` out of its environment
 --> ../rust-prog/issue_26046.rs:5:9
  |
5 |         vec
  |         ^^^

error: aborting due to previous error(s)

If there is no valid table associated with the inference context it would just fallback to the existing message, ie:

note: the requirement to implement `Fn` derives from here
  --> test.rs:21:5
   |
21 |     Box::new(closure)
   |     ^^^^^^^^^^^^^^^^^

[rozbb]

Speaking for myself, I think that error message is great