Skip to content

Commit

Permalink
Rollup merge of rust-lang#41205 - estebank:shorter-mismatched-types-2…
Browse files Browse the repository at this point in the history 
…, r=nikomatsakis

Highlight and simplify mismatched types

Shorten mismatched types errors by replacing subtypes that are not
different with `_`, and highlighting only the subtypes that are
different.

Given a file

```rust
struct X<T1, T2> {
    x: T1,
    y: T2,
}

fn foo() -> X<X<String, String>, String> {
    X { x: X {x: "".to_string(), y: 2}, y: "".to_string()}
}

fn bar() -> Option<String> {
    "".to_string()
}
```

provide the following output

```rust
error[E0308]: mismatched types
  --> file.rs:6:5
   |
 6 |     X { x: X {x: "".to_string(), y: 2}, y: "".to_string()}
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `std::string::String`, found {integer}
   |
   = note: expected type `X<X<_, std::string::String>, _>`
                                 ^^^^^^^^^^^^^^^^^^^   // < highlighted
              found type `X<X<_, {integer}>, _>`
                                 ^^^^^^^^^             // < highlighted

error[E0308]: mismatched types
  --> file.rs:6:5
   |
10 |     "".to_string()
   |     ^^^^^^^^^^^^^^ expected struct `std::option::Option`, found `std::string::String`
   |
   = note: expected type `Option<std::string::String>`
                          ^^^^^^^                   ^  // < highlighted
              found type `std::string::String`
```

Fix rust-lang#21025. Re: rust-lang#40186. Follow up to rust-lang#39906.

I'm looking to change how this output is accomplished so that it doesn't create list of strings to pass around, but rather add an elided `Ty` placeholder, and use the same string formatting for normal types. I'll be doing that soonish.

r? @nikomatsakis
  • Loading branch information
@TimNN
TimNN committed Apr 12, 2017
2 parents 5c23e70 + 2389830 commit afb300d
Show file tree
Hide file tree
Showing 11 changed files with 485 additions and 32 deletions.
288 changes: 279 additions & 9 deletions src/librustc/infer/error_reporting/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ use ty::{self, TyCtxt, TypeFoldable};
use ty::{Region, Issue32330};
use ty::error::TypeError;
use syntax_pos::{Pos, Span};
use errors::DiagnosticBuilder;
use errors::{DiagnosticBuilder, DiagnosticStyledString};

mod note;

Expand Down Expand Up @@ -365,6 +365,262 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}
}

/// Given that `other_ty` is the same as a type argument for `name` in `sub`, populate `value`
/// highlighting `name` and every type argument that isn't at `pos` (which is `other_ty`), and
/// populate `other_value` with `other_ty`.
///
/// ```text
/// Foo<Bar<Qux>>
/// ^^^^--------^ this is highlighted
/// | |
/// | this type argument is exactly the same as the other type, not highlighted
/// this is highlighted
/// Bar<Qux>
/// -------- this type is the same as a type argument in the other type, not highlighted
/// ```
fn highlight_outer(&self,
mut value: &mut DiagnosticStyledString,
mut other_value: &mut DiagnosticStyledString,
name: String,
sub: &ty::subst::Substs<'tcx>,
pos: usize,
other_ty: &ty::Ty<'tcx>) {
// `value` and `other_value` hold two incomplete type representation for display.
// `name` is the path of both types being compared. `sub`
value.push_highlighted(name);
let len = sub.len();
if len > 0 {
value.push_highlighted("<");
}

// Output the lifetimes fot the first type
let lifetimes = sub.regions().map(|lifetime| {
let s = format!("{}", lifetime);
if s.is_empty() {
"'_".to_string()
} else {
s
}
}).collect::<Vec<_>>().join(", ");
if !lifetimes.is_empty() {
if sub.regions().count() < len {
value.push_normal(lifetimes + &", ");
} else {
value.push_normal(lifetimes);
}
}

// Highlight all the type arguments that aren't at `pos` and compare the type argument at
// `pos` and `other_ty`.
for (i, type_arg) in sub.types().enumerate() {
if i == pos {
let values = self.cmp(type_arg, other_ty);
value.0.extend((values.0).0);
other_value.0.extend((values.1).0);
} else {
value.push_highlighted(format!("{}", type_arg));
}

if len > 0 && i != len - 1 {
value.push_normal(", ");
}
//self.push_comma(&mut value, &mut other_value, len, i);
}
if len > 0 {
value.push_highlighted(">");
}
}

