Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incomprehensible error message when inference fails for closure #24680

Closed
seanmonstar opened this issue Apr 22, 2015 · 22 comments
Closed

Incomprehensible error message when inference fails for closure #24680

seanmonstar opened this issue Apr 22, 2015 · 22 comments
Assignees
Labels
A-closures Area: Closures (`|…| { … }`) A-diagnostics Area: Messages for errors, warnings, and lints A-typesystem Area: The type system P-medium Medium priority

Comments

@seanmonstar
Copy link
Contributor

struct Foo<'a> {
    _s: &'a str
}

struct Bar<H: Handler> {
    handler: H
}

trait Handler {
    fn handle<'a>(&'a self, foo: Foo<'a>);
}

impl<F> Handler for F where F: Fn(Foo) {
    fn handle<'a>(&'a self, foo: Foo<'a>) {
        self(foo)
    }
}

fn main() {
    let foo = Foo { _s: "foo" };
    let bar = Bar { handler: |_f /*: Foo*/| {} }; // uncomment to work
    (bar.handler)(foo);
}

Gives:

error: type mismatch: the type `[closure test.rs:8:23: 20:6]` implements the trait `core::ops::Fn<(_, _)>`, but the trait `for<'r,'r,'r> core::ops::Fn<(hyper::server::request::Request
<'r, 'r>, hyper::server::response::Response<'r>)>` is required (expected concrete lifetime, found bound lifetime parameter ) [E0281]

Playpen: http://is.gd/E7sskd

Couldn't tell if this was #16473, @reem thinks it might be different. Either way, this hurts using hyper's server easily: Server::http(|req, res| {}).listen(8080).

@reem
Copy link
Contributor

reem commented Apr 22, 2015

cc @aturon @nikomatsakis do you think it would be feasible to fix this by the 1.0 release? It's a pretty unfortunate papercut for both iron and hyper.

I remember I recently helped @brson with this problem when he was using iron.

@steveklabnik steveklabnik added A-typesystem Area: The type system A-closures Area: Closures (`|…| { … }`) labels Apr 22, 2015
@Ryman
Copy link
Contributor

Ryman commented Apr 23, 2015

Very similar case based on nickel's usage of hyper:

#[allow(dead_code)]
struct Bar<'a, 'b> {
    a: &'a (),
    b: &'b ()
}

#[allow(dead_code)]
struct Qux<'a> {
    a: &'a ()
}

#[allow(dead_code)]
struct Foo<'a> {
    a: &'a ()
}

trait Handler {
    fn handle<'a, 'k>(&'a self, Bar<'a, 'k>, Qux<'a>) -> Foo<'a>;
}

impl<F> Handler for F where F: for<'a, 'k> Fn(Bar<'a, 'k>, Qux<'a>) -> Foo<'a> + Sync + Send {
    fn handle<'a, 'k>(&'a self, req: Bar<'a, 'k>, res: Qux<'a>) -> Foo<'a> {
        self(req, res)
    }
}

fn call_handler<H: Handler>(h: H) {
    println!("call_handler");
    h.handle(Bar { a: &(), b: &() }, Qux { a: &() });
}

