Skip to content

Commit

Permalink
auto merge of #18264 : jakub-/rust/var-ids-in-error-messages, r=nikom…
Browse files Browse the repository at this point in the history
…atsakis

This PR aims to improve the readability of diagnostic messages that involve unresolved type variables. Currently, messages like the following:

```rust
mismatched types: expected `core::result::Result<uint,()>`, found `core::option::Option<<generic #1>>`
<anon>:6     let a: Result<uint, ()> = None;
                                       ^~~~
mismatched types: expected `&mut <generic #2>`, found `uint`
<anon>:7     f(42u);
               ^~~
```

tend to appear unapproachable to new users. [0] While specific type var IDs are valuable in
diagnostics that deal with more than one such variable, in practice many messages
only mention one. In those cases, leaving out the specific number makes the messages
slightly less terrifying.

```rust
mismatched types: expected `core::result::Result<uint, ()>`, found `core::option::Option<_>`
<anon>:6     let a: Result<uint, ()> = None;
                                       ^~~~
mismatched types: expected `&mut _`, found `uint`
<anon>:7     f(42u);
               ^~~
```

As you can see, I also tweaked the aesthetics slightly by changing type variables to use the type hole syntax _. For integer variables, the syntax used is:

```rust
mismatched types: expected `core::result::Result<uint, ()>`, found `core::option::Option<_#1i>`
<anon>:6     let a: Result<uint, ()> = Some(1);
```

and float variables:

```rust
mismatched types: expected `core::result::Result<uint, ()>`, found `core::option::Option<_#1f>`
<anon>:6     let a: Result<uint, ()> = Some(0.5);
```

[0] https://twitter.com/coda/status/517713085465772032

Closes #2632.
Closes #3404.
Closes #18426.
  • Loading branch information
bors committed Oct 31, 2014
2 parents 065caf3 + a2624fc commit 82045ca
Show file tree
Hide file tree
Showing 29 changed files with 240 additions and 194 deletions.
39 changes: 17 additions & 22 deletions src/librustc/middle/ty.rs
Expand Up @@ -1032,10 +1032,8 @@ pub enum type_err {
terr_ref_mutability,
terr_vec_mutability,
terr_tuple_size(expected_found<uint>),
terr_fixed_array_size(expected_found<uint>),
terr_ty_param_size(expected_found<uint>),
terr_record_size(expected_found<uint>),
terr_record_mutability,
terr_record_fields(expected_found<Ident>),
terr_arg_count,
terr_regions_does_not_outlive(Region, Region),
terr_regions_not_same(Region, Region),
Expand Down Expand Up @@ -3790,8 +3788,8 @@ pub fn ty_sort_string(cx: &ctxt, t: t) -> String {

ty_enum(id, _) => format!("enum {}", item_path_str(cx, id)),
ty_uniq(_) => "box".to_string(),
ty_vec(_, Some(_)) => "array".to_string(),
ty_vec(_, None) => "unsized array".to_string(),
ty_vec(_, Some(n)) => format!("array of {} elements", n),
ty_vec(_, None) => "slice".to_string(),
ty_ptr(_) => "*-ptr".to_string(),
ty_rptr(_, _) => "&-ptr".to_string(),
ty_bare_fn(_) => "extern fn".to_string(),
Expand Down Expand Up @@ -3874,27 +3872,18 @@ pub fn type_err_to_str(cx: &ctxt, err: &type_err) -> String {
values.expected,
values.found)
}
terr_tuple_size(values) => {
format!("expected a tuple with {} elements, \
terr_fixed_array_size(values) => {
format!("expected an array with a fixed size of {} elements, \
found one with {} elements",
values.expected,
values.found)
}
terr_record_size(values) => {
format!("expected a record with {} fields, \
found one with {} fields",
terr_tuple_size(values) => {
format!("expected a tuple with {} elements, \
found one with {} elements",
values.expected,
values.found)
}
terr_record_mutability => {
"record elements differ in mutability".to_string()
}
terr_record_fields(values) => {
format!("expected a record with field `{}`, found one \
with field `{}`",
token::get_ident(values.expected),
token::get_ident(values.found))
}
terr_arg_count => {
"incorrect number of function parameters".to_string()
}
Expand Down Expand Up @@ -3923,9 +3912,15 @@ pub fn type_err_to_str(cx: &ctxt, err: &type_err) -> String {
trait_store_to_string(cx, (*values).found))
}
terr_sorts(values) => {
format!("expected {}, found {}",
ty_sort_string(cx, values.expected),
ty_sort_string(cx, values.found))
// A naive approach to making sure that we're not reporting silly errors such as:
// (expected closure, found closure).
let expected_str = ty_sort_string(cx, values.expected);
let found_str = ty_sort_string(cx, values.found);
if expected_str == found_str {
format!("expected {}, found a different {}", expected_str, found_str)
} else {
format!("expected {}, found {}", expected_str, found_str)
}
}
terr_traits(values) => {
format!("expected trait `{}`, found trait `{}`",
Expand Down
10 changes: 10 additions & 0 deletions src/librustc/middle/typeck/infer/combine.rs
Expand Up @@ -515,6 +515,16 @@ pub fn super_tys<'tcx, C: Combine<'tcx>>(this: &C, a: ty::t, b: ty::t) -> cres<t
Ok(ty::mk_rptr(tcx, r, mt))
}

(&ty::ty_vec(a_t, Some(sz_a)), &ty::ty_vec(b_t, Some(sz_b))) => {
this.tys(a_t, b_t).and_then(|t| {
if sz_a == sz_b {
Ok(ty::mk_vec(tcx, t, Some(sz_a)))
} else {
Err(ty::terr_fixed_array_size(expected_found(this, sz_a, sz_b)))
}
})
}

(&ty::ty_vec(a_t, sz_a), &ty::ty_vec(b_t, sz_b)) => {
this.tys(a_t, b_t).and_then(|t| {
if sz_a == sz_b {
Expand Down
12 changes: 4 additions & 8 deletions src/librustc/middle/typeck/infer/error_reporting.rs
Expand Up @@ -111,7 +111,7 @@ pub trait ErrorReporting {

fn values_str(&self, values: &ValuePairs) -> Option<String>;

fn expected_found_str<T:UserString+Resolvable>(
fn expected_found_str<T: UserString + Resolvable>(
&self,
exp_found: &ty::expected_found<T>)
-> Option<String>;
Expand Down Expand Up @@ -396,16 +396,12 @@ impl<'a, 'tcx> ErrorReporting for InferCtxt<'a, 'tcx> {
* or None if this is a derived error.
*/
match *values {
infer::Types(ref exp_found) => {
self.expected_found_str(exp_found)
}
infer::TraitRefs(ref exp_found) => {
self.expected_found_str(exp_found)
}
infer::Types(ref exp_found) => self.expected_found_str(exp_found),
infer::TraitRefs(ref exp_found) => self.expected_found_str(exp_found)
}
}

fn expected_found_str<T:UserString+Resolvable>(
fn expected_found_str<T: UserString + Resolvable>(
&self,
exp_found: &ty::expected_found<T>)
-> Option<String>
Expand Down
65 changes: 26 additions & 39 deletions src/librustc/middle/typeck/infer/mod.rs
Expand Up @@ -36,7 +36,8 @@ use syntax::ast;
use syntax::codemap;
use syntax::codemap::Span;
use util::common::indent;
use util::ppaux::{bound_region_to_string, ty_to_string, trait_ref_to_string, Repr};
use util::ppaux::{bound_region_to_string, ty_to_string};
use util::ppaux::{trait_ref_to_string, Repr};

use self::coercion::Coerce;
use self::combine::{Combine, CombineFields};
Expand Down Expand Up @@ -900,32 +901,25 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
err: Option<&ty::type_err>) {
debug!("hi! expected_ty = {}, actual_ty = {}", expected_ty, actual_ty);

let error_str = err.map_or("".to_string(), |t_err| {
format!(" ({})", ty::type_err_to_str(self.tcx, t_err))
});
let resolved_expected = expected_ty.map(|e_ty| {
self.resolve_type_vars_if_possible(e_ty)
});
if !resolved_expected.map_or(false, |e| { ty::type_is_error(e) }) {
match resolved_expected {
None => {
self.tcx
.sess
.span_err(sp,
format!("{}{}",
mk_msg(None, actual_ty),
error_str).as_slice())
}
Some(e) => {
self.tcx.sess.span_err(sp,
format!("{}{}",
mk_msg(Some(self.ty_to_string(e)), actual_ty),
error_str).as_slice());

match resolved_expected {
Some(t) if ty::type_is_error(t) => (),
_ => {
let error_str = err.map_or("".to_string(), |t_err| {
format!(" ({})", ty::type_err_to_str(self.tcx, t_err))
});

self.tcx.sess.span_err(sp, format!("{}{}",
mk_msg(resolved_expected.map(|t| self.ty_to_string(t)), actual_ty),
error_str).as_slice());

for err in err.iter() {
ty::note_and_explain_type_err(self.tcx, *err)
}
}
for err in err.iter() {
ty::note_and_explain_type_err(self.tcx, *err)
}
}
}

Expand All @@ -945,25 +939,18 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}

pub fn report_mismatched_types(&self,
sp: Span,
e: ty::t,
a: ty::t,
span: Span,
expected: ty::t,
actual: ty::t,
err: &ty::type_err) {
let resolved_expected =
self.resolve_type_vars_if_possible(e);
let mk_msg = match ty::get(resolved_expected).sty {
// Don't report an error if expected is ty_err
ty::ty_err => return,
_ => {
// if I leave out : String, it infers &str and complains
|actual: String| {
format!("mismatched types: expected `{}`, found `{}`",
self.ty_to_string(resolved_expected),
actual)
}
}
let trace = TypeTrace {
origin: Misc(span),
values: Types(ty::expected_found {
expected: expected,
found: actual
})
};
self.type_error_message(sp, mk_msg, a, Some(err));
self.report_and_explain_type_error(trace, err);
}

pub fn replace_late_bound_regions_with_fresh_regions(&self,
Expand Down

0 comments on commit 82045ca

Please sign in to comment.