refactor type error reporting

Author: arielb1

src/librustc/infer/error_reporting.rs

@@ -83,7 +83,7 @@ use hir::def_id::DefId;
 use infer::{self, TypeOrigin};
 use middle::region;
 use ty::subst;
-use ty::{self, Ty, TyCtxt, TypeFoldable};
+use ty::{self, TyCtxt, TypeFoldable};
 use ty::{Region, ReFree};
 use ty::error::TypeError;
 
@@ -462,52 +462,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         }
     }
 
-    fn report_type_error(&self,
-                         trace: TypeTrace<'tcx>,
-                         terr: &TypeError<'tcx>)
-                         -> DiagnosticBuilder<'tcx> {
-        let (expected, found) = match self.values_str(&trace.values) {
-            Some(v) => v,
-            None => {
-                return self.tcx.sess.diagnostic().struct_dummy(); /* derived error */
-            }
-        };
-
-        let is_simple_error = if let &TypeError::Sorts(ref values) = terr {
-            values.expected.is_primitive() && values.found.is_primitive()
-        } else {
-            false
-        };
-
-        let mut err = struct_span_err!(self.tcx.sess,
-                                       trace.origin.span(),
-                                       E0308,
-                                       "{}",
-                                       trace.origin);
-
-        if !is_simple_error || check_old_school() {
-            err.note_expected_found(&"type", &expected, &found);
-        }
-
-        err.span_label(trace.origin.span(), &terr);
-
-        self.check_and_note_conflicting_crates(&mut err, terr, trace.origin.span());
-
-        match trace.origin {
-            TypeOrigin::MatchExpressionArm(_, arm_span, source) => match source {
-                hir::MatchSource::IfLetDesugar{..} => {
-                    err.span_note(arm_span, "`if let` arm with an incompatible type");
-                }
-                _ => {
-                    err.span_note(arm_span, "match arm with an incompatible type");
-                }
-            },
-            _ => ()
-        }
-
-        err
-    }
-
     /// Adds a note if the types come from similarly named crates
     fn check_and_note_conflicting_crates(&self,
                                          err: &mut DiagnosticBuilder,
@@ -550,43 +504,91 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         }
     }
 