fn call_closure<F: for<'a, 'k> Fn(Bar<'a, 'k>, Qux<'a>) -> Foo<'a> + Sync + Send>(f: F) {
    println!("call_closure");
    f(Bar { a: &(), b: &() }, Qux { a: &() });
}

fn wrapped_call_handler<F: for<'a, 'k> Fn(Bar<'a, 'k>, Qux<'a>) -> Foo<'a> + Sync + Send>(f: F) {
    println!("wrapped_call_handler");
    call_handler(f)
}

fn main() {
    // These work
    call_closure(|_bar, qux| Foo { a: qux.a });
    wrapped_call_handler(|_bar, qux| Foo { a: qux.a });

    // These fail

    // 'Type must be known'
    //call_handler(|_bar, qux| Foo { a: qux.a });

    // error: type mismatch resolving `for<'a,'k> <[closure <anon>:52:18: 52:57] as core::ops::FnOnce<(Bar<'a, 'k>, Qux<'a>)>>::Output == Foo<'a>`:
    // expected bound lifetime parameter 'a
    //call_handler(|_bar: Bar, qux: Qux|  Foo { a: qux.a });
}

Playpen: http://is.gd/OEkSd4

The fact that it works with a wrapping function means it's possible to get things done with convenience macros, which suggests the compiler is at least able to reason about this at some level.

@brson
Copy link
Contributor

brson commented Apr 23, 2015

Nominating because I hit this quickly trying to use Iron, the error was impenetrable and I had to ask for help.

@bstrie
Copy link
Contributor

bstrie commented Apr 23, 2015

Accidentally duped this bug, but not before coming up with a more minimal test case:

trait Foo {}

impl<T: Fn(&())> Foo for T {}

fn baz<T: Foo>(_: T) {}

fn main() {
    baz(|_| ());
}

@nikomatsakis nikomatsakis changed the title Inference fails for closure Incomprehensible error message when inference fails for closure Apr 23, 2015
@nikomatsakis
Copy link
Contributor

I updated the title to reflect the fact that the error message is really bad here. It'd be nice to improve the inference, but in short term error message is priority.

@pnkfelix
Copy link
Member

P-high : provide better feedback in the error message to guide the user towards adding type annotations on the closure arguments.

@pnkfelix pnkfelix added P-high High priority and removed I-nominated labels Apr 23, 2015
@Ryman
Copy link
Contributor

Ryman commented Apr 23, 2015

@nikomatsakis Should I open another issue for the example I added? I don't know of any syntax available for type annotating the return type with lifetime requirements other than wrapping with a function?

@pnkfelix
Copy link
Member

@Ryman I'm not sure whether opening a separate issue is necessary, as long as we make sure not to close this one with whatever error message improvements we land to address the P-high part of this.

@kornelski
Copy link
Contributor

I've ran into this with:

fn main() {
    let head = [(1,2), (3,4)];
    let tail = [(4,5), (6,7)];

    // missing .cloned()
    let _:Vec<_> = head.iter().chain(tail.iter().map(|x|{(x.0+1,x.1+1)})).collect(); 
}

error: type mismatch resolving <[closure@<anon>:5:54: 5:72] as core::ops::FnOnce<(&(_, _),)>>::Output == &(_, _):
expected tuple,
found &-ptr [E0271]

:5:75: 5:84 error: no method named collect found for type core::iter::Chain<core::slice::Iter<'_, (_, _)>, core::iter::Map<core::slice::Iter<'_, (_, _)>, [closure@<anon>:5:54: 5:72]>> in the current scope

To Rust's credit "expected tuple, found &-ptr" was a good hint, but the error as a whole—with anon closure and iterator's guts—looks rather scary.

@ticki
Copy link
Contributor

ticki commented Dec 25, 2015

Just ran into this while using hyper. It's completely incomprehensible and obscure.

Is there a general fix to it?

@nikomatsakis
Copy link
Contributor

cc @brson is this roughly the case you were talking about today as well?

@jonas-schievink
Copy link
Contributor

Why does @bstrie's example give a better error message on stable than on beta and nightly? Neither beta nor nightly mention "type X implements Fn(...) but Fn(...) is required"

@nikomatsakis
Copy link
Contributor

I think there are two separable (and important) steps to take here---

  1. We can add some logic that targets the Fn traits and closure types specifically. We should be able to diagnose common failures (argument N has wrong type; wrong number of arguments; etc).
  2. It is also interesting to contemplate ways to format the more general error that make it more readable. @brson suggested adopting a format where we extract out the types from the main message, for example. So maybe say something like `the type A (see below) does not implement the required trait B" and then down below write "type A is: ...". Have to play with it, but I think the idea is sound.

@nikomatsakis
Copy link
Contributor

cc @jonathandturner :)

@nikomatsakis nikomatsakis added the A-diagnostics Area: Messages for errors, warnings, and lints label May 16, 2016
@shepmaster
Copy link
Member

@nikomatsakis do you think that #26937 is a duplicate?

@nikomatsakis
Copy link
Contributor

@shepmaster well my feeling is that there are various intersecting problems here. First off, our output is pretty hard to read in general (because we don't do a good job "desugaring" the fn traits and so forth). Second, we don't handle higher-ranked messages very well. Finally, and least likely to be fixed, the inference has some limitations. So probably the answer is "yes", but maybe the right thing is to try and separate out the various problems into distinct issues that are more specific.

@brson brson added the P-medium Medium priority label Jun 23, 2016
@brson
Copy link
Contributor

brson commented Jun 23, 2016

triage: Dropped to P-medium just because the impact is relatively low.

@brson brson removed the P-high High priority label Jun 23, 2016
@sanmai-NL
Copy link

@brson: I remember a forum thread on the future priorities for Rust, which I couldn't find back now. It referred to the 2016 survey that found that the biggest challenge for Rust adoption in the eyes of the users is the learning curve. I think there is a connection between learnability and how easily developers understand error messages.
Could you please explain a bit your assessment of this issue as P-medium given this context?

@sophiajt
Copy link
Contributor

@sanmai-NL - absolutely error messages are an important part of learnability/usability.

Here, I think priorities above medium mean that this bug severely impacts the correctness of code or stability of the compiler (crashing the compiler, for example).

We're doing on-going work to address error messages holistically (eg #35233 as one example), so as a whole improving errors is a high priority for us, but as far as bug triage goes, individual bugs fall closer to the medium priority when weighed against much worse experiences (like compiler crashes or bad code)

@FraGag
Copy link
Contributor

FraGag commented Nov 14, 2016

Yesterday, I answered a question on Stack Overflow about this issue. I tried to come up with an explanation for the error message, in the hope that it would provide some insight on how to fix the issue in the compiler. (I didn't debug the compiler compiling the code below or even read the compiler's source code to validate any of this, so my analysis could be completely wrong!)

In case the above link becomes broken in the future, I'm reproducing parts of the question (from Jacob Brown) and of my answer below.

The code that triggers the issue is this:

use std::boxed::Box;

#[derive(Debug)]
pub struct Foo<'a>(pub &'a str);

pub trait IntoBox {
    fn into_box<'a>(self) -> Box<Fn(Foo) -> String>;
}

impl<B> IntoBox for B where B: Fn(Foo) -> String + 'static {
    fn into_box(self) -> Box<Fn(Foo) -> String> { Box::new(self) }
}

fn direct_into_box<B: Fn(Foo) -> String + 'static>(b: B) -> Box<Fn(Foo) -> String> {
    Box::new(b)
}

fn main() {
    // Doesn't work
    let x = IntoBox::into_box(|i| format!("{:?}", i) );

    // Works
    let y = IntoBox::into_box(|i: Foo| format!("{:?}", i) );

    // Also works
    let z = direct_into_box(|i| format!("{:?}", i) );
}

This code raises the following compiler errors:

error[E0271]: type mismatch resolving `for<'r> <[closure@<anon>:20:31: 20:53] as std::ops::FnOnce<(Foo<'r>,)>>::Output == std::string::String`
  --> <anon>:20:13
   |
20 |     let x = IntoBox::into_box(|i| format!("{:?}", i) );
   |             ^^^^^^^^^^^^^^^^^ expected bound lifetime parameter , found concrete lifetime
   |
   = note: concrete lifetime that was found is lifetime '_#29r
   = note: required because of the requirements on the impl of `IntoBox` for `[closure@<anon>:20:31: 20:53]`
   = note: required by `IntoBox::into_box`

error[E0281]: type mismatch: the type `[closure@<anon>:20:31: 20:53]` implements the trait `std::ops::Fn<(_,)>`, but the trait `for<'r> std::ops::Fn<(Foo<'r>,)>` is required (expected concrete lifetime, found bound lifetime parameter )
  --> <anon>:20:13
   |
20 |     let x = IntoBox::into_box(|i| format!("{:?}", i) );
   |             ^^^^^^^^^^^^^^^^^
   |
   = note: required because of the requirements on the impl of `IntoBox` for `[closure@<anon>:20:31: 20:53]`
   = note: required by `IntoBox::into_box`

What seems to happen is that the compiler implements Fn(Foo<'x>) for one specific lifetime 'x instead of Fn(Foo<'a>) for any lifetime 'a on the closure that causes the error.

I tried to replicate the error by defining a struct by hand (using unstable features). First, I defined the struct the correct way:

#![feature(fn_traits)]
#![feature(unboxed_closures)]

// Foo and IntoBox unchanged

struct Func;

impl<'a> FnOnce<(Foo<'a>,)> for Func {
    type Output = String;

    extern "rust-call" fn call_once(self, args: (Foo<'a>,)) -> String {
        self.call(args)
    }
}

impl<'a> FnMut<(Foo<'a>,)> for Func {
    extern "rust-call" fn call_mut(&mut self, args: (Foo<'a>,)) -> String {
        self.call(args)
    }
}

impl<'a> Fn<(Foo<'a>,)> for Func {
    extern "rust-call" fn call(&self, (i,): (Foo<'a>,)) -> String {
        format!("{:?}", i)
    }
}

fn main() {
    let x = IntoBox::into_box(Func);
}

This Func struct compiles fine and behaves just like the original closure.

Then, I broke it:

impl FnOnce<(Foo<'static>,)> for Func {
    type Output = String;

    extern "rust-call" fn call_once(self, args: (Foo<'static>,)) -> String {
        self.call(args)
    }
}

impl FnMut<(Foo<'static>,)> for Func {
    extern "rust-call" fn call_mut(&mut self, args: (Foo<'static>,)) -> String {
        self.call(args)
    }
}

impl Fn<(Foo<'static>,)> for Func {
    extern "rust-call" fn call(&self, (i,): (Foo<'static>,)) -> String {
        format!("{:?}", i)
    }
}

What I've done here is that I've removed the <'a> on each impl, so that the impls are no longer generic over a lifetime, and I've replaced Foo<'a> with Foo<'static>. This means that now, the traits are only implemented when the "closure"'s argument is a Foo<'static>.

This fails to compile with the following errors:

error[E0271]: type mismatch resolving `for<'r> <Func as std::ops::FnOnce<(Foo<'r>,)>>::Output == std::string::String`
  --> <anon>:51:13
   |
51 |     let x = IntoBox::into_box(Func);
   |             ^^^^^^^^^^^^^^^^^ expected bound lifetime parameter , found concrete lifetime
   |
   = note: concrete lifetime that was found is the static lifetime
   = note: required because of the requirements on the impl of `IntoBox` for `Func`
   = note: required by `IntoBox::into_box`

error[E0277]: the trait bound `for<'r> Func: std::ops::Fn<(Foo<'r>,)>` is not satisfied
  --> <anon>:51:13
   |
51 |     let x = IntoBox::into_box(Func);
   |             ^^^^^^^^^^^^^^^^^ the trait `for<'r> std::ops::Fn<(Foo<'r>,)>` is not implemented for `Func`
   |
   = help: the following implementations were found:
   = help:   <Func as std::ops::Fn<(Foo<'static>,)>>
   = note: required because of the requirements on the impl of `IntoBox` for `Func`
   = note: required by `IntoBox::into_box`

The first error is the same, but instead of an internal name like '_#29r, the compiler mentions the static lifetime, because that's what I used here (in fact, I couldn't use any other lifetime). I suspect that what the compiler is doing with the closure that doesn't compile in the original code is similar to my second set of impls, just that instead of 'static, it's some other concrete lifetime that we can't name in Rust. I believe that's where the confusion comes from: the compiler does something that is not possible in normal Rust, so the poor developer doesn't understand what the compiler did.

The second error is different but means pretty much the same thing.

@seanmonstar
Copy link
Contributor Author

I just hit this again in a new crate, which made me decide to make the generic be Fn instead of a trait that all closures would be implemented for: https://github.com/seanmonstar/reqwest/pull/29/files#diff-6c0665a7b81d5ea7ac8728b2cfa41e64R109

@Mark-Simulacrum
Copy link
Member

Closing in favor of #41078.

bors added a commit that referenced this issue May 2, 2017
Clean up callable type mismatch errors

```rust
error[E0593]: closure takes 1 argument but 2 arguments are required here
  --> ../../src/test/ui/mismatched_types/closure-arg-count.rs:13:15
   |
13 |     [1, 2, 3].sort_by(|(tuple, tuple2)| panic!());
   |               ^^^^^^^ -------------------------- takes 1 argument
   |               |
   |               expected closure that takes 2 arguments
```

instead of

```rust
error[E0281]: type mismatch: the type `[closure@../../src/test/ui/mismatched_types/closure-arg-count.rs:13:23: 13:49]` implements the trait `for<'r> std::ops::FnMut<(&'r {integer},)>`, but the trait `for<'r, 'r> std::ops::FnMut<(&'r {integer}, &'r {integer})>` is required (expected a tuple with 2 elements, found one with 1 elements)
  --> ../../src/test/ui/mismatched_types/closure-arg-count.rs:13:15
   |
13 |     [1, 2, 3].sort_by(|(tuple, tuple2)| panic!());
   |               ^^^^^^^
```

Fix #21857, re #24680.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-closures Area: Closures (`|…| { … }`) A-diagnostics Area: Messages for errors, warnings, and lints A-typesystem Area: The type system P-medium Medium priority
Projects
None yet
Development

No branches or pull requests