Auto merge of #34907 - arielb1:found-parse-error, r=nikomatsakis

Centralize and clean type error reporting

Refactors the code that handles type errors to be cleaner and fixes various edge cases.

This made the already-bad "type mismatch resolving" error message somewhat uglier. I want to fix that in another commit before this PR is merged.

Fixes #31173

r? @jonathandturner, cc @nikomatsakis

Author: bors

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,42 +504,102 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         }
     }
 
+    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 note_type_err(&self,
+                         diag: &mut DiagnosticBuilder<'tcx>,
+                         origin: TypeOrigin,
+                         values: Option<ValuePairs<'tcx>>,
+                         terr: &TypeError<'tcx>)
+    {
+        let expected_found = match values {
+            None => None,
+            Some(values) => match self.values_str(&values) {
+                Some((expected, found)) => Some((expected, found)),
+                None => {
+                    // Derived error. Cancel the emitter.
+                    self.tcx.sess.diagnostic().cancel(diag);
+                    return
+                }
+            }
+        };
+
+        let span = origin.span();
+
+        let mut is_simple_error = false;
+
+        if let Some((expected, found)) = expected_found {
+            is_simple_error = if let &TypeError::Sorts(ref values) = terr {
+                values.expected.is_primitive() && values.found.is_primitive()
+            } else {
+                false
+            };
+
+            if !is_simple_error || check_old_school() {
+                diag.note_expected_found(&"type", &expected, &found);
+            }
+        }
+
+        if !is_simple_error && check_old_school() {
+            diag.span_note(span, &format!("{}", terr));
+        } else {
+            diag.span_label(span, &terr);
+        }
+
+        self.note_error_origin(diag, &origin);
+        self.check_and_note_conflicting_crates(diag, terr, span);
+        self.tcx.note_and_explain_type_err(diag, terr, span);
+    }
+
     pub fn report_and_explain_type_error(&self,
                                          trace: TypeTrace<'tcx>,
                                          terr: &TypeError<'tcx>)
-                                         -> DiagnosticBuilder<'tcx> {
-        let span = trace.origin.span();
-        let mut err = self.report_type_error(trace, terr);
-        self.tcx.note_and_explain_type_err(&mut err, terr, span);
-        err
+                                         -> DiagnosticBuilder<'tcx>
+    {
+        // FIXME: do we want to use a different error code for each origin?
+        let mut diag = struct_span_err!(
+            self.tcx.sess, trace.origin.span(), E0308,
+            "{}", trace.origin.as_failure_str()
+        );
+        self.note_type_err(&mut diag, trace.origin, Some(trace.values), terr);
+        diag
     }
 
-    /// Returns a string of the form "expected `{}`, found `{}`", or None if this is a derived
-    /// error.
+    /// 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() {
-            return None;
-        }
-
-        let found = exp_found.found.resolve(self);
-        if found.references_error() {
+        let exp_found = self.resolve_type_vars_if_possible(exp_found);
+        if exp_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,
@@ -1608,59 +1622,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"
-                    }
-                };
-
-                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) => {
@@ -1803,32 +1779,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> {