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

Diagnostics for blanket impl of closure traits are suboptimal #24210

Closed
huonw opened this issue Apr 8, 2015 · 6 comments
Closed

Diagnostics for blanket impl of closure traits are suboptimal #24210

huonw opened this issue Apr 8, 2015 · 6 comments
Labels
A-closures Area: Closures (`|…| { … }`) A-diagnostics Area: Messages for errors, warnings, and lints A-trait-system Area: Trait system P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@huonw
Copy link
Member

huonw commented Apr 8, 2015

fn direct<F: Fn()>(_: F) {}

trait Foo { fn method(&self) {} }

impl<F: Fn()> Foo for F {}

fn blanket<F: Foo>(_: F) {}

struct Moves;
fn main() {
    let x = Moves;   direct(|| drop(x));

    // let y = Moves;   blanket(|| drop(y));
}

The x line gives

<anon>:11:37: 11:38 error: cannot move out of captured outer variable in an `Fn` closure
<anon>:11     let x = Moves;   direct(|| drop(x));
                                              ^

while the y line gives:

<anon>:13:22: 13:29 error: the trait `core::ops::Fn<()>` is not implemented for the type `[closure <anon>:13:30: 13:40]` [E0277]
<anon>:13     let y = Moves;   blanket(|| drop(y));
                               ^~~~~~~

The latter doesn't really give much information about the problem (moving out of y) and, AFAIK, the most effective way to debug is to find the offending statement via binary search commenting out chunks of code. :(

@huonw huonw added A-diagnostics Area: Messages for errors, warnings, and lints A-trait-system Area: Trait system A-closures Area: Closures (`|…| { … }`) labels Apr 8, 2015
@nikomatsakis nikomatsakis self-assigned this Apr 8, 2015
@nikomatsakis
Copy link
Contributor

triage: P-high (1.0)

I consider this a nice to have, but it's prob not too hard to really improve this error message.

@rust-highfive rust-highfive added the P-medium Medium priority label Apr 8, 2015
@rust-highfive rust-highfive added this to the 1.0 milestone Apr 8, 2015
@ftxqxd
Copy link
Contributor

ftxqxd commented Apr 11, 2015

You don’t even need a blanket trait impl to get a poor error message; using an intermediate variable binding also gives no hint as to what stops the closure from implementing Fn:

fn foo<T: Fn()>(_: T) {}

fn main() {
    let a = Box::new(1);
    let x = || {
        a;
    };
    foo(x);
}

gives

<anon>:8:5: 8:8 error: the trait `core::ops::Fn<()>` is not implemented for the type `[closure <anon>:5:13: 7:6]` [E0277]
<anon>:8     foo(x);
             ^~~
error: aborting due to previous error

@blaenk
Copy link
Contributor

blaenk commented Apr 11, 2015

hahahaha yes, I'm glad I'm not the only one that thinks this. I think this is the weakest area I've witnessed in Rust's error diagnostics and I think it will be a major pain-point for newcomers, which will probably lead them to see Rust closures as very complex and difficult to understand. I had a lot of trouble with this myself until I ran into it multiple times and @p1start helped me track down the problem, so I think I know what to look for now.

I'm happy to see that this is a high priority!

@steveklabnik steveklabnik removed this from the 1.0 milestone May 21, 2015
bors added a commit that referenced this issue Apr 13, 2016
Replace consider_unification_despite_ambiguity with new obligation variant

Is work towards #32730. Addresses part one of #32286. Addresses #24210 and #26046 to some degree.

r? @nikomatsakis
@brson brson added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 14, 2016
@alexcrichton
Copy link
Member

triage: P-low

Note a P-medium issue by today's standards, and also hasn't gone fixed for quite awhile! Also cc @jonathandturner, issue about diagnostics. Also removing the assignee of @nikomatsakis as I believe this isn't actively being worked on

@rust-highfive rust-highfive added P-low Low priority and removed P-medium Medium priority labels Jul 14, 2016
@arielb1 arielb1 self-assigned this Jul 17, 2016
@Mark-Simulacrum
Copy link
Member

@arielb1 Unassigning you since this isn't being worked on as far as I know.

with x:

error[E0507]: cannot move out of captured outer variable in an `Fn` closure
  --> test.rs:11:37
   |
11 |     let x = Moves;   direct(|| drop(x));
   |         - captured outer variable   ^ cannot move out of captured outer variable in an `Fn` closure

error: aborting due to previous error

with y:

error[E0525]: expected a closure that implements the `Fn` trait, but this closure only implements `FnOnce`
  --> test.rs:13:30
   |
13 |     let y = Moves;   blanket(|| drop(y));
   |                              ^^^^^^^^^^
   |
note: the requirement to implement `Fn` derives from here
  --> test.rs:13:22
   |
13 |     let y = Moves;   blanket(|| drop(y));
   |                      ^^^^^^^

error: aborting due to previous error

@Mark-Simulacrum
Copy link
Member

Today, the y case is fixed. There appear to be UI tests for this already (src/test/ui/closure_context/issue-26046-fn-once.stderr) and an fn-mut partner to it so closing.

error[E0525]: expected a closure that implements the `Fn` trait, but this closure only implements `FnOnce`
  --> test.rs:12:28
   |
12 |     let y = Moves; blanket(|| drop(y));
   |                    ------- ^^^^^^^^^^
   |                    |
   |                    the requirement to implement `Fn` derives from here
   |
note: closure is `FnOnce` because it moves the variable `y` out of its environment
  --> test.rs:12:36
   |
12 |     let y = Moves; blanket(|| drop(y));
   |                                    ^

error: aborting due to previous error(s)

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-trait-system Area: Trait system P-low Low priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

10 participants