Skip to content

Highlight parts of fn in type errors #66682

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.

Sign up for GitHub

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 2 commits into from
Nov 25, 2019
Merged

Highlight parts of fn in type errors #66682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d92355c
Highlight parts of fn in type errors
estebank Nov 24, 2019
9d7774c
review comments: remove unnecessary `&str` to `String` conversions
estebank Nov 24, 2019
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
177 changes: 174 additions & 3 deletions src/librustc/infer/error_reporting/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ use crate::ty::{self, subst::{Subst, SubstsRef}, Region, Ty, TyCtxt, TypeFoldabl

use errors::{Applicability, DiagnosticBuilder, DiagnosticStyledString};
use rustc_error_codes::*;
use rustc_target::spec::abi;
use syntax_pos::{Pos, Span};

use std::{cmp, fmt};

mod note;
Expand Down Expand Up @@ -766,7 +766,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
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(">");
Expand Down Expand Up @@ -868,6 +867,120 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
substs.truncate_to(self.tcx, &generics)
}

/// Given two `fn` signatures highlight only sub-parts that are different.
fn cmp_fn_sig(
&self,
sig1: &ty::PolyFnSig<'tcx>,
sig2: &ty::PolyFnSig<'tcx>,
) -> (DiagnosticStyledString, DiagnosticStyledString) {
let get_lifetimes = |sig| {
use crate::hir::def::Namespace;
let mut s = String::new();
let (_, (sig, reg)) = ty::print::FmtPrinter::new(self.tcx, &mut s, Namespace::TypeNS)
.name_all_regions(sig)
.unwrap();
let lts: Vec<String> = reg.into_iter().map(|(_, kind)| kind.to_string()).collect();
(if lts.is_empty() {
String::new()
} else {
format!("for<{}> ", lts.join(", "))
}, sig)
};

let (lt1, sig1) = get_lifetimes(sig1);
let (lt2, sig2) = get_lifetimes(sig2);

// unsafe extern "C" for<'a> fn(&'a T) -> &'a T
let mut values = (
DiagnosticStyledString::normal("".to_string()),
DiagnosticStyledString::normal("".to_string()),
);

// unsafe extern "C" for<'a> fn(&'a T) -> &'a T
// ^^^^^^
values.0.push(sig1.unsafety.prefix_str(), sig1.unsafety != sig2.unsafety);
values.1.push(sig2.unsafety.prefix_str(), sig1.unsafety != sig2.unsafety);

// unsafe extern "C" for<'a> fn(&'a T) -> &'a T
// ^^^^^^^^^^
if sig1.abi != abi::Abi::Rust {
values.0.push(format!("extern {} ", sig1.abi), sig1.abi != sig2.abi);
}
if sig2.abi != abi::Abi::Rust {
values.1.push(format!("extern {} ", sig2.abi), sig1.abi != sig2.abi);
}

// unsafe extern "C" for<'a> fn(&'a T) -> &'a T
// ^^^^^^^^
let lifetime_diff = lt1 != lt2;
values.0.push(lt1, lifetime_diff);
values.1.push(lt2, lifetime_diff);

// unsafe extern "C" for<'a> fn(&'a T) -> &'a T
// ^^^
values.0.push_normal("fn(");
values.1.push_normal("fn(");

// unsafe extern "C" for<'a> fn(&'a T) -> &'a T
// ^^^^^
let len1 = sig1.inputs().len();
let len2 = sig2.inputs().len();
if len1 == len2 {
for (i, (l, r)) in sig1.inputs().iter().zip(sig2.inputs().iter()).enumerate() {
let (x1, x2) = self.cmp(l, r);
(values.0).0.extend(x1.0);
(values.1).0.extend(x2.0);
self.push_comma(&mut values.0, &mut values.1, len1, i);
}
} else {
for (i, l) in sig1.inputs().iter().enumerate() {
values.0.push_highlighted(l.to_string());
if i != len1 - 1 {
values.0.push_highlighted(", ");
}
}
for (i, r) in sig2.inputs().iter().enumerate() {
values.1.push_highlighted(r.to_string());
if i != len2 - 1 {
values.1.push_highlighted(", ");
}
}
}

if sig1.c_variadic {
if len1 > 0 {
values.0.push_normal(", ");
}
values.0.push("...", !sig2.c_variadic);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like a lot of things are repeated twice in this method; could we try to rewrite things so that it isn't repeated by computing the diffs first and then doing all the logic for a single case?

if sig2.c_variadic {
if len2 > 0 {
values.1.push_normal(", ");
}
values.1.push("...", !sig1.c_variadic);
}

// unsafe extern "C" for<'a> fn(&'a T) -> &'a T
// ^
values.0.push_normal(")");
values.1.push_normal(")");

// unsafe extern "C" for<'a> fn(&'a T) -> &'a T
// ^^^^^^^^
let output1 = sig1.output();
let output2 = sig2.output();
let (x1, x2) = self.cmp(output1, output2);
if !output1.is_unit() {
values.0.push_normal(" -> ");
(values.0).0.extend(x1.0);
}
if !output2.is_unit() {
values.1.push_normal(" -> ");
(values.1).0.extend(x2.0);
}
values
}

/// Compares 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<'tcx>, t2: Ty<'tcx>) -> (DiagnosticStyledString, DiagnosticStyledString) {
Expand Down Expand Up @@ -968,7 +1081,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
for (i, lifetimes) in lifetimes.enumerate() {
let l1 = lifetime_display(lifetimes.0);
let l2 = lifetime_display(lifetimes.1);
if l1 == l2 {
if lifetimes.0 == lifetimes.1 {
values.0.push_normal("'_");
values.1.push_normal("'_");
} else {
Expand Down Expand Up @@ -1124,6 +1237,64 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
values
}

// When encountering tuples of the same size, highlight only the differing types
(&ty::Tuple(substs1), &ty::Tuple(substs2)) if substs1.len() == substs2.len() => {
let mut values = (
DiagnosticStyledString::normal("("),
DiagnosticStyledString::normal("("),
);
let len = substs1.len();
for (i, (left, right)) in substs1.types().zip(substs2.types()).enumerate() {
let (x1, x2) = self.cmp(left, right);
(values.0).0.extend(x1.0);
(values.1).0.extend(x2.0);
self.push_comma(&mut values.0, &mut values.1, len, i);
}
if len == 1 { // Keep the output for single element tuples as `(ty,)`.
values.0.push_normal(",");
values.1.push_normal(",");
}
values.0.push_normal(")");
values.1.push_normal(")");
values
}

(ty::FnDef(did1, substs1), ty::FnDef(did2, substs2)) => {
let sig1 = self.tcx.fn_sig(*did1).subst(self.tcx, substs1);
let sig2 = self.tcx.fn_sig(*did2).subst(self.tcx, substs2);
let mut values = self.cmp_fn_sig(&sig1, &sig2);
let path1 = format!(" {{{}}}", self.tcx.def_path_str_with_substs(*did1, substs1));
let path2 = format!(" {{{}}}", self.tcx.def_path_str_with_substs(*did2, substs2));
let same_path = path1 == path2;
values.0.push(path1, !same_path);
values.1.push(path2, !same_path);
values
}

(ty::FnDef(did1, substs1), ty::FnPtr(sig2)) => {
let sig1 = self.tcx.fn_sig(*did1).subst(self.tcx, substs1);
let mut values = self.cmp_fn_sig(&sig1, sig2);
values.0.push_normal(format!(
" {{{}}}",
self.tcx.def_path_str_with_substs(*did1, substs1)),
);
values
}

(ty::FnPtr(sig1), ty::FnDef(did2, substs2)) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

btw, this method is some 325+ LOC... would be good to split it up

let sig2 = self.tcx.fn_sig(*did2).subst(self.tcx, substs2);
let mut values = self.cmp_fn_sig(sig1, &sig2);
values.1.push_normal(format!(
" {{{}}}",
self.tcx.def_path_str_with_substs(*did2, substs2)),
);
values
}

(ty::FnPtr(sig1), ty::FnPtr(sig2)) => {
self.cmp_fn_sig(sig1, sig2)
}

_ => {
if t1 == t2 {
// The two types are the same, elide and don't highlight.
Expand Down
31 changes: 23 additions & 8 deletions src/librustc/ty/print/pretty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use syntax::attr::{SignedInt, UnsignedInt};
use syntax::symbol::{kw, Symbol};

use std::cell::Cell;
use std::collections::BTreeMap;
use std::fmt::{self, Write as _};
use std::ops::{Deref, DerefMut};

Expand Down Expand Up @@ -1054,7 +1055,7 @@ impl<F> FmtPrinter<'a, 'tcx, F> {
}
}

impl TyCtxt<'_> {
impl TyCtxt<'t> {
// HACK(eddyb) get rid of `def_path_str` and/or pass `Namespace` explicitly always
// (but also some things just print a `DefId` generally so maybe we need this?)
fn guess_def_namespace(self, def_id: DefId) -> Namespace {
Expand All @@ -1077,11 +1078,14 @@ impl TyCtxt<'_> {
/// Returns a string identifying this `DefId`. This string is
/// suitable for user output.
pub fn def_path_str(self, def_id: DefId) -> String {
self.def_path_str_with_substs(def_id, &[])
}

pub fn def_path_str_with_substs(self, def_id: DefId, substs: &'t [GenericArg<'t>]) -> String {
let ns = self.guess_def_namespace(def_id);
debug!("def_path_str: def_id={:?}, ns={:?}", def_id, ns);
let mut s = String::new();
let _ = FmtPrinter::new(self, &mut s, ns)
.print_def_path(def_id, &[]);
let _ = FmtPrinter::new(self, &mut s, ns).print_def_path(def_id, substs);
s
}
}
Expand Down Expand Up @@ -1494,7 +1498,10 @@ impl<F: fmt::Write> FmtPrinter<'_, '_, F> {
// HACK(eddyb) limited to `FmtPrinter` because of `binder_depth`,
// `region_index` and `used_region_names`.
impl<F: fmt::Write> FmtPrinter<'_, 'tcx, F> {
pub fn pretty_in_binder<T>(mut self, value: &ty::Binder<T>) -> Result<Self, fmt::Error>
pub fn name_all_regions<T>(
mut self,
value: &ty::Binder<T>,
) -> Result<(Self, (T, BTreeMap<ty::BoundRegion, ty::Region<'tcx>>)), fmt::Error>
where
T: Print<'tcx, Self, Output = Self, Error = fmt::Error> + TypeFoldable<'tcx>,
{
Expand Down Expand Up @@ -1527,8 +1534,7 @@ impl<F: fmt::Write> FmtPrinter<'_, 'tcx, F> {

define_scoped_cx!(self);

let old_region_index = self.region_index;
let mut region_index = old_region_index;
let mut region_index = self.region_index;
let new_value = self.tcx.replace_late_bound_regions(value, |br| {
let _ = start_or_continue(&mut self, "for<", ", ");
let br = match br {
Expand All @@ -1550,12 +1556,21 @@ impl<F: fmt::Write> FmtPrinter<'_, 'tcx, F> {
}
};
self.tcx.mk_region(ty::ReLateBound(ty::INNERMOST, br))
}).0;
});
start_or_continue(&mut self, "", "> ")?;

self.binder_depth += 1;
self.region_index = region_index;
let mut inner = new_value.print(self)?;
Ok((self, new_value))
}

pub fn pretty_in_binder<T>(self, value: &ty::Binder<T>) -> Result<Self, fmt::Error>
where
T: Print<'tcx, Self, Output = Self, Error = fmt::Error> + TypeFoldable<'tcx>,
{
let old_region_index = self.region_index;
let (new, new_value) = self.name_all_regions(value)?;
let mut inner = new_value.0.print(new)?;
inner.region_index = old_region_index;
inner.binder_depth -= 1;
Ok(inner)
Expand Down
7 changes: 7 additions & 0 deletions src/librustc_errors/diagnostic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ impl DiagnosticStyledString {
pub fn push_highlighted<S: Into<String>>(&mut self, t: S) {
self.0.push(StringPart::Highlighted(t.into()));
}
pub fn push<S: Into<String>>(&mut self, t: S, highlight: bool) {
if highlight {
self.push_highlighted(t);
} else {
self.push_normal(t);
}
}
pub fn normal<S: Into<String>>(t: S) -> DiagnosticStyledString {
DiagnosticStyledString(vec![StringPart::Normal(t.into())])
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/borrowck/regions-bound-missing-bound-in-impl.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ error[E0308]: method not compatible with trait
LL | fn wrong_bound1<'b,'c,'d:'a+'c>(self, b: Inv<'b>, c: Inv<'c>, d: Inv<'d>) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ lifetime mismatch
|
= note: expected fn pointer `fn(&'a isize, Inv<'c>, Inv<'c>, Inv<'d>)`
found fn pointer `fn(&'a isize, Inv<'_>, Inv<'c>, Inv<'d>)`
= note: expected fn pointer `fn(&'a isize, Inv<'c>, Inv<'c>, Inv<'_>)`
found fn pointer `fn(&'a isize, Inv<'_>, Inv<'c>, Inv<'_>)`
note: the lifetime `'c` as defined on the method body at 27:24...
--> $DIR/regions-bound-missing-bound-in-impl.rs:27:24
|
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/c-variadic/variadic-ffi-1.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,17 @@ error[E0308]: mismatched types
LL | let x: unsafe extern "C" fn(f: isize, x: u8) = foo;
| ^^^ expected non-variadic fn, found variadic function
|
= note: expected fn pointer `unsafe extern "C" fn(isize, u8)`
found fn item `unsafe extern "C" fn(isize, u8, ...) {foo}`
= note: expected fn pointer `unsafe extern "C" fn(_, _)`
found fn item `unsafe extern "C" fn(_, _, ...) {foo}`

error[E0308]: mismatched types
--> $DIR/variadic-ffi-1.rs:20:54
|
LL | let y: extern "C" fn(f: isize, x: u8, ...) = bar;
| ^^^ expected variadic fn, found non-variadic function
|
= note: expected fn pointer `extern "C" fn(isize, u8, ...)`
found fn item `extern "C" fn(isize, u8) {bar}`
= note: expected fn pointer `extern "C" fn(_, _, ...)`
found fn item `extern "C" fn(_, _) {bar}`

error[E0617]: can't pass `f32` to variadic function
--> $DIR/variadic-ffi-1.rs:22:19
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/const-generics/fn-const-param-infer.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ error[E0308]: mismatched types
LL | let _ = Checked::<{generic_arg::<u32>}>;
| ^^^^^^^^^^^^^^^^^^ expected `usize`, found `u32`
|
= note: expected fn pointer `fn(usize) -> bool`
found fn item `fn(u32) -> bool {generic_arg::<u32>}`
= note: expected fn pointer `fn(usize) -> _`
found fn item `fn(u32) -> _ {generic_arg::<u32>}`

error[E0282]: type annotations needed
--> $DIR/fn-const-param-infer.rs:22:23
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/fn/fn-item-type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ impl<T> Foo for T { /* `foo` is still default here */ }
fn main() {
eq(foo::<u8>, bar::<u8>);
//~^ ERROR mismatched types
//~| expected fn item `fn(isize) -> isize {foo::<u8>}`
//~| found fn item `fn(isize) -> isize {bar::<u8>}`
//~| expected fn item `fn(_) -> _ {foo::<u8>}`
//~| found fn item `fn(_) -> _ {bar::<u8>}`
//~| expected fn item, found a different fn item

eq(foo::<u8>, foo::<i8>);
Expand All @@ -22,8 +22,8 @@ fn main() {

eq(bar::<String>, bar::<Vec<u8>>);
//~^ ERROR mismatched types
//~| expected fn item `fn(isize) -> isize {bar::<std::string::String>}`
//~| found fn item `fn(isize) -> isize {bar::<std::vec::Vec<u8>>}`
//~| expected fn item `fn(_) -> _ {bar::<std::string::String>}`
//~| found fn item `fn(_) -> _ {bar::<std::vec::Vec<u8>>}`
//~| expected struct `std::string::String`, found struct `std::vec::Vec`

// Make sure we distinguish between trait methods correctly.
Expand Down
Loading