/// If `other_ty` is the same as a type argument present in `sub`, highlight `path` in `t1_out`,
/// as that is the difference to the other type.
///
/// For the following code:
///
/// ```norun
/// let x: Foo<Bar<Qux>> = foo::<Bar<Qux>>();
/// ```
///
/// The type error output will behave in the following way:
///
/// ```text
/// Foo<Bar<Qux>>
/// ^^^^--------^ this is highlighted
/// | |
/// | this type argument is exactly the same as the other type, not highlighted
/// this is highlighted
/// Bar<Qux>
/// -------- this type is the same as a type argument in the other type, not highlighted
/// ```
fn cmp_type_arg(&self,
mut t1_out: &mut DiagnosticStyledString,
mut t2_out: &mut DiagnosticStyledString,
path: String,
sub: &ty::subst::Substs<'tcx>,
other_path: String,
other_ty: &ty::Ty<'tcx>) -> Option<()> {
for (i, ta) in sub.types().enumerate() {
if &ta == other_ty {
self.highlight_outer(&mut t1_out, &mut t2_out, path, sub, i, &other_ty);
return Some(());
}
if let &ty::TyAdt(def, _) = &ta.sty {
let path_ = self.tcx.item_path_str(def.did.clone());
if path_ == other_path {
self.highlight_outer(&mut t1_out, &mut t2_out, path, sub, i, &other_ty);
return Some(());
}
}
}
None
}

/// Add a `,` to the type representation only if it is appropriate.
fn push_comma(&self,
value: &mut DiagnosticStyledString,
other_value: &mut DiagnosticStyledString,
len: usize,
pos: usize) {
if len > 0 && pos != len - 1 {
value.push_normal(", ");
other_value.push_normal(", ");
}
}

/// Compare two given types, eliding parts that are the same between them and highlighting
/// relevant differences, and return two representation of those types for highlighted printing.
fn cmp(&self, t1: ty::Ty<'tcx>, t2: ty::Ty<'tcx>)
-> (DiagnosticStyledString, DiagnosticStyledString)
{
match (&t1.sty, &t2.sty) {
(&ty::TyAdt(def1, sub1), &ty::TyAdt(def2, sub2)) => {
let mut values = (DiagnosticStyledString::new(), DiagnosticStyledString::new());
let path1 = self.tcx.item_path_str(def1.did.clone());
let path2 = self.tcx.item_path_str(def2.did.clone());
if def1.did == def2.did {
// Easy case. Replace same types with `_` to shorten the output and highlight
// the differing ones.
// let x: Foo<Bar, Qux> = y::<Foo<Quz, Qux>>();
// Foo<Bar, _>
// Foo<Quz, _>
// --- ^ type argument elided
// |
// highlighted in output
values.0.push_normal(path1);
values.1.push_normal(path2);

// Only draw `<...>` if there're lifetime/type arguments.
let len = sub1.len();
if len > 0 {
values.0.push_normal("<");
values.1.push_normal("<");
}

fn lifetime_display(lifetime: &Region) -> String {
let s = format!("{}", lifetime);
if s.is_empty() {
"'_".to_string()
} else {
s
}
}
// At one point we'd like to elide all lifetimes here, they are irrelevant for
// all diagnostics that use this output
//
// Foo<'x, '_, Bar>
// Foo<'y, '_, Qux>
// ^^ ^^ --- type arguments are not elided
// | |
// | elided as they were the same
// not elided, they were different, but irrelevant
let lifetimes = sub1.regions().zip(sub2.regions());
for (i, lifetimes) in lifetimes.enumerate() {
let l1 = lifetime_display(lifetimes.0);
let l2 = lifetime_display(lifetimes.1);
if l1 == l2 {
values.0.push_normal("'_");
values.1.push_normal("'_");
} else {
values.0.push_highlighted(l1);
values.1.push_highlighted(l2);
}
self.push_comma(&mut values.0, &mut values.1, len, i);
}

// We're comparing two types with the same path, so we compare the type
// arguments for both. If they are the same, do not highlight and elide from the
// output.
// Foo<_, Bar>
// Foo<_, Qux>
// ^ elided type as this type argument was the same in both sides
let type_arguments = sub1.types().zip(sub2.types());
let regions_len = sub1.regions().collect::<Vec<_>>().len();
for (i, (ta1, ta2)) in type_arguments.enumerate() {
let i = i + regions_len;
if ta1 == ta2 {
values.0.push_normal("_");
values.1.push_normal("_");
} else {
let (x1, x2) = self.cmp(ta1, ta2);
(values.0).0.extend(x1.0);
(values.1).0.extend(x2.0);
}
self.push_comma(&mut values.0, &mut values.1, len, i);
}

// Close the type argument bracket.
// Only draw `<...>` if there're lifetime/type arguments.
if len > 0 {
values.0.push_normal(">");
values.1.push_normal(">");
}
values
} else {
// Check for case:
// let x: Foo<Bar<Qux> = foo::<Bar<Qux>>();
// Foo<Bar<Qux>
// ------- this type argument is exactly the same as the other type
// Bar<Qux>
if self.cmp_type_arg(&mut values.0,
&mut values.1,
path1.clone(),
sub1,
path2.clone(),
&t2).is_some() {
return values;
}
// Check for case:
// let x: Bar<Qux> = y:<Foo<Bar<Qux>>>();
// Bar<Qux>
// Foo<Bar<Qux>>
// ------- this type argument is exactly the same as the other type
if self.cmp_type_arg(&mut values.1,
&mut values.0,
path2,
sub2,
path1,
&t1).is_some() {
return values;
}

// We couldn't find anything in common, highlight everything.
// let x: Bar<Qux> = y::<Foo<Zar>>();
(DiagnosticStyledString::highlighted(format!("{}", t1)),
DiagnosticStyledString::highlighted(format!("{}", t2)))
}
}
_ => {
if t1 == t2 {
// The two types are the same, elide and don't highlight.
(DiagnosticStyledString::normal("_"), DiagnosticStyledString::normal("_"))
} else {
// We couldn't find anything in common, highlight everything.
(DiagnosticStyledString::highlighted(format!("{}", t1)),
DiagnosticStyledString::highlighted(format!("{}", t2)))
}
}
}
}

