Refactor the default type parameter algorithm

The algorithm was not correctly detecting conflicts after moving
defaults into TypeVariableValue. The updated algorithm
correctly detects and reports conflicts with information about
where the conflict occured and which items the defaults were
introduced by. The span's for said items are not being correctly
attached and still need to be patched.

Author: jroesch

src/librustc/middle/infer/error_reporting.rs

@@ -893,8 +893,8 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> {
             self.report_inference_failure(vo.clone());
         }
         self.give_suggestion(same_regions);
-        for &(ref trace, terr) in trace_origins {
-            self.report_and_explain_type_error(trace.clone(), &terr);
+        for &(ref trace, ref terr) in trace_origins {
+            self.report_and_explain_type_error(trace.clone(), terr);
         }
     }
 

src/librustc/middle/infer/mod.rs

@@ -40,6 +40,7 @@ use syntax::codemap;
 use syntax::codemap::{Span, DUMMY_SP};
 use util::nodemap::{FnvHashMap, NodeMap};
 
+use ast_map;
 use self::combine::CombineFields;
 use self::region_inference::{RegionVarBindings, RegionSnapshot};
 use self::error_reporting::ErrorReporting;
@@ -72,7 +73,7 @@ pub struct InferCtxt<'a, 'tcx: 'a> {
     // We instantiate UnificationTable with bounds<Ty> because the
     // types that might instantiate a general type variable have an
     // order, represented by its upper and lower bounds.
-    type_variables: RefCell<type_variable::TypeVariableTable<'tcx>>,
+    pub type_variables: RefCell<type_variable::TypeVariableTable<'tcx>>,
 
     // Map from integral variable to the kind of integer it represents
     int_unification_table: RefCell<UnificationTable<ty::IntVid>>,
@@ -690,7 +691,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         variables.extend(unbound_ty_vars);
         variables.extend(unbound_int_vars);
         variables.extend(unbound_float_vars);
-        
+
         return variables;
     }
 
