Skip to content

Clean up callable type mismatch errors #41488

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

Merged
merged 4 commits into from
May 2, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Clean up closure type mismatch errors
  • Loading branch information
estebank committed Apr 23, 2017
commit e8cf5f366263533bd739c4dda9bc8a57ec55b8b9
2 changes: 2 additions & 0 deletions src/librustc/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1850,4 +1850,6 @@ register_diagnostics! {
E0495, // cannot infer an appropriate lifetime due to conflicting requirements
E0566, // conflicting representation hints
E0587, // conflicting packed and align representation hints
E0593, // closure argument count mismatch
E0594 // closure mismatch
}
66 changes: 58 additions & 8 deletions src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use rustc::lint::builtin::EXTRA_REQUIREMENT_IN_IMPL;
use std::fmt;
use syntax::ast::{self, NodeId};
use ty::{self, AdtKind, ToPredicate, ToPolyTraitRef, Ty, TyCtxt, TypeFoldable, TyInfer, TyVar};
use ty::error::ExpectedFound;
use ty::error::{ExpectedFound, TypeError};
use ty::fast_reject;
use ty::fold::TypeFolder;
use ty::subst::Subst;
Expand Down Expand Up @@ -663,13 +663,63 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
if actual_trait_ref.self_ty().references_error() {
return;
}
struct_span_err!(self.tcx.sess, span, E0281,
"type mismatch: the type `{}` implements the trait `{}`, \
but the trait `{}` is required ({})",
expected_trait_ref.self_ty(),
expected_trait_ref,
actual_trait_ref,
e)
let expected_trait_ty = expected_trait_ref.self_ty();
if expected_trait_ty.is_closure() {
if let &TypeError::TupleSize(ref expected_found) = e {
Copy link
Contributor

@arielb1 arielb1 Apr 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

let mut err = struct_span_err!(self.tcx.sess, span, E0593,
"closure takes {} parameter{} but {} parameter{} are required here",
expected_found.found,
if expected_found.found == 1 { "" } else { "s" },
expected_found.expected,
if expected_found.expected == 1 { "" } else { "s" });

err.span_label(span, &format!("expected closure that takes {} parameter{}",
expected_found.expected,
if expected_found.expected == 1 {
""
} else {
"s"
}));
let closure_span = expected_trait_ty.ty_to_def_id().and_then(|did| {
self.tcx.hir.span_if_local(did)
});
if let Some(span) = closure_span {
err.span_label(span, &format!("takes {} parameter{}",
expected_found.found,
if expected_found.found == 1 {
""
} else {
"s"
}));
}
err
} else {
let mut err = struct_span_err!(self.tcx.sess, span, E0594,
"closure mismatch: `{}` implements the trait `{}`, \
but the trait `{}` is required",
expected_trait_ty,
expected_trait_ref,
actual_trait_ref);

let closure_span = expected_trait_ty.ty_to_def_id().and_then(|did| {
self.tcx.hir.span_if_local(did)
});
if let Some(span) = closure_span {
err.span_label(span, &format!("{}", e));
} else {
err.note(&format!("{}", e));
}
err
}
} else {
struct_span_err!(self.tcx.sess, span, E0281,
"type mismatch: the type `{}` implements the trait `{}`, \
but the trait `{}` is required ({})",
expected_trait_ty,
expected_trait_ref,
actual_trait_ref,
e)
}
}

TraitNotObjectSafe(did) => {
Expand Down
16 changes: 16 additions & 0 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,15 @@ pub enum BoundRegion {
BrEnv,
}

impl BoundRegion {
pub fn is_named(&self) -> bool {
match *self {
BoundRegion::BrNamed(..) => true,
_ => false,
}
}
}

/// When a region changed from late-bound to early-bound when #32330
/// was fixed, its `RegionParameterDef` will have one of these
/// structures that we can use to give nicer errors.
Expand Down Expand Up @@ -1193,6 +1202,13 @@ impl<'a, 'gcx, 'tcx> TyS<'tcx> {
}
}

pub fn is_closure(&self) -> bool {
match self.sty {
TyClosure(..) => true,
_ => false,
}
}

pub fn is_integral(&self) -> bool {
match self.sty {
TyInfer(IntVar(_)) | TyInt(_) | TyUint(_) => true,
Expand Down
14 changes: 14 additions & 0 deletions src/test/ui/mismatched_types/closure-arg-count.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

fn main() {
[1, 2, 3].sort_by(|tuple| panic!());
[1, 2, 3].sort_by(|(tuple, tuple2)| panic!());
}
43 changes: 43 additions & 0 deletions src/test/ui/mismatched_types/closure-arg-count.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
error[E0593]: closure takes 1 parameter but 2 parameters are required here
--> $DIR/closure-arg-count.rs:12:15
|
12 | [1, 2, 3].sort_by(|tuple| panic!());
| ^^^^^^^ ---------------- takes 1 parameter
| |
| expected closure that takes 2 parameters

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

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

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

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

error: aborting due to 5 previous errors

19 changes: 19 additions & 0 deletions src/test/ui/mismatched_types/closure-mismatch.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

trait Foo {}

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

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

fn main() {
baz(|_| ());
}
21 changes: 21 additions & 0 deletions src/test/ui/mismatched_types/closure-mismatch.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
error[E0271]: type mismatch resolving `for<'r> <[closure@$DIR/closure-mismatch.rs:18:9: 18:15] as std::ops::FnOnce<(&'r (),)>>::Output == ()`
--> $DIR/closure-mismatch.rs:18:5
|
18 | baz(|_| ());
| ^^^ 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]`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that's fine

= note: required by `baz`

error[E0594]: closure mismatch: `[closure@$DIR/closure-mismatch.rs:18:9: 18:15]` implements the trait `std::ops::Fn<(_,)>`, but the trait `for<'r> std::ops::Fn<(&'r (),)>` is required
--> $DIR/closure-mismatch.rs:18:5
|
18 | baz(|_| ());
| ^^^ ------ expected concrete lifetime, found bound lifetime parameter
|
= note: required because of the requirements on the impl of `Foo` for `[closure@$DIR/closure-mismatch.rs:18:9: 18:15]`
= note: required by `baz`

error: aborting due to 2 previous errors