-    pub fn report_and_explain_type_error(&self,
-                                         trace: TypeTrace<'tcx>,
-                                         terr: &TypeError<'tcx>)
-                                         -> DiagnosticBuilder<'tcx> {
-        let trace = self.resolve_type_vars_if_possible(&trace);
+    fn note_error_origin(&self,
+                         err: &mut DiagnosticBuilder<'tcx>,
+                         origin: &TypeOrigin)
+    {
+        match origin {
+            &TypeOrigin::MatchExpressionArm(_, arm_span, source) => match source {
+                hir::MatchSource::IfLetDesugar {..} => {
+                    err.span_note(arm_span, "`if let` arm with an incompatible type");
+                }
+                _ => {
+                    err.span_note(arm_span, "match arm with an incompatible type");
+                }
+            },
+            _ => ()
+        }
+    }
+
+    pub fn report_and_explain_type_error_with_code(&self,
+                                                   trace: TypeTrace<'tcx>,
+                                                   terr: &TypeError<'tcx>,
+                                                   message: &str,
+                                                   code: &str)
+                                                   -> DiagnosticBuilder<'tcx>
+    {
+        let (expected, found) = match self.values_str(&trace.values) {
+            Some((expected, found)) => (expected, found),
+            None => return self.tcx.sess.diagnostic().struct_dummy() /* derived error */
+        };
+
         let span = trace.origin.span();
-        let mut err = self.report_type_error(trace, terr);
+
+        let is_simple_error = if let &TypeError::Sorts(ref values) = terr {
+            values.expected.is_primitive() && values.found.is_primitive()
+        } else {
+            false
+        };
+
+        let mut err = self.tcx.sess.struct_span_err_with_code(
+            trace.origin.span(),
+            message,
+            code);
+
+        if !is_simple_error || check_old_school() {
+            err.note_expected_found(&"type", &expected, &found);
+        }
+
+        err.span_label(span, &terr);
+
+        self.note_error_origin(&mut err, &trace.origin);
+        self.check_and_note_conflicting_crates(&mut err, terr, span);
         self.tcx.note_and_explain_type_err(&mut err, terr, span);
+
         err
     }
 
-    /// Returns a string of the form "expected `{}`, found `{}`", or None if this is a derived
-    /// error.
+    pub fn report_and_explain_type_error(&self,
+                                         trace: TypeTrace<'tcx>,
+                                         terr: &TypeError<'tcx>)
+                                         -> DiagnosticBuilder<'tcx>
+    {
+        // FIXME: do we want to use a different error code for each origin?
+        let failure_str = trace.origin.as_failure_str();
+        type_err!(self, trace, terr, E0308, "{}", failure_str)
+    }
+
+    /// Returns a string of the form "expected `{}`, found `{}`".
     fn values_str(&self, values: &ValuePairs<'tcx>) -> Option<(String, String)> {
         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::PolyTraitRefs(ref exp_found) => self.expected_found_str(exp_found)
+            infer::PolyTraitRefs(ref exp_found) => self.expected_found_str(exp_found),
         }
     }
 
-    fn expected_found_str<T: fmt::Display + Resolvable<'tcx> + TypeFoldable<'tcx>>(
+    fn expected_found_str<T: fmt::Display + TypeFoldable<'tcx>>(
         &self,
         exp_found: &ty::error::ExpectedFound<T>)
         -> Option<(String, String)>
     {
-        let expected = exp_found.expected.resolve(self);
-        if expected.references_error() {
+        let exp_found = self.resolve_type_vars_if_possible(exp_found);
+        if exp_found.references_error() {
             return None;
         }
 
-        let found = exp_found.found.resolve(self);
-        if found.references_error() {
-            return None;
-        }
-
-        Some((format!("{}", expected), format!("{}", found)))
+        Some((format!("{}", exp_found.expected), format!("{}", exp_found.found)))
     }
 
     fn report_generic_bound_failure(&self,
@@ -1609,68 +1611,21 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     fn note_region_origin(&self, err: &mut DiagnosticBuilder, origin: &SubregionOrigin<'tcx>) {
         match *origin {
             infer::Subtype(ref trace) => {
-                let desc = match trace.origin {
-                    TypeOrigin::Misc(_) => {
-                        "types are compatible"
-                    }
-                    TypeOrigin::MethodCompatCheck(_) => {
-                        "method type is compatible with trait"
-                    }
-                    TypeOrigin::ExprAssignable(_) => {
-                        "expression is assignable"
-                    }
-                    TypeOrigin::RelateTraitRefs(_) => {
-                        "traits are compatible"
-                    }
-                    TypeOrigin::RelateSelfType(_) => {
-                        "self type matches impl self type"
-                    }
-                    TypeOrigin::RelateOutputImplTypes(_) => {
-                        "trait type parameters matches those \
-                                 specified on the impl"
-                    }
-                    TypeOrigin::MatchExpressionArm(_, _, _) => {
-                        "match arms have compatible types"
-                    }
-                    TypeOrigin::IfExpression(_) => {
-                        "if and else have compatible types"
-                    }
-                    TypeOrigin::IfExpressionWithNoElse(_) => {
-                        "if may be missing an else clause"
-                    }
-                    TypeOrigin::RangeExpression(_) => {
-                        "start and end of range have compatible types"
-                    }
-                    TypeOrigin::EquatePredicate(_) => {
-                        "equality where clause is satisfied"
-                    }
-                    TypeOrigin::MainFunctionType(_) => {
-                        "the `main` function has the correct type"
-                    }
-                    TypeOrigin::StartFunctionType(_) => {
-                        "the `start` function has the correct type"
-                    }
-                    TypeOrigin::IntrinsicType(_) => {
-                        "the intrinsic has the correct type"
-                    }
-                };
-
-                match self.values_str(&trace.values) {
-                    Some((expected, found)) => {
-                        err.span_note(
-                            trace.origin.span(),
-                            &format!("...so that {} (expected {}, found {})",
-                                    desc, expected, found));
-                    }
-                    None => {
-                        // Really should avoid printing this error at
-                        // all, since it is derived, but that would
-                        // require more refactoring than I feel like
-                        // doing right now. - nmatsakis
-                        err.span_note(
-                            trace.origin.span(),
-                            &format!("...so that {}", desc));
-                    }
+                if let Some((expected, found)) = self.values_str(&trace.values) {
+                    // FIXME: do we want a "the" here?
+                    err.span_note(
+                        trace.origin.span(),
+                        &format!("...so that {} (expected {}, found {})",
+                                 trace.origin.as_requirement_str(), expected, found));
+                } else {
+                    // FIXME: this really should be handled at some earlier stage. Our
+                    // handling of region checking when type errors are present is
+                    // *terrible*.
+
+                    err.span_note(
+                        trace.origin.span(),
+                        &format!("...so that {}",
+                                 trace.origin.as_requirement_str()));
                 }
             }
             infer::Reborrow(span) => {
@@ -1813,32 +1768,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     }
 }
 
-pub trait Resolvable<'tcx> {
-    fn resolve<'a, 'gcx>(&self, infcx: &InferCtxt<'a, 'gcx, 'tcx>) -> Self;
-}
-
-impl<'tcx> Resolvable<'tcx> for Ty<'tcx> {
-    fn resolve<'a, 'gcx>(&self, infcx: &InferCtxt<'a, 'gcx, 'tcx>) -> Ty<'tcx> {
-        infcx.resolve_type_vars_if_possible(self)
-    }
-}
-
-impl<'tcx> Resolvable<'tcx> for ty::TraitRef<'tcx> {
-    fn resolve<'a, 'gcx>(&self, infcx: &InferCtxt<'a, 'gcx, 'tcx>)
-                         -> ty::TraitRef<'tcx> {
-        infcx.resolve_type_vars_if_possible(self)
-    }
-}
-
-impl<'tcx> Resolvable<'tcx> for ty::PolyTraitRef<'tcx> {
-    fn resolve<'a, 'gcx>(&self,
-                         infcx: &InferCtxt<'a, 'gcx, 'tcx>)
-                         -> ty::PolyTraitRef<'tcx>
-    {
-        infcx.resolve_type_vars_if_possible(self)
-    }
-}
-
 fn lifetimes_in_scope<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
                                       scope_id: ast::NodeId)
                                       -> Vec<hir::LifetimeDef> {

src/librustc/infer/mod.rs

@@ -231,7 +231,7 @@ pub enum TypeOrigin {
 }
 
 impl TypeOrigin {
-    fn as_str(&self) -> &'static str {
+    fn as_failure_str(&self) -> &'static str {
         match self {
             &TypeOrigin::Misc(_) |
             &TypeOrigin::RelateSelfType(_) |
@@ -252,11 +252,26 @@ impl TypeOrigin {
             &TypeOrigin::IntrinsicType(_) => "intrinsic has wrong type",
         }
     }
-}
 
-impl fmt::Display for TypeOrigin {
-    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(),fmt::Error> {
-        fmt::Display::fmt(self.as_str(), f)
+    fn as_requirement_str(&self) -> &'static str {
+        match self {
+            &TypeOrigin::Misc(_) => "types are compatible",
+            &TypeOrigin::MethodCompatCheck(_) => "method type is compatible with trait",
+            &TypeOrigin::ExprAssignable(_) => "expression is assignable",
+            &TypeOrigin::RelateTraitRefs(_) => "traits are compatible",
+            &TypeOrigin::RelateSelfType(_) => "self type matches impl self type",
+            &TypeOrigin::RelateOutputImplTypes(_) => {
+                "trait type parameters matches those specified on the impl"
+            }
+            &TypeOrigin::MatchExpressionArm(_, _, _) => "match arms have compatible types",
+            &TypeOrigin::IfExpression(_) => "if and else have compatible types",
+            &TypeOrigin::IfExpressionWithNoElse(_) => "if missing an else returns ()",
+            &TypeOrigin::RangeExpression(_) => "start and end of range have compatible types",
+            &TypeOrigin::EquatePredicate(_) => "equality where clause is satisfied",
+            &TypeOrigin::MainFunctionType(_) => "`main` function has the correct type",
+            &TypeOrigin::StartFunctionType(_) => "`start` function has the correct type",
+            &TypeOrigin::IntrinsicType(_) => "intrinsic has the correct type",
+        }
     }
 }
 
@@ -1489,22 +1504,20 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     pub fn type_error_message<M>(&self,
                                  sp: Span,
                                  mk_msg: M,
-                                 actual_ty: Ty<'tcx>,
-                                 err: Option<&TypeError<'tcx>>)
+                                 actual_ty: Ty<'tcx>)
         where M: FnOnce(String) -> String,
     {
-        self.type_error_struct(sp, mk_msg, actual_ty, err).emit();
+        self.type_error_struct(sp, mk_msg, actual_ty).emit();
     }
 
     pub fn type_error_struct<M>(&self,
                                 sp: Span,
                                 mk_msg: M,
-                                actual_ty: Ty<'tcx>,
-                                err: Option<&TypeError<'tcx>>)
+                                actual_ty: Ty<'tcx>)
                                 -> DiagnosticBuilder<'tcx>
         where M: FnOnce(String) -> String,
     {
-        debug!("type_error_struct({:?}, {:?}, {:?})", sp, actual_ty, err);
+        debug!("type_error_struct({:?}, {:?})", sp, actual_ty);
 
         let actual_ty = self.resolve_type_vars_if_possible(&actual_ty);
 
@@ -1513,21 +1526,10 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
             return self.tcx.sess.diagnostic().struct_dummy();
         }
 
-        let error_str = err.map_or("".to_string(), |t_err| {
-            format!(" ({})", t_err)
-        });
-
         let msg = mk_msg(self.ty_to_string(actual_ty));
 
         // FIXME: use an error code.
-        let mut db = self.tcx.sess.struct_span_err(
-            sp, &format!("{} {}", msg, error_str));
-
-        if let Some(err) = err {
-            self.tcx.note_and_explain_type_err(&mut db, err, sp);
-        }
-
-        db
+        self.tcx.sess.struct_span_err(sp, &msg)
     }
 
     pub fn report_mismatched_types(&self,

src/librustc/macros.rs

@@ -59,3 +59,15 @@ macro_rules! span_bug {
         $crate::session::span_bug_fmt(file!(), line!(), $span, format_args!($($message)*))
     })
 }
+
+#[macro_export]
+macro_rules! type_err {
+    ($infcx:expr, $trace: expr, $terr: expr, $code:ident, $($message:tt)*) => ({
+        __diagnostic_used!($code);
+        $infcx.report_and_explain_type_error_with_code(
+            $trace,
+            $terr,
+            &format!($($message)*),
+            stringify!($code))
+    })
+}