pub fn note_type_err(&self,
diag: &mut DiagnosticBuilder<'tcx>,
cause: &ObligationCause<'tcx>,
Expand Down Expand Up @@ -397,14 +653,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {

if let Some((expected, found)) = expected_found {
match (terr, is_simple_error, expected == found) {
(&TypeError::Sorts(ref values), false, true) => {
(&TypeError::Sorts(ref values), false, true) => {
diag.note_expected_found_extra(
&"type", &expected, &found,
&"type", expected, found,
&format!(" ({})", values.expected.sort_string(self.tcx)),
&format!(" ({})", values.found.sort_string(self.tcx)));
}
(_, false, _) => {
diag.note_expected_found(&"type", &expected, &found);
diag.note_expected_found(&"type", expected, found);
}
_ => (),
}
Expand Down Expand Up @@ -472,26 +728,40 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
diag
}

/// Returns a string of the form "expected `{}`, found `{}`".
fn values_str(&self, values: &ValuePairs<'tcx>) -> Option<(String, String)> {
fn values_str(&self, values: &ValuePairs<'tcx>)
-> Option<(DiagnosticStyledString, DiagnosticStyledString)>
{
match *values {
infer::Types(ref exp_found) => self.expected_found_str(exp_found),
infer::Types(ref exp_found) => self.expected_found_str_ty(exp_found),
infer::TraitRefs(ref exp_found) => self.expected_found_str(exp_found),
infer::PolyTraitRefs(ref exp_found) => self.expected_found_str(exp_found),
}
}

fn expected_found_str_ty(&self,
exp_found: &ty::error::ExpectedFound<ty::Ty<'tcx>>)
-> Option<(DiagnosticStyledString, DiagnosticStyledString)> {
let exp_found = self.resolve_type_vars_if_possible(exp_found);
if exp_found.references_error() {
return None;
}

Some(self.cmp(exp_found.expected, exp_found.found))
}

/// Returns a string of the form "expected `{}`, found `{}`".
fn expected_found_str<T: fmt::Display + TypeFoldable<'tcx>>(
&self,
exp_found: &ty::error::ExpectedFound<T>)
-> Option<(String, String)>
-> Option<(DiagnosticStyledString, DiagnosticStyledString)>
{
let exp_found = self.resolve_type_vars_if_possible(exp_found);
if exp_found.references_error() {
return None;
}

Some((format!("{}", exp_found.expected), format!("{}", exp_found.found)))
Some((DiagnosticStyledString::highlighted(format!("{}", exp_found.expected)),
DiagnosticStyledString::highlighted(format!("{}", exp_found.found))))
}

fn report_generic_bound_failure(&self,
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/infer/error_reporting/note.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
match *origin {
infer::Subtype(ref trace) => {
if let Some((expected, found)) = self.values_str(&trace.values) {
let expected = expected.content();
let found = found.content();
// FIXME: do we want a "the" here?
err.span_note(trace.cause.span,
&format!("...so that {} (expected {}, found {})",
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2241,7 +2241,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
/// `DefId` is really just an interned def-path).
///
/// Note that if `id` is not local to this crate, the result will
// be a non-local `DefPath`.
/// be a non-local `DefPath`.
pub fn def_path(self, id: DefId) -> hir_map::DefPath {
if id.is_local() {
self.hir.def_path(id)
Expand Down
Loading

0 comments on commit afb300d

Please sign in to comment.