Clean up callable type mismatch errors #41488

estebank · 2017-04-23T22:39:41Z

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

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.

rust-highfive · 2017-04-23T22:39:45Z

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

eddyb · 2017-04-24T03:25:12Z

r? @jonathandturner

arielb1 · 2017-04-24T08:52:34Z

src/librustc/traits/error_reporting.rs

- e)
+ let expected_trait_ty = expected_trait_ref.self_ty();
+ if expected_trait_ty.is_closure() {
+ if let &TypeError::TupleSize(ref expected_found) = e {


Shouldn't you match on expected_trait_ref & actual_trait_ref, to avoid Fn(X<()>) vs. Fn(X<(Foo, Bar)>) etc.

Also, this error message is probably good for all Fn() trait users, even if they are not closures.

leonardo-m · 2017-04-24T09:10:38Z

This code:

#![allow(unused_variables)]
fn main() {
    [1, 2, 3].sort_by(|(x, y)| panic!());
}

Currently gives me lot of error output:

error[E0308]: mismatched types
 --> ...\test.rs:3:24
  |
3 |     [1, 2, 3].sort_by(|(x, y)| panic!());
  |                        ^^^^^^ expected &{integer}, found tuple
  |
  = note: expected type `&{integer}`
             found type `(_, _)`

error[E0281]: type mismatch: the type `[closure@...\test.rs:3:23: 3:40]` 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)
 --> ...\test.rs:3:15
  |
3 |     [1, 2, 3].sort_by(|(x, y)| panic!());
  |               ^^^^^^^

error[E0281]: type mismatch: the type `[closure@...\test.rs:3:23: 3:40]` implements the trait `for<'r> std::ops::FnOnce<(&'r {integer},)>`, but the trait `for<'r, 'r> std::ops::FnOnce<(&'r {integer}, &'r {integer})>` is required (expected a tuple with 2 elements, found one with 1 elements)
 --> ...\test.rs:3:15
  |
3 |     [1, 2, 3].sort_by(|(x, y)| panic!());
  |               ^^^^^^^

error: aborting due to 3 previous errors

A better error output could be:

error[E0593]: ../../src/test/ui/mismatched_types/closure-arg-count.rs:13:15
   |
13 |     [1, 2, 3].sort_by(|(x, y)| panic!());
   |                        ^^^^^^ -------- 1 argument given
   |                        |
   |                        expected closure that takes 2 arguments

nagisa · 2017-04-24T20:50:25Z

Nit on wording. In definitions/declarations it is “arguments”, not “parameters”. “parameters” is for expressions that get passed into the function call.

estebank · 2017-04-25T01:57:15Z

@nagisa @arielb1 done.

sophiajt · 2017-04-25T19:12:32Z

@leonardo-m - did the error message get worse or stay the same? If it's the same, we could take this PR and then tackle your example as a separate issue.

sophiajt · 2017-04-25T19:14:16Z

src/test/compile-fail/E0281.rs

- foo(|y| { }); //~ ERROR E0281
- //~^ ERROR E0281
+ foo(|y: String| { });
+ //~^ ERROR E0281


Can we move this to a ui test?

sophiajt · 2017-04-25T19:14:22Z

src/test/compile-fail/fn-variance-1.rs

@@ -19,9 +19,13 @@ fn apply<T, F>(t: T, f: F) where F: FnOnce(T) {
 fn main() {
 apply(&3, takes_imm);
 apply(&3, takes_mut);
- //~^ ERROR (types differ in mutability)


sophiajt · 2017-04-25T19:14:27Z

src/test/compile-fail/issue-36053-2.rs

@@ -18,4 +18,11 @@ fn main() {
 //~^ ERROR no method named `count`
 //~| ERROR E0281
 //~| ERROR E0281
+ //~| NOTE expected &str, found str


sophiajt · 2017-04-25T19:14:34Z

src/test/compile-fail/unboxed-closures-vtable-mismatch.rs

 let z = call_it(3, f);
 //~^ ERROR type mismatch
 //~| ERROR type mismatch
+ //~| NOTE expected isize, found usize


Done for all.

sophiajt · 2017-04-25T19:15:29Z

src/test/ui/mismatched_types/closure-mismatch.stderr

+ | ^^^ expected bound lifetime parameter, found concrete lifetime
+ |
+ = note: concrete lifetime that was found is lifetime '_#0r
+ = note: required because of the requirements on the impl of `Foo` for `[closure@$DIR/closure-mismatch.rs:18:9: 18:15]`


These two notes don't seem helplful. Are they what is there currently, rather than being added by this PR?

That's already part of the current output. I would prefer to remove them in a separate (smaller) PR.

Sure, that's fine

sophiajt · 2017-04-26T01:35:00Z

Yeah, that looks pretty good. 👍 from me.

arielb1 · 2017-04-26T09:12:20Z

src/librustc/traits/error_reporting.rs

 for error in errors {
- self.report_fulfillment_error(error);
+ // Avoid multiple errors saying the same thing for closures


Is this targeted at something specific? e.g. it would be nice if we could filter projection errors if there is a corresponding selection error.

You can see the effect of this in the stderr output, right now for closure type mismatch errors we output two (may be three in some cases?, haven't seen that yet) after the compiler tries to coerce to Fn, FnMut and FnOnce. This change was hacky and want to fix it properly somewhere closer to the source but this is as close as I got to the culprit last night. We can either remove this hack now and merge the PR with the duplicated errors, or merge this as is and I send a follow up PR fully addressing this.

leonardo-m · 2017-04-26T09:48:08Z

@jonathandturner: >did the error message get worse or stay the same?

I don't fully understand your question. Given this code:

#![allow(unused_variables)]
fn main() {
    [1, 2, 3].sort_by(|(x, y)| panic!());
}

This is how its error messages have evolved:

--------------------------------
1.0:

<source>:3:24: 3:30 error: mismatched types:
 expected `&_`,
    found `(_, _)`
(expected &-ptr,
    found tuple) [E0308]
<source>:3     [1, 2, 3].sort_by(|(x, y)| panic!());
                                                                                              ^~~~~~
<source>:3:15: 3:41 error: type mismatch: the type `[closure /tmp/compiler-explorer-compiler117326-8-ixlnde.do96qrggb9/example.rs:3:23: 3:40]` implements the trait `for<'r> core::ops::FnMut<(&'r _,)>`, but the trait `for<'r,'r> core::ops::FnMut<(&'r _, &'r _)>` is required (expected a tuple with 2 elements, found one with 1 elements) [E0281]
<source>:3     [1, 2, 3].sort_by(|(x, y)| panic!());
                                                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~
<source>:3:15: 3:41 error: type mismatch: the type `[closure /tmp/compiler-explorer-compiler117326-8-ixlnde.do96qrggb9/example.rs:3:23: 3:40]` implements the trait `for<'r> core::ops::FnOnce<(&'r _,)>`, but the trait `for<'r,'r> core::ops::FnOnce<(&'r _, &'r _)>` is required (expected a tuple with 2 elements, found one with 1 elements) [E0281]
<source>:3     [1, 2, 3].sort_by(|(x, y)| panic!());
                                                                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~
error: aborting due to 3 previous errors
Compiler exited with result code 101

--------------------------------
1.11:

<source>:3:24: 3:30 error: mismatched types [E0308]
<source>:3     [1, 2, 3].sort_by(|(x, y)| panic!());
                                                                                               ^~~~~~
<source>:3:24: 3:30 help: run `rustc --explain E0308` to see a detailed explanation
<source>:3:24: 3:30 note: expected type `&_`
<source>:3:24: 3:30 note:    found type `(_, _)`
<source>:3:15: 3:22 error: type mismatch: the type `[closure@/tmp/compiler-explorer-compiler117326-8-1ggak45.5sng6wdn29/example.rs:3:23: 3:40]` implements the trait `for<'r> std::ops::FnMut<(&'r _,)>`, but the trait `for<'r, 'r> std::ops::FnMut<(&'r _, &'r _)>` is required (expected a tuple with 2 elements, found one with 1 elements) [E0281]
<source>:3     [1, 2, 3].sort_by(|(x, y)| panic!());
                                                                                      ^~~~~~~
<source>:3:15: 3:22 help: run `rustc --explain E0281` to see a detailed explanation
<source>:3:15: 3:22 error: type mismatch: the type `[closure@/tmp/compiler-explorer-compiler117326-8-1ggak45.5sng6wdn29/example.rs:3:23: 3:40]` implements the trait `for<'r> std::ops::FnOnce<(&'r _,)>`, but the trait `for<'r, 'r> std::ops::FnOnce<(&'r _, &'r _)>` is required (expected a tuple with 2 elements, found one with 1 elements) [E0281]
<source>:3     [1, 2, 3].sort_by(|(x, y)| panic!());
                                                                                      ^~~~~~~
<source>:3:15: 3:22 help: run `rustc --explain E0281` to see a detailed explanation
error: aborting due to 3 previous errors
Compiler exited with result code 101

--------------------------------
1.12 - 1.18nightly:

error[E0308]: mismatched types
 --> <source>:3:24
  |
3 |     [1, 2, 3].sort_by(|(x, y)| panic!());
  |                        ^^^^^^ expected &{integer}, found tuple
  |
  = note: expected type `&{integer}`
  = note:    found type `(_, _)`
error[E0281]: type mismatch: the type `[closure@<source>:3:23: 3:40]` 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)
 --> <source>:3:15
  |
3 |     [1, 2, 3].sort_by(|(x, y)| panic!());
  |               ^^^^^^^
error[E0281]: type mismatch: the type `[closure@<source>:3:23: 3:40]` implements the trait `for<'r> std::ops::FnOnce<(&'r {integer},)>`, but the trait `for<'r, 'r> std::ops::FnOnce<(&'r {integer}, &'r {integer})>` is required (expected a tuple with 2 elements, found one with 1 elements)
 --> <source>:3:15
  |
3 |     [1, 2, 3].sort_by(|(x, y)| panic!());
  |               ^^^^^^^
error: aborting due to 3 previous errors
Compiler exited with result code 101

--------------------------------

So the error messages are essentially the same. And I think they could use an improvement (the first improvement is to generate only one error message).

estebank · 2017-04-26T16:01:50Z

@leonardo-m the output for that file with the changes on this PR is:

error[E0308]: mismatched types
  --> $DIR/closure-arg-count.rs:3:24
   |
 3 |     [1, 2, 3].sort_by(|(tuple, tuple2)| panic!());
   |                        ^^^^^^^^^^^^^^^ expected &{integer}, found tuple
   |
   = note: expected type `&{integer}`
              found type `(_, _)`

error[E0593]: closure takes 1 argument but 2 arguments are required
  --> $DIR/closure-arg-count.rs:3:15
   |
 3 |     [1, 2, 3].sort_by(|(tuple, tuple2)| panic!());
   |               ^^^^^^^ -------------------------- takes 1 argument
   |               |
   |               expected closure that takes 2 arguments


error: aborting due to 2 previous errors

leonardo-m · 2017-04-26T16:58:54Z

It's it too much to tell me five times what's the problem? Telling me once or twice should suffice.

estebank · 2017-04-26T17:17:08Z

@leonardo-m I think there might have been a misunderstanding. I believe you were pointing out the output for this error over time, since 1.0 to current nightly, where there were 3 different, but equally unspecific, outputs, as well as suggesting a possible new output. I was pointing out what the new output with this code would be like. Your proposal to point out at only the tuple arg of the closure is not trivial at the moment because I only have the full closure's span to point at, but could be done with some more work. Apologies if you felt I was talking down to you in any way.

So the error messages are essentially the same. And I think they could use an improvement (the first improvement is to generate only one error message).

This PR does that (as pointed out in a prior comment), other than removing the mismatched types error, which I'll try to remove in a clean way in a follow up PR.

sophiajt · 2017-04-26T19:19:37Z

@leonardo-m - yeah sorry, I wasn't clear either. I think you're pointing out a way it can be improved even further. Can you file an issue with your example so we don't forget and can improve your example also?

I was just trying to make sure we weren't making your example worse with this PR, but it looks like we just have more work to do after this PR.

leonardo-m · 2017-04-26T19:50:45Z

Thank you for the work.

Can you file an issue with your example so we don't forget and can improve your example also?

I think it's better to wait for this PR to be merged and inside one Nightly, then I'll take a look at the error message it gives and we can open a new enhancement request.

estebank · 2017-04-28T18:44:35Z

Ping @arielb1, @jonathandturner already 👍'd, I think the only thing left is to make a decision on wether we can leave the last commit's deduplication logic as is until I submit a cleaner follow up PR for that part or we land this PR without that code.

arielb1 · 2017-05-01T10:02:43Z

@estebank

The last commit's deduplication is a hack. I am working on a more principled version but this week is an holiday.

estebank · 2017-05-02T00:09:39Z

@arielb1 removed the hacky commit.

arielb1 · 2017-05-02T09:07:32Z

@bors r+

bors · 2017-05-02T09:07:33Z

📌 Commit b10e293 has been approved by arielb1

bors · 2017-05-02T09:41:45Z

⌛ Testing commit b10e293 with merge 50517d5...

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.

bors · 2017-05-02T12:11:48Z

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing 50517d5 to master...

rust-highfive assigned eddyb Apr 23, 2017

estebank changed the title ~~Closure args~~ Clean up closure type mismatch errors Apr 23, 2017

estebank added 2 commits April 23, 2017 15:54

Clean up closure type mismatch errors

e8cf5f3

Avoid spurious in lifetime diagnostics

366dd1b

estebank force-pushed the closure-args branch from 63b6ec6 to 366dd1b Compare April 23, 2017 22:54

estebank mentioned this pull request Apr 24, 2017

Error message for wrong number of arguments to a closure is bad #21857

Closed

estebank force-pushed the closure-args branch from be6c2af to a42832d Compare April 24, 2017 01:54

rust-highfive assigned sophiajt and unassigned eddyb Apr 24, 2017

arielb1 suggested changes Apr 24, 2017

View reviewed changes

carols10cents added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 24, 2017

estebank force-pushed the closure-args branch 3 times, most recently from d05bbec to ad291b7 Compare April 24, 2017 22:10

estebank changed the title ~~Clean up closure type mismatch errors~~ Clean up callable type mismatch errors Apr 24, 2017

Reorder code, fix unittests

b52c8c2

estebank force-pushed the closure-args branch from ad291b7 to b52c8c2 Compare April 24, 2017 23:44

sophiajt reviewed Apr 25, 2017

View reviewed changes

Move cfail tests to ui

b10e293

estebank force-pushed the closure-args branch from 430adac to d3f64a6 Compare April 26, 2017 05:01

arielb1 reviewed Apr 26, 2017

View reviewed changes

estebank mentioned this pull request Apr 26, 2017

Make unsatisfied trait bounds note multiline #41489

Merged

estebank force-pushed the closure-args branch from d3f64a6 to b10e293 Compare May 2, 2017 00:08

bors merged commit b10e293 into rust-lang:master May 2, 2017

estebank mentioned this pull request May 5, 2017

Remove duplicated errors for closure type mismatch #41760

Closed

estebank mentioned this pull request May 24, 2017

Wrong error message with closures on nightly #42143

Closed

estebank deleted the closure-args branch November 9, 2023 05:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up callable type mismatch errors #41488

Clean up callable type mismatch errors #41488

estebank commented Apr 23, 2017 •

edited

Loading

rust-highfive commented Apr 23, 2017

eddyb commented Apr 24, 2017

arielb1 Apr 24, 2017 •

edited

Loading

leonardo-m commented Apr 24, 2017

nagisa commented Apr 24, 2017

estebank commented Apr 25, 2017

sophiajt commented Apr 25, 2017

sophiajt Apr 25, 2017

sophiajt Apr 25, 2017

sophiajt Apr 25, 2017

sophiajt Apr 25, 2017

estebank Apr 26, 2017

sophiajt Apr 25, 2017

estebank Apr 26, 2017

sophiajt Apr 26, 2017

sophiajt commented Apr 26, 2017

arielb1 Apr 26, 2017 •

edited

Loading

estebank Apr 26, 2017

leonardo-m commented Apr 26, 2017

estebank commented Apr 26, 2017

leonardo-m commented Apr 26, 2017

estebank commented Apr 26, 2017 •

edited

Loading

sophiajt commented Apr 26, 2017

leonardo-m commented Apr 26, 2017

estebank commented Apr 28, 2017

arielb1 commented May 1, 2017 •

edited

Loading

estebank commented May 2, 2017

arielb1 commented May 2, 2017

bors commented May 2, 2017

bors commented May 2, 2017

bors commented May 2, 2017

Clean up callable type mismatch errors #41488

Clean up callable type mismatch errors #41488

Conversation

estebank commented Apr 23, 2017 • edited Loading

rust-highfive commented Apr 23, 2017

eddyb commented Apr 24, 2017

arielb1 Apr 24, 2017 • edited Loading

Choose a reason for hiding this comment

leonardo-m commented Apr 24, 2017

nagisa commented Apr 24, 2017

estebank commented Apr 25, 2017

sophiajt commented Apr 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sophiajt commented Apr 26, 2017

arielb1 Apr 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leonardo-m commented Apr 26, 2017

estebank commented Apr 26, 2017

leonardo-m commented Apr 26, 2017

estebank commented Apr 26, 2017 • edited Loading

sophiajt commented Apr 26, 2017

leonardo-m commented Apr 26, 2017

estebank commented Apr 28, 2017

arielb1 commented May 1, 2017 • edited Loading

estebank commented May 2, 2017

arielb1 commented May 2, 2017

bors commented May 2, 2017

bors commented May 2, 2017

bors commented May 2, 2017

estebank commented Apr 23, 2017 •

edited

Loading

arielb1 Apr 24, 2017 •

edited

Loading

arielb1 Apr 26, 2017 •

edited

Loading

estebank commented Apr 26, 2017 •

edited

Loading

arielb1 commented May 1, 2017 •

edited

Loading