Skip to content

Commit afb300d

Browse files
authored
Rollup merge of rust-lang#41205 - estebank:shorter-mismatched-types-2, 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
2 parents 5c23e70 + 2389830 commit afb300d

File tree

11 files changed

+485
-32
lines changed

11 files changed

+485
-32
lines changed

src/librustc/infer/error_reporting/mod.rs

+279-9
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ use ty::{self, TyCtxt, TypeFoldable};
7070
use ty::{Region, Issue32330};
7171
use ty::error::TypeError;
7272
use syntax_pos::{Pos, Span};
73-
use errors::DiagnosticBuilder;
73+
use errors::{DiagnosticBuilder, DiagnosticStyledString};
7474

7575
mod note;
7676

@@ -365,6 +365,262 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
365365
}
366366
}
367367

368+
/// Given that `other_ty` is the same as a type argument for `name` in `sub`, populate `value`
369+
/// highlighting `name` and every type argument that isn't at `pos` (which is `other_ty`), and
370+
/// populate `other_value` with `other_ty`.
371+
///
372+
/// ```text
373+
/// Foo<Bar<Qux>>
374+
/// ^^^^--------^ this is highlighted
375+
/// | |
376+
/// | this type argument is exactly the same as the other type, not highlighted
377+
/// this is highlighted
378+
/// Bar<Qux>
379+
/// -------- this type is the same as a type argument in the other type, not highlighted
380+
/// ```
381+
fn highlight_outer(&self,
382+
mut value: &mut DiagnosticStyledString,
383+
mut other_value: &mut DiagnosticStyledString,
384+
name: String,
385+
sub: &ty::subst::Substs<'tcx>,
386+
pos: usize,
387+
other_ty: &ty::Ty<'tcx>) {
388+
// `value` and `other_value` hold two incomplete type representation for display.
389+
// `name` is the path of both types being compared. `sub`
390+
value.push_highlighted(name);
391+
let len = sub.len();
392+
if len > 0 {
393+
value.push_highlighted("<");
394+
}
395+
396+
// Output the lifetimes fot the first type
397+
let lifetimes = sub.regions().map(|lifetime| {
398+
let s = format!("{}", lifetime);
399+
if s.is_empty() {
400+
"'_".to_string()
401+
} else {
402+
s
403+
}
404+
}).collect::<Vec<_>>().join(", ");
405+
if !lifetimes.is_empty() {
406+
if sub.regions().count() < len {
407+
value.push_normal(lifetimes + &", ");
408+
} else {
409+
value.push_normal(lifetimes);
410+
}
411+
}
412+
413+
// Highlight all the type arguments that aren't at `pos` and compare the type argument at
414+
// `pos` and `other_ty`.
415+
for (i, type_arg) in sub.types().enumerate() {
416+
if i == pos {
417+
let values = self.cmp(type_arg, other_ty);
418+
value.0.extend((values.0).0);
419+
other_value.0.extend((values.1).0);
420+
} else {
421+
value.push_highlighted(format!("{}", type_arg));
422+
}
423+
424+
if len > 0 && i != len - 1 {
425+
value.push_normal(", ");
426+
}
427+
//self.push_comma(&mut value, &mut other_value, len, i);
428+
}
429+
if len > 0 {
430+
value.push_highlighted(">");
431+
}
432+
}
433+
434+
/// If `other_ty` is the same as a type argument present in `sub`, highlight `path` in `t1_out`,
435+
/// as that is the difference to the other type.
436+
///
437+
/// For the following code:
438+
///
439+
/// ```norun
440+
/// let x: Foo<Bar<Qux>> = foo::<Bar<Qux>>();
441+
/// ```
442+
///
443+
/// The type error output will behave in the following way:
444+
///
445+
/// ```text
446+
/// Foo<Bar<Qux>>
447+
/// ^^^^--------^ this is highlighted
448+
/// | |
449+
/// | this type argument is exactly the same as the other type, not highlighted
450+
/// this is highlighted
451+
/// Bar<Qux>
452+
/// -------- this type is the same as a type argument in the other type, not highlighted
453+
/// ```
454+
fn cmp_type_arg(&self,
455+
mut t1_out: &mut DiagnosticStyledString,
456+
mut t2_out: &mut DiagnosticStyledString,
457+
path: String,
458+
sub: &ty::subst::Substs<'tcx>,
459+
other_path: String,
460+
other_ty: &ty::Ty<'tcx>) -> Option<()> {
461+
for (i, ta) in sub.types().enumerate() {
462+
if &ta == other_ty {
463+
self.highlight_outer(&mut t1_out, &mut t2_out, path, sub, i, &other_ty);
464+
return Some(());
465+
}
466+
if let &ty::TyAdt(def, _) = &ta.sty {
467+
let path_ = self.tcx.item_path_str(def.did.clone());
468+
if path_ == other_path {
469+
self.highlight_outer(&mut t1_out, &mut t2_out, path, sub, i, &other_ty);
470+
return Some(());
471+
}
472+
}
473+
}
474+
None
475+
}
476+
477+
/// Add a `,` to the type representation only if it is appropriate.
478+
fn push_comma(&self,
479+
value: &mut DiagnosticStyledString,
480+
other_value: &mut DiagnosticStyledString,
481+
len: usize,
482+
pos: usize) {
483+
if len > 0 && pos != len - 1 {
484+
value.push_normal(", ");
485+
other_value.push_normal(", ");
486+
}
487+
}
488+
489+
/// Compare two given types, eliding parts that are the same between them and highlighting
490+
/// relevant differences, and return two representation of those types for highlighted printing.
491+
fn cmp(&self, t1: ty::Ty<'tcx>, t2: ty::Ty<'tcx>)
492+
-> (DiagnosticStyledString, DiagnosticStyledString)
493+
{
494+
match (&t1.sty, &t2.sty) {
495+
(&ty::TyAdt(def1, sub1), &ty::TyAdt(def2, sub2)) => {
496+
let mut values = (DiagnosticStyledString::new(), DiagnosticStyledString::new());
497+
let path1 = self.tcx.item_path_str(def1.did.clone());
498+
let path2 = self.tcx.item_path_str(def2.did.clone());
499+
if def1.did == def2.did {
500+
// Easy case. Replace same types with `_` to shorten the output and highlight
501+
// the differing ones.
502+
// let x: Foo<Bar, Qux> = y::<Foo<Quz, Qux>>();
503+
// Foo<Bar, _>
504+
// Foo<Quz, _>
505+
// --- ^ type argument elided
506+
// |
507+
// highlighted in output
508+
values.0.push_normal(path1);
509+
values.1.push_normal(path2);
510+
511+
// Only draw `<...>` if there're lifetime/type arguments.
512+
let len = sub1.len();
513+
if len > 0 {
514+
values.0.push_normal("<");
515+
values.1.push_normal("<");
516+
}
517+
518+
fn lifetime_display(lifetime: &Region) -> String {
519+
let s = format!("{}", lifetime);
520+
if s.is_empty() {
521+
"'_".to_string()
522+
} else {
523+
s
524+
}
525+
}
526+
// At one point we'd like to elide all lifetimes here, they are irrelevant for
527+
// all diagnostics that use this output
528+
//
529+
// Foo<'x, '_, Bar>
530+
// Foo<'y, '_, Qux>
531+
// ^^ ^^ --- type arguments are not elided
532+
// | |
533+
// | elided as they were the same
534+
// not elided, they were different, but irrelevant
535+
let lifetimes = sub1.regions().zip(sub2.regions());
536+
for (i, lifetimes) in lifetimes.enumerate() {
537+
let l1 = lifetime_display(lifetimes.0);
538+
let l2 = lifetime_display(lifetimes.1);
539+
if l1 == l2 {
540+
values.0.push_normal("'_");
541+
values.1.push_normal("'_");
542+
} else {
543+
values.0.push_highlighted(l1);
544+
values.1.push_highlighted(l2);
545+
}
546+
self.push_comma(&mut values.0, &mut values.1, len, i);
547+
}
548+
549+
// We're comparing two types with the same path, so we compare the type
550+
// arguments for both. If they are the same, do not highlight and elide from the
551+
// output.
552+
// Foo<_, Bar>
553+
// Foo<_, Qux>
554+
// ^ elided type as this type argument was the same in both sides
555+
let type_arguments = sub1.types().zip(sub2.types());
556+
let regions_len = sub1.regions().collect::<Vec<_>>().len();
557+
for (i, (ta1, ta2)) in type_arguments.enumerate() {
558+
let i = i + regions_len;
559+
if ta1 == ta2 {
560+
values.0.push_normal("_");
561+
values.1.push_normal("_");
562+
} else {
563+
let (x1, x2) = self.cmp(ta1, ta2);
564+
(values.0).0.extend(x1.0);
565+
(values.1).0.extend(x2.0);
566+
}
567+
self.push_comma(&mut values.0, &mut values.1, len, i);
568+
}
569+
570+
// Close the type argument bracket.
571+
// Only draw `<...>` if there're lifetime/type arguments.
572+
if len > 0 {
573+
values.0.push_normal(">");
574+
values.1.push_normal(">");
575+
}
576+
values
577+
} else {
578+
// Check for case:
579+
// let x: Foo<Bar<Qux> = foo::<Bar<Qux>>();
580+
// Foo<Bar<Qux>
581+
// ------- this type argument is exactly the same as the other type
582+
// Bar<Qux>
583+
if self.cmp_type_arg(&mut values.0,
584+
&mut values.1,
585+
path1.clone(),
586+
sub1,
587+
path2.clone(),
588+
&t2).is_some() {
589+
return values;
590+
}
591+
// Check for case:
592+
// let x: Bar<Qux> = y:<Foo<Bar<Qux>>>();
593+
// Bar<Qux>
594+
// Foo<Bar<Qux>>
595+
// ------- this type argument is exactly the same as the other type
596+
if self.cmp_type_arg(&mut values.1,
597+
&mut values.0,
598+
path2,
599+
sub2,
600+
path1,
601+
&t1).is_some() {
602+
return values;
603+
}
604+
605+
// We couldn't find anything in common, highlight everything.
606+
// let x: Bar<Qux> = y::<Foo<Zar>>();
607+
(DiagnosticStyledString::highlighted(format!("{}", t1)),
608+
DiagnosticStyledString::highlighted(format!("{}", t2)))
609+
}
610+
}
611+
_ => {
612+
if t1 == t2 {
613+
// The two types are the same, elide and don't highlight.
614+
(DiagnosticStyledString::normal("_"), DiagnosticStyledString::normal("_"))
615+
} else {
616+
// We couldn't find anything in common, highlight everything.
617+
(DiagnosticStyledString::highlighted(format!("{}", t1)),
618+
DiagnosticStyledString::highlighted(format!("{}", t2)))
619+
}
620+
}
621+
}
622+
}
623+
368624
pub fn note_type_err(&self,
369625
diag: &mut DiagnosticBuilder<'tcx>,
370626
cause: &ObligationCause<'tcx>,
@@ -397,14 +653,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
397653

398654
if let Some((expected, found)) = expected_found {
399655
match (terr, is_simple_error, expected == found) {
400-
(&TypeError::Sorts(ref values), false, true) => {
656+
(&TypeError::Sorts(ref values), false, true) => {
401657
diag.note_expected_found_extra(
402-
&"type", &expected, &found,
658+
&"type", expected, found,
403659
&format!(" ({})", values.expected.sort_string(self.tcx)),
404660
&format!(" ({})", values.found.sort_string(self.tcx)));
405661
}
406662
(_, false, _) => {
407-
diag.note_expected_found(&"type", &expected, &found);
663+
diag.note_expected_found(&"type", expected, found);
408664
}
409665
_ => (),
410666
}
@@ -472,26 +728,40 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
472728
diag
473729
}
474730

475-
/// Returns a string of the form "expected `{}`, found `{}`".
476-
fn values_str(&self, values: &ValuePairs<'tcx>) -> Option<(String, String)> {
731+
fn values_str(&self, values: &ValuePairs<'tcx>)
732+
-> Option<(DiagnosticStyledString, DiagnosticStyledString)>
733+
{
477734
match *values {
478-
infer::Types(ref exp_found) => self.expected_found_str(exp_found),
735+
infer::Types(ref exp_found) => self.expected_found_str_ty(exp_found),
479736
infer::TraitRefs(ref exp_found) => self.expected_found_str(exp_found),
480737
infer::PolyTraitRefs(ref exp_found) => self.expected_found_str(exp_found),
481738
}
482739
}
483740

741+
fn expected_found_str_ty(&self,
742+
exp_found: &ty::error::ExpectedFound<ty::Ty<'tcx>>)
743+
-> Option<(DiagnosticStyledString, DiagnosticStyledString)> {
744+
let exp_found = self.resolve_type_vars_if_possible(exp_found);
745+
if exp_found.references_error() {
746+
return None;
747+
}
748+
749+
Some(self.cmp(exp_found.expected, exp_found.found))
750+
}
751+
752+
/// Returns a string of the form "expected `{}`, found `{}`".
484753
fn expected_found_str<T: fmt::Display + TypeFoldable<'tcx>>(
485754
&self,
486755
exp_found: &ty::error::ExpectedFound<T>)
487-
-> Option<(String, String)>
756+
-> Option<(DiagnosticStyledString, DiagnosticStyledString)>
488757
{
489758
let exp_found = self.resolve_type_vars_if_possible(exp_found);
490759
if exp_found.references_error() {
491760
return None;
492761
}
493762

494-
Some((format!("{}", exp_found.expected), format!("{}", exp_found.found)))
763+
Some((DiagnosticStyledString::highlighted(format!("{}", exp_found.expected)),
764+
DiagnosticStyledString::highlighted(format!("{}", exp_found.found))))
495765
}
496766

497767
fn report_generic_bound_failure(&self,

src/librustc/infer/error_reporting/note.rs

+2
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
2020
match *origin {
2121
infer::Subtype(ref trace) => {
2222
if let Some((expected, found)) = self.values_str(&trace.values) {
23+
let expected = expected.content();
24+
let found = found.content();
2325
// FIXME: do we want a "the" here?
2426
err.span_note(trace.cause.span,
2527
&format!("...so that {} (expected {}, found {})",

src/librustc/ty/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2241,7 +2241,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
22412241
/// `DefId` is really just an interned def-path).
22422242
///
22432243
/// Note that if `id` is not local to this crate, the result will
2244-
// be a non-local `DefPath`.
2244+
/// be a non-local `DefPath`.
22452245
pub fn def_path(self, id: DefId) -> hir_map::DefPath {
22462246
if id.is_local() {
22472247
self.hir.def_path(id)

0 commit comments

Comments
 (0)