From ed3fbba797b0a68b797469f49f878307cc64c993 Mon Sep 17 00:00:00 2001 From: Jared Roesch Date: Thu, 16 Jul 2015 11:26:02 -0700 Subject: [PATCH] Fix error message spans --- src/librustc/ast_map/mod.rs | 21 ++++- src/librustc/metadata/tydecode.rs | 2 + src/librustc/metadata/tyencode.rs | 4 +- src/librustc/middle/infer/mod.rs | 28 ++----- src/librustc/middle/ty.rs | 9 +- src/librustc/middle/ty_fold.rs | 1 + src/librustc_trans/trans/monomorphize.rs | 1 + src/librustc_typeck/astconv.rs | 22 +++-- src/librustc_typeck/check/mod.rs | 84 +++++++++++-------- src/librustc_typeck/collect.rs | 9 +- src/rust-installer | 2 +- .../default_ty_param_dependent_defaults.rs | 4 +- 12 files changed, 110 insertions(+), 77 deletions(-) diff --git a/src/librustc/ast_map/mod.rs b/src/librustc/ast_map/mod.rs index bdb481bc9385..10552791d8b8 100644 --- a/src/librustc/ast_map/mod.rs +++ b/src/librustc/ast_map/mod.rs @@ -119,6 +119,7 @@ pub enum Node<'ast> { NodeStructCtor(&'ast StructDef), NodeLifetime(&'ast Lifetime), + NodeTyParam(&'ast TyParam) } /// Represents an entry and its parent NodeID. @@ -142,6 +143,7 @@ enum MapEntry<'ast> { EntryBlock(NodeId, &'ast Block), EntryStructCtor(NodeId, &'ast StructDef), EntryLifetime(NodeId, &'ast Lifetime), + EntryTyParam(NodeId, &'ast TyParam), /// Roots for node trees. RootCrate, @@ -175,7 +177,8 @@ impl<'ast> MapEntry<'ast> { NodePat(n) => EntryPat(p, n), NodeBlock(n) => EntryBlock(p, n), NodeStructCtor(n) => EntryStructCtor(p, n), - NodeLifetime(n) => EntryLifetime(p, n) + NodeLifetime(n) => EntryLifetime(p, n), + NodeTyParam(n) => EntryTyParam(p, n), } } @@ -194,6 +197,7 @@ impl<'ast> MapEntry<'ast> { EntryBlock(id, _) => id, EntryStructCtor(id, _) => id, EntryLifetime(id, _) => id, + EntryTyParam(id, _) => id, _ => return None }) } @@ -213,6 +217,7 @@ impl<'ast> MapEntry<'ast> { EntryBlock(_, n) => NodeBlock(n), EntryStructCtor(_, n) => NodeStructCtor(n), EntryLifetime(_, n) => NodeLifetime(n), + EntryTyParam(_, n) => NodeTyParam(n), _ => return None }) } @@ -573,6 +578,7 @@ impl<'ast> Map<'ast> { Some(NodePat(pat)) => pat.span, Some(NodeBlock(block)) => block.span, Some(NodeStructCtor(_)) => self.expect_item(self.get_parent(id)).span, + Some(NodeTyParam(ty_param)) => ty_param.span, _ => return None, }; Some(sp) @@ -815,6 +821,14 @@ impl<'ast> Visitor<'ast> for NodeCollector<'ast> { self.parent_node = parent_node; } + fn visit_generics(&mut self, generics: &'ast Generics) { + for ty_param in generics.ty_params.iter() { + self.insert(ty_param.id, NodeTyParam(ty_param)); + } + + visit::walk_generics(self, generics); + } + fn visit_trait_item(&mut self, ti: &'ast TraitItem) { let parent_node = self.parent_node; self.parent_node = ti.id; @@ -1015,7 +1029,7 @@ impl<'a> NodePrinter for pprust::State<'a> { NodePat(a) => self.print_pat(&*a), NodeBlock(a) => self.print_block(&*a), NodeLifetime(a) => self.print_lifetime(&*a), - + NodeTyParam(_) => panic!("cannot print TyParam"), // these cases do not carry enough information in the // ast_map to reconstruct their full structure for pretty // printing. @@ -1123,6 +1137,9 @@ fn node_id_to_string(map: &Map, id: NodeId, include_id: bool) -> String { format!("lifetime {}{}", pprust::lifetime_to_string(&**l), id_str) } + Some(NodeTyParam(ref ty_param)) => { + format!("typaram {:?}{}", ty_param, id_str) + } None => { format!("unknown node{}", id_str) } diff --git a/src/librustc/metadata/tydecode.rs b/src/librustc/metadata/tydecode.rs index 72e1525b506d..54c55d76a827 100644 --- a/src/librustc/metadata/tydecode.rs +++ b/src/librustc/metadata/tydecode.rs @@ -833,6 +833,7 @@ fn parse_type_param_def_<'a, 'tcx, F>(st: &mut PState<'a, 'tcx>, conv: &mut F) assert_eq!(next(st), '|'); let index = parse_u32(st); assert_eq!(next(st), '|'); + let default_def_id = parse_def_(st, NominalType, conv); let default = parse_opt(st, |st| parse_ty_(st, conv)); let object_lifetime_default = parse_object_lifetime_default(st, conv); @@ -841,6 +842,7 @@ fn parse_type_param_def_<'a, 'tcx, F>(st: &mut PState<'a, 'tcx>, conv: &mut F) def_id: def_id, space: space, index: index, + default_def_id: default_def_id, default: default, object_lifetime_default: object_lifetime_default, } diff --git a/src/librustc/metadata/tyencode.rs b/src/librustc/metadata/tyencode.rs index c77e96f16488..597401daccfd 100644 --- a/src/librustc/metadata/tyencode.rs +++ b/src/librustc/metadata/tyencode.rs @@ -409,9 +409,9 @@ pub fn enc_region_bounds<'a, 'tcx>(w: &mut Encoder, pub fn enc_type_param_def<'a, 'tcx>(w: &mut Encoder, cx: &ctxt<'a, 'tcx>, v: &ty::TypeParameterDef<'tcx>) { - mywrite!(w, "{}:{}|{}|{}|", + mywrite!(w, "{}:{}|{}|{}|{}|", token::get_name(v.name), (cx.ds)(v.def_id), - v.space.to_uint(), v.index); + v.space.to_uint(), v.index, (cx.ds)(v.default_def_id)); enc_opt(w, v.default, |w, t| enc_ty(w, cx, t)); enc_object_lifetime_default(w, cx, v.object_lifetime_default); } diff --git a/src/librustc/middle/infer/mod.rs b/src/librustc/middle/infer/mod.rs index 253f6379d155..d10dd7073532 100644 --- a/src/librustc/middle/infer/mod.rs +++ b/src/librustc/middle/infer/mod.rs @@ -40,7 +40,6 @@ 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; @@ -658,6 +657,9 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { /// must be attached to the variable when created, if it is created /// without a default, this will return None. /// + /// This code does not apply to integral or floating point variables, + /// only to use declared defaults. + /// /// See `new_ty_var_with_default` to create a type variable with a default. /// See `type_variable::Default` for details about what a default entails. pub fn default(&self, ty: Ty<'tcx>) -> Option> { @@ -1055,31 +1057,15 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { substs: &mut Substs<'tcx>, defs: &[ty::TypeParameterDef<'tcx>]) { - // This doesn't work ... - 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 vars = Vec::with_capacity(defs.len()); for def in defs.iter() { - let default = def.default.subst_spanned(self.tcx, substs, Some(span)).map(|default| { + let default = def.default.map(|default| { + let definition_span = self.tcx.map.opt_span(def.def_id.node); type_variable::Default { - ty: default, + ty: default.subst_spanned(self.tcx, substs, Some(span)), origin_span: span, - definition_span: definition_span(self.tcx, def.def_id) + definition_span: definition_span.unwrap_or(DUMMY_SP) } }); diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 4945e0766e75..a4f714b3bf9c 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -115,8 +115,6 @@ pub struct Field<'tcx> { pub mt: TypeAndMut<'tcx> } - - // Enum information #[derive(Clone)] pub struct VariantInfo<'tcx> { @@ -2282,6 +2280,7 @@ pub struct TypeParameterDef<'tcx> { pub def_id: ast::DefId, pub space: subst::ParamSpace, pub index: u32, + pub default_def_id: DefId, // for use in error reporing about defaults pub default: Option>, pub object_lifetime_default: ObjectLifetimeDefault, } @@ -5084,7 +5083,7 @@ impl<'tcx> fmt::Display for TypeError<'tcx> { values.found) }, TyParamDefaultMismatch(ref values) => { - write!(f, "conflicting type parameter defaults {} and {}", + write!(f, "conflicting type parameter defaults `{}` and `{}`", values.expected.ty, values.found.ty) } @@ -5453,11 +5452,11 @@ impl<'tcx> ctxt<'tcx> { expected.ty, found.ty)); self.sess.span_note(expected.definition_span, - &format!("...a default was defined")); + &format!("a default was defined here...")); 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")); + &format!("a second default was defined here...")); self.sess.span_note(found.origin_span, &format!("...that also applies to the same type variable here")); } diff --git a/src/librustc/middle/ty_fold.rs b/src/librustc/middle/ty_fold.rs index b6bb82ad7b15..0c694926ba4b 100644 --- a/src/librustc/middle/ty_fold.rs +++ b/src/librustc/middle/ty_fold.rs @@ -340,6 +340,7 @@ impl<'tcx> TypeFoldable<'tcx> for ty::TypeParameterDef<'tcx> { space: self.space, index: self.index, default: self.default.fold_with(folder), + default_def_id: self.default_def_id, object_lifetime_default: self.object_lifetime_default.fold_with(folder), } } diff --git a/src/librustc_trans/trans/monomorphize.rs b/src/librustc_trans/trans/monomorphize.rs index 217181da1421..31e4b9c48e20 100644 --- a/src/librustc_trans/trans/monomorphize.rs +++ b/src/librustc_trans/trans/monomorphize.rs @@ -266,6 +266,7 @@ pub fn monomorphic_fn<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, // Ugh -- but this ensures any new variants won't be forgotten ast_map::NodeForeignItem(..) | ast_map::NodeLifetime(..) | + ast_map::NodeTyParam(..) | ast_map::NodeExpr(..) | ast_map::NodeStmt(..) | ast_map::NodeArg(..) | diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 3925f4e751c2..120cfcbca374 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -55,7 +55,7 @@ use middle::def; use middle::implicator::object_region_bounds; use middle::resolve_lifetime as rl; use middle::privacy::{AllPublic, LastMod}; -use middle::subst::{FnSpace, TypeSpace, SelfSpace, Subst, Substs}; +use middle::subst::{FnSpace, TypeSpace, SelfSpace, Subst, Substs, ParamSpace}; use middle::traits; use middle::ty::{self, RegionEscape, Ty, ToPredicate, HasTypeFlags}; use middle::ty_fold; @@ -111,7 +111,11 @@ pub trait AstConv<'tcx> { } /// What type should we use when a type is omitted? - fn ty_infer(&self, default: Option>, span: Span) -> Ty<'tcx>; + fn ty_infer(&self, + param_and_substs: Option>, + substs: Option<&mut Substs<'tcx>>, + space: Option, + span: Span) -> Ty<'tcx>; /// Projecting an associated type from a (potentially) /// higher-ranked trait reference is more complicated, because of @@ -403,7 +407,11 @@ 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(Some(p.clone()), span)).collect() + let mut substs = region_substs.clone(); + ty_param_defs + .iter() + .map(|p| this.ty_infer(Some(p.clone()), Some(&mut substs), Some(TypeSpace), span)) + .collect() } else { types_provided }; @@ -1661,7 +1669,7 @@ pub fn ast_ty_to_ty<'tcx>(this: &AstConv<'tcx>, // values in a ExprClosure, or as // the type of local variables. Both of these cases are // handled specially and will not descend into this routine. - this.ty_infer(None, ast_ty.span) + this.ty_infer(None, None, None, ast_ty.span) } }; @@ -1677,7 +1685,7 @@ pub fn ty_of_arg<'tcx>(this: &AstConv<'tcx>, { match a.ty.node { ast::TyInfer if expected_ty.is_some() => expected_ty.unwrap(), - ast::TyInfer => this.ty_infer(None, a.ty.span), + ast::TyInfer => this.ty_infer(None, None, None, a.ty.span), _ => ast_ty_to_ty(this, rscope, &*a.ty), } } @@ -1796,7 +1804,7 @@ fn ty_of_method_or_bare_fn<'a, 'tcx>(this: &AstConv<'tcx>, let output_ty = match decl.output { ast::Return(ref output) if output.node == ast::TyInfer => - ty::FnConverging(this.ty_infer(None, output.span)), + ty::FnConverging(this.ty_infer(None, None, None, output.span)), ast::Return(ref output) => ty::FnConverging(convert_ty_with_lifetime_elision(this, implied_output_region, @@ -1936,7 +1944,7 @@ pub fn ty_of_closure<'tcx>( _ if is_infer && expected_ret_ty.is_some() => expected_ret_ty.unwrap(), _ if is_infer => - ty::FnConverging(this.ty_infer(None, decl.output.span())), + ty::FnConverging(this.ty_infer(None, None, None, decl.output.span())), ast::Return(ref output) => ty::FnConverging(ast_ty_to_ty(this, &rb, &**output)), ast::DefaultReturn(..) => unreachable!(), diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index fd99d1ddba9c..76886fc1275b 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1139,14 +1139,31 @@ impl<'a, 'tcx> AstConv<'tcx> for FnCtxt<'a, 'tcx> { trait_def.associated_type_names.contains(&assoc_name) } - fn ty_infer(&self, ty_param_def: Option>, span: Span) -> Ty<'tcx> { - let default = ty_param_def.and_then(|t| - t.default.map(|ty| type_variable::Default { - ty: ty, + fn ty_infer(&self, + ty_param_def: Option>, + substs: Option<&mut subst::Substs<'tcx>>, + space: Option, + span: Span) -> Ty<'tcx> { + // Grab the default doing subsitution + let default = ty_param_def.and_then(|def| { + let definition_span = self.tcx() + .map + .opt_span(def.def_id.node); + + def.default.map(|ty| type_variable::Default { + ty: ty.subst_spanned(self.tcx(), substs.as_ref().unwrap(), Some(span)), origin_span: span, - definition_span: span - })); - self.infcx().next_ty_var_with_default(default) + definition_span: definition_span.unwrap_or(codemap::DUMMY_SP) + }) + }); + + let ty_var = self.infcx().next_ty_var_with_default(default); + + // Finally we add the type variable to the substs + match substs { + None => ty_var, + Some(substs) => { substs.types.push(space.unwrap(), ty_var); ty_var } + } } fn projected_ty_from_poly_trait_ref(&self, @@ -1829,10 +1846,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // a unification failure and then report an error for each. for (conflict, default) in conflicts { let conflicting_default = - self.find_conflicting_default( - &unbound_tyvars, - &default_map, - conflict).unwrap_or(type_variable::Default { + self.find_conflicting_default(&unbound_tyvars, &default_map, conflict) + .unwrap_or(type_variable::Default { ty: self.infcx().next_ty_var(), origin_span: codemap::DUMMY_SP, definition_span: codemap::DUMMY_SP @@ -1871,36 +1886,33 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // We also run this inside snapshot that never commits so we can do error // reporting for more then one conflict. - //let _ = self.infcx().commit_if_ok(|_: &infer::CombinedSnapshot| { - for ty in &unbound_tyvars { - if self.infcx().type_var_diverges(ty) { - demand::eqtype(self, codemap::DUMMY_SP, *ty, self.tcx().mk_nil()); - } else { - match self.infcx().type_is_unconstrained_numeric(ty) { - UnconstrainedInt => { - demand::eqtype(self, codemap::DUMMY_SP, *ty, self.tcx().types.i32) - }, - UnconstrainedFloat => { - demand::eqtype(self, codemap::DUMMY_SP, *ty, self.tcx().types.f64) - }, - Neither => { - if let Some(default) = default_map.get(ty) { - let default = default.clone(); - match infer::mk_eqty(self.infcx(), false, - infer::Misc(default.origin_span), - ty, default.ty) { - Ok(()) => {} - Err(_) => { - result = Some(default); - } + for ty in &unbound_tyvars { + if self.infcx().type_var_diverges(ty) { + demand::eqtype(self, codemap::DUMMY_SP, *ty, self.tcx().mk_nil()); + } else { + match self.infcx().type_is_unconstrained_numeric(ty) { + UnconstrainedInt => { + demand::eqtype(self, codemap::DUMMY_SP, *ty, self.tcx().types.i32) + }, + UnconstrainedFloat => { + demand::eqtype(self, codemap::DUMMY_SP, *ty, self.tcx().types.f64) + }, + Neither => { + if let Some(default) = default_map.get(ty) { + let default = default.clone(); + match infer::mk_eqty(self.infcx(), false, + infer::Misc(default.origin_span), + ty, default.ty) { + Ok(()) => {} + Err(_) => { + result = Some(default); } } } } } } - // let result: Result<(), ()> = Err(()); result - //}); + } return result; } @@ -4613,7 +4625,6 @@ pub fn instantiate_path<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, } } } - if let Some(self_ty) = opt_self_ty { if type_defs.len(subst::SelfSpace) == 1 { substs.types.push(subst::SelfSpace, self_ty); @@ -4839,6 +4850,7 @@ pub fn instantiate_path<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, // Nothing specified at all: supply inference variables for // everything. if provided_len == 0 && !(require_type_space && space == subst::TypeSpace) { + substs.types.replace(space, Vec::new()); fcx.infcx().type_vars_for_defs(span, space, substs, &desired[..]); return; } diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 8b38c58ce7a1..6bff90825f32 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -404,7 +404,11 @@ impl<'a, 'tcx> AstConv<'tcx> for ItemCtxt<'a, 'tcx> { } } - fn ty_infer(&self, _default: Option>, span: Span) -> Ty<'tcx> { + fn ty_infer(&self, + _ty_param_def: Option>, + _substs: Option<&mut Substs<'tcx>>, + _space: Option, + span: Span) -> Ty<'tcx> { span_err!(self.tcx().sess, span, E0121, "the type placeholder `_` is not allowed within types on item signatures"); self.tcx().types.err @@ -1648,6 +1652,7 @@ fn ty_generics_for_trait<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>, index: 0, name: special_idents::type_self.name, def_id: local_def(param_id), + default_def_id: local_def(param_id), default: None, object_lifetime_default: ty::ObjectLifetimeDefault::BaseDefault, }; @@ -1921,6 +1926,8 @@ fn get_or_create_type_parameter_def<'a,'tcx>(ccx: &CrateCtxt<'a,'tcx>, index: index, name: param.ident.name, def_id: local_def(param.id), + // what do I return? should this be an option as well + default_def_id: local_def(param.default.as_ref().map(|d| d.id).unwrap_or(param.id)), default: default, object_lifetime_default: object_lifetime_default, }; diff --git a/src/rust-installer b/src/rust-installer index 8e4f8ea58150..c37d3747da75 160000 --- a/src/rust-installer +++ b/src/rust-installer @@ -1 +1 @@ -Subproject commit 8e4f8ea581502a2edc8177a040300e05ff7f91e3 +Subproject commit c37d3747da75c280237dc2d6b925078e69555499 diff --git a/src/test/run-pass/default_ty_param_dependent_defaults.rs b/src/test/run-pass/default_ty_param_dependent_defaults.rs index 9322c9ad165a..eba86415af4c 100644 --- a/src/test/run-pass/default_ty_param_dependent_defaults.rs +++ b/src/test/run-pass/default_ty_param_dependent_defaults.rs @@ -11,8 +11,8 @@ use std::marker::PhantomData; -struct Foo { data: PhantomData<(T, U)> } +struct Foo { t: T, data: PhantomData } fn main() { - let foo = Foo { data: PhantomData }; + let foo = Foo { t: 'a', data: PhantomData }; }