@@ -1047,15 +1048,36 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
     }
 
     pub fn type_vars_for_defs(&self,
+                              span: Span,
+                              // substs: Substs,
                               defs: &[ty::TypeParameterDef<'tcx>])
                               -> Vec<ty::Ty<'tcx>> {
+
+        fn definition_span<'tcx>(tcx: &ty::ctxt<'tcx>, def_id: ast::DefId) -> Span {
+            let parent = tcx.map.get_parent(def_id.node);
+            debug!("definition_span def_id={:?} parent={:?} node={:?} parent_node={:?}",
+                def_id, parent, tcx.map.find(def_id.node), tcx.map.find(parent));
+            match tcx.map.find(parent) {
+                None => DUMMY_SP,
+                Some(ref node) => match *node {
+                    ast_map::NodeItem(ref item) => item.span,
+                    ast_map::NodeForeignItem(ref item) => item.span,
+                    ast_map::NodeTraitItem(ref item) => item.span,
+                    ast_map::NodeImplItem(ref item) => item.span,
+                    _ => DUMMY_SP
+                }
+            }
+        }
+
         let mut substs = Substs::empty();
         let mut vars = Vec::with_capacity(defs.len());
 
         for def in defs.iter() {
             let default = def.default.map(|default| {
                 type_variable::Default {
-                    ty: default
+                    ty: default,
+                    origin_span: span,
+                    definition_span: definition_span(self.tcx, def.def_id)
                 }
             });
             //.subst(self.tcx, &substs)
@@ -1077,7 +1099,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         let mut type_params = subst::VecPerParamSpace::empty();
 
         for space in subst::ParamSpace::all().iter() {
-            type_params.replace(*space, self.type_vars_for_defs(generics.types.get_slice(*space)))
+            type_params.replace(*space,
+                                self.type_vars_for_defs(span, generics.types.get_slice(*space)))
         }
 
         let region_params =
@@ -1102,7 +1125,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
         assert!(generics.regions.len(subst::FnSpace) == 0);
 
         let type_parameter_defs = generics.types.get_slice(subst::TypeSpace);
-        let type_parameters = self.type_vars_for_defs(type_parameter_defs);
+        let type_parameters = self.type_vars_for_defs(span, type_parameter_defs);
 
         let region_param_defs = generics.regions.get_slice(subst::TypeSpace);
         let regions = self.region_vars_for_defs(span, region_param_defs);
@@ -1344,13 +1367,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
 
     pub fn report_conflicting_default_types(&self,
                                    span: Span,
-                                   expected: Ty<'tcx>,
-                                   actual: Ty<'tcx>) {
+                                   expected: type_variable::Default<'tcx>,
+                                   actual: type_variable::Default<'tcx>) {
         let trace = TypeTrace {
             origin: Misc(span),
             values: Types(ty::expected_found {
-                expected: expected,
-                found: actual
+                expected: expected.ty,
+                found: actual.ty
             })
         };
 

src/librustc/middle/infer/type_variable.rs

@@ -11,8 +11,9 @@
 pub use self::RelationDir::*;
 use self::TypeVariableValue::*;
 use self::UndoEntry::*;
-
 use middle::ty::{self, Ty};
+use syntax::codemap::Span;
+
 use std::cmp::min;
 use std::marker::PhantomData;
 use std::mem;
@@ -38,9 +39,13 @@ enum TypeVariableValue<'tcx> {
 
 // We will use this to store the required information to recapitulate what happened when
 // an error occurs.
-#[derive(Clone)]
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
 pub struct Default<'tcx> {
-    pub ty: Ty<'tcx>
+    pub ty: Ty<'tcx>,
+    /// The span where the default was incurred
+    pub origin_span: Span,
+    /// The definition that the default originates from
+    pub definition_span: Span
 }
 
 pub struct Snapshot {

src/librustc/middle/ty.rs

@@ -54,6 +54,7 @@ use middle::lang_items::{FnTraitLangItem, FnMutTraitLangItem, FnOnceTraitLangIte
 use middle::region;
 use middle::resolve_lifetime;
 use middle::infer;
+use middle::infer::type_variable;
 use middle::pat_util;
 use middle::region::RegionMaps;
 use middle::stability;
@@ -2068,7 +2069,7 @@ pub enum TypeError<'tcx> {
     ConvergenceMismatch(ExpectedFound<bool>),
     ProjectionNameMismatched(ExpectedFound<ast::Name>),
     ProjectionBoundsLength(ExpectedFound<usize>),
-    terr_ty_param_default_mismatch(expected_found<Ty<'tcx>>)
+    TyParamDefaultMismatch(ExpectedFound<Ty<'tcx>>)
 }
 
 /// Bounds suitable for an existentially quantified type parameter
@@ -5083,9 +5084,9 @@ impl<'tcx> fmt::Display for TypeError<'tcx> {
                        values.found)
             },
             terr_ty_param_default_mismatch(ref values) => {
-                write!(f, "conflicting type parameter defaults {} {}",
-                       values.expected,
-                       values.found)
+                write!(f, "conflicting type parameter defaults {} and {}",
+                       values.expected.ty,
+                       values.found.ty)
             }
         }
     }
@@ -5405,7 +5406,7 @@ impl<'tcx> ctxt<'tcx> {
     pub fn note_and_explain_type_err(&self, err: &TypeError<'tcx>, sp: Span) {
         use self::TypeError::*;
 
-        match *err {
+        match err.clone() {
             RegionsDoesNotOutlive(subregion, superregion) => {
                 self.note_and_explain_region("", subregion, "...");
                 self.note_and_explain_region("...does not necessarily outlive ",
@@ -5444,10 +5445,21 @@ impl<'tcx> ctxt<'tcx> {
                                   using it as a trait object"));
                 }
             },
-            terr_ty_param_default_mismatch(expected) => {
+            terr_ty_param_default_mismatch(values) => {
+                let expected = values.expected;
+                let found = values.found;
                 self.sess.span_note(sp,
-                    &format!("found conflicting defaults {:?} {:?}",
-                             expected.expected, expected.found))
+                    &format!("conflicting type parameter defaults {} and {}",
+                             expected.ty,
+                             found.ty));
+                self.sess.span_note(expected.definition_span,
+                    &format!("...a default was defined"));
+                self.sess.span_note(expected.origin_span,
+                    &format!("...that was applied to an unconstrained type variable here"));
+                self.sess.span_note(found.definition_span,
+                    &format!("...a second default was defined"));
+                self.sess.span_note(found.origin_span,
+                    &format!("...that also applies to the same type variable here"));
             }
             _ => {}
         }

src/librustc_typeck/astconv.rs

@@ -111,7 +111,7 @@ pub trait AstConv<'tcx> {
     }
 
     /// What type should we use when a type is omitted?
-    fn ty_infer(&self, default: Option<Ty<'tcx>>, span: Span) -> Ty<'tcx>;
+    fn ty_infer(&self, default: Option<ty::TypeParameterDef<'tcx>>, span: Span) -> Ty<'tcx>;
 
     /// Projecting an associated type from a (potentially)
     /// higher-ranked trait reference is more complicated, because of
@@ -403,7 +403,7 @@ fn create_substs_for_ast_path<'tcx>(
     // they were optional (e.g. paths inside expressions).
     let mut type_substs = if param_mode == PathParamMode::Optional &&
                              types_provided.is_empty() {
-        ty_param_defs.iter().map(|p| this.ty_infer(p.default, span)).collect()
+        ty_param_defs.iter().map(|p| this.ty_infer(Some(p.clone()), span)).collect()
     } else {
         types_provided
     };

src/librustc_typeck/check/method/confirm.rs

@@ -315,11 +315,11 @@ impl<'a,'tcx> ConfirmContext<'a,'tcx> {
 
         let method_types = {
             if num_supplied_types == 0 {
-                self.fcx.infcx().type_vars_for_defs(method_types)
+                self.fcx.infcx().type_vars_for_defs(self.span, method_types)
             } else if num_method_types == 0 {
                 span_err!(self.tcx().sess, self.span, E0035,
                     "does not take type parameters");
-                self.fcx.infcx().type_vars_for_defs(method_types)
+                self.fcx.infcx().type_vars_for_defs(self.span, method_types)
             } else if num_supplied_types != num_method_types {
                 span_err!(self.tcx().sess, self.span, E0036,
                     "incorrect number of type parameters given for this method");

src/librustc_typeck/check/method/mod.rs

@@ -176,7 +176,7 @@ pub fn lookup_in_trait_adjusted<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>,
         }
 
         None => {
-            fcx.inh.infcx.type_vars_for_defs(type_parameter_defs)
+            fcx.inh.infcx.type_vars_for_defs(span, type_parameter_defs)
         }
     };
 

src/librustc_typeck/check/method/probe.rs

@@ -1207,7 +1207,7 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
             !method.generics.regions.is_empty_in(subst::FnSpace)
         {
             let method_types =
-                self.infcx().type_vars_for_defs(
+                self.infcx().type_vars_for_defs(self.span,
                     method.generics.types.get_slice(subst::FnSpace));
 
             // In general, during probe we erase regions. See