Auto merge of #66682 - estebank:fn-type-err, r=davidtwco

Highlight parts of fn in type errors

When a type error arises between two fn items, fn pointers or tuples,
highlight only the differing parts of each.

Examples:

<img width="699" alt="" src="https://user-images.githubusercontent.com/1606434/69487597-ab561600-0e11-11ea-9b4e-d4fd9e91d5dc.png">
<img width="528" alt="" src="https://user-images.githubusercontent.com/1606434/69487207-9033d800-0e0a-11ea-93e3-8c4d002411a5.png">
<img width="468" alt="" src="https://user-images.githubusercontent.com/1606434/69487208-9033d800-0e0a-11ea-92e3-2b2cee120335.png">
<img width="775" alt="" src="https://user-images.githubusercontent.com/1606434/69487209-9033d800-0e0a-11ea-9e68-7f6ed5c8cb08.png">

Author: bors

src/librustc/infer/error_reporting/mod.rs

@@ -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;
@@ -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(">");
@@ -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);
+        }
+        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) {
@@ -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 {
@@ -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)) => {
+                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.

src/librustc/ty/print/pretty.rs

@@ -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};
 
@@ -1081,7 +1082,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 {
@@ -1104,11 +1105,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
     }
 }
@@ -1521,7 +1525,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>,
     {
@@ -1554,8 +1561,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 {
@@ -1577,12 +1583,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)

src/librustc_errors/diagnostic.rs

@@ -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())])
     }

src/test/ui/borrowck/regions-bound-missing-bound-in-impl.stderr

@@ -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
    |

src/test/ui/c-variadic/variadic-ffi-1.stderr

@@ -30,8 +30,8 @@ LL |         let x: unsafe extern "C" fn(f: isize, x: u8) = foo;
    |                |
    |                expected due to this
    |
-   = 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
@@ -41,8 +41,8 @@ LL |         let y: extern "C" fn(f: isize, x: u8, ...) = bar;
    |                |
    |                expected due to this
    |
-   = 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

src/test/ui/const-generics/fn-const-param-infer.stderr

@@ -23,8 +23,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

src/test/ui/fn/fn-item-type.rs

@@ -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>);
@@ -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.