From a159f856d54c507052d24a026ccc8a568aa454f3 Mon Sep 17 00:00:00 2001 From: Michael Sullivan Date: Fri, 7 Jun 2013 10:29:21 -0700 Subject: [PATCH 1/3] Properly translate calls to default methods in a number of cases. Closes #4350. --- src/librustc/middle/trans/meth.rs | 69 ++++++------------- .../run-pass/traits-default-method-self.rs | 4 +- 2 files changed, 23 insertions(+), 50 deletions(-) diff --git a/src/librustc/middle/trans/meth.rs b/src/librustc/middle/trans/meth.rs index f05165fe256..9d8ef2e603b 100644 --- a/src/librustc/middle/trans/meth.rs +++ b/src/librustc/middle/trans/meth.rs @@ -207,7 +207,9 @@ pub fn trans_method_callee(bcx: block, let method_name = ty::trait_method(tcx, trait_id, method_index).ident; let method_id = - method_with_name(bcx.ccx(), impl_def_id, method_name); + method_with_name_or_default(bcx.ccx(), + impl_def_id, + method_name); origin = typeck::method_static(method_id); } typeck::method_super(trait_id, method_index) => { @@ -229,9 +231,10 @@ pub fn trans_method_callee(bcx: block, ty::method(tcx, supertrait_method_def_ids[method_index]).ident; // Now that we know the impl ID, we can look up the method // ID from its name - origin = typeck::method_static(method_with_name(bcx.ccx(), - impl_id, - method_name)); + origin = typeck::method_static( + method_with_name_or_default(bcx.ccx(), + impl_id, + method_name)); } typeck::method_static(*) | typeck::method_param(*) | typeck::method_trait(*) => {} @@ -345,7 +348,9 @@ pub fn trans_static_method_callee(bcx: block, typeck::vtable_static(impl_did, ref rcvr_substs, rcvr_origins) => { assert!(rcvr_substs.all(|t| !ty::type_needs_infer(*t))); - let mth_id = method_with_name(bcx.ccx(), impl_did, mname); + let mth_id = method_with_name_or_default(bcx.ccx(), + impl_did, + mname); let callee_substs = combine_impl_and_methods_tps( bcx, mth_id, impl_did, callee_id, *rcvr_substs); let callee_origins = combine_impl_and_methods_origins( @@ -374,23 +379,6 @@ pub fn method_from_methods(ms: &[@ast::method], name: ast::ident) ms.find(|m| m.ident == name).map(|m| ast_util::local_def(m.id)) } -pub fn method_with_name(ccx: @CrateContext, impl_id: ast::def_id, - name: ast::ident) -> ast::def_id { - if impl_id.crate == ast::local_crate { - match ccx.tcx.items.get_copy(&impl_id.node) { - ast_map::node_item(@ast::item { - node: ast::item_impl(_, _, _, ref ms), - _ - }, _) => { - method_from_methods(*ms, name).get() - } - _ => fail!("method_with_name") - } - } else { - csearch::get_impl_method(ccx.sess.cstore, impl_id, name) - } -} - pub fn method_with_name_or_default(ccx: @CrateContext, impl_id: ast::def_id, name: ast::ident) -> ast::def_id { @@ -770,17 +758,17 @@ pub fn vtable_id(ccx: @CrateContext, /// Creates a returns a dynamic vtable for the given type and vtable origin. /// This is used only for objects. -pub fn get_vtable(ccx: @CrateContext, +pub fn get_vtable(bcx: block, self_ty: ty::t, origin: typeck::vtable_origin) -> ValueRef { - let hash_id = vtable_id(ccx, &origin); - match ccx.vtables.find(&hash_id) { + let hash_id = vtable_id(bcx.ccx(), &origin); + match bcx.ccx().vtables.find(&hash_id) { Some(&val) => val, None => { match origin { typeck::vtable_static(id, substs, sub_vtables) => { - make_impl_vtable(ccx, id, self_ty, substs, sub_vtables) + make_impl_vtable(bcx, id, self_ty, substs, sub_vtables) } _ => fail!("get_vtable: expected a static origin"), } @@ -814,12 +802,13 @@ pub fn make_vtable(ccx: @CrateContext, } /// Generates a dynamic vtable for objects. -pub fn make_impl_vtable(ccx: @CrateContext, +pub fn make_impl_vtable(bcx: block, impl_id: ast::def_id, self_ty: ty::t, substs: ~[ty::t], vtables: typeck::vtable_res) -> ValueRef { + let ccx = bcx.ccx(); let _icx = ccx.insn_ctxt("impl::make_impl_vtable"); let tcx = ccx.tcx; @@ -829,9 +818,6 @@ pub fn make_impl_vtable(ccx: @CrateContext, make a vtable for a type impl!") }; - let has_tps = - !ty::lookup_item_type(ccx.tcx, impl_id).generics.type_param_defs.is_empty(); - let trait_method_def_ids = ty::trait_method_def_ids(tcx, trt_id); let methods = do trait_method_def_ids.map |method_def_id| { let im = ty::method(tcx, *method_def_id); @@ -846,22 +832,11 @@ pub fn make_impl_vtable(ccx: @CrateContext, } else { debug!("(making impl vtable) adding method to vtable: %s", *tcx.sess.str_of(im.ident)); - let mut m_id = method_with_name(ccx, impl_id, im.ident); - if has_tps { - // If the method is in another crate, need to make an inlined - // copy first - if m_id.crate != ast::local_crate { - // XXX: Set impl ID here? - m_id = inline::maybe_instantiate_inline(ccx, m_id, true); - } - let (val, _) = monomorphize::monomorphic_fn(ccx, m_id, substs, - Some(vtables), None, None); - val - } else if m_id.crate == ast::local_crate { - get_item_val(ccx, m_id.node) - } else { - trans_external_path(ccx, m_id, fty) - } + let m_id = method_with_name_or_default(ccx, impl_id, im.ident); + + trans_fn_ref_with_vtables(bcx, m_id, 0, + substs, Some(vtables)).llfn + } }; @@ -903,7 +878,7 @@ pub fn trans_trait_cast(bcx: block, // Store the vtable into the pair or triple. let orig = /*bad*/copy ccx.maps.vtable_map.get(&id)[0]; let orig = resolve_vtable_in_fn_ctxt(bcx.fcx, orig); - let vtable = get_vtable(bcx.ccx(), v_ty, orig); + let vtable = get_vtable(bcx, v_ty, orig); Store(bcx, vtable, PointerCast(bcx, GEPi(bcx, lldest, [0u, abi::trt_field_vtable]), T_ptr(val_ty(vtable)))); diff --git a/src/test/run-pass/traits-default-method-self.rs b/src/test/run-pass/traits-default-method-self.rs index c17b45d6ea1..65009bb716d 100644 --- a/src/test/run-pass/traits-default-method-self.rs +++ b/src/test/run-pass/traits-default-method-self.rs @@ -8,9 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -//xfail-test - -// Currently failing with an ICE in trans. (FIXME: #4350) +#[allow(default_methods)]; trait Cat { fn meow(&self) -> bool; From 5835158170cd4cb0abc0518de37d67b3f887ad2e Mon Sep 17 00:00:00 2001 From: Michael Sullivan Date: Tue, 11 Jun 2013 14:47:57 -0700 Subject: [PATCH 2/3] Drop some dead method handling code. --- src/librustc/middle/typeck/check/method.rs | 51 +--------------------- 1 file changed, 2 insertions(+), 49 deletions(-) diff --git a/src/librustc/middle/typeck/check/method.rs b/src/librustc/middle/typeck/check/method.rs index 19acadfc572..b2c9d27241d 100644 --- a/src/librustc/middle/typeck/check/method.rs +++ b/src/librustc/middle/typeck/check/method.rs @@ -192,7 +192,7 @@ impl<'self> LookupContext<'self> { // Prepare the list of candidates self.push_inherent_candidates(self_ty); - self.push_extension_candidates(self_ty); + self.push_extension_candidates(); let mut enum_dids = ~[]; let mut self_ty = self_ty; @@ -326,7 +326,7 @@ impl<'self> LookupContext<'self> { } } - pub fn push_extension_candidates(&self, self_ty: ty::t) { + pub fn push_extension_candidates(&self) { // If the method being called is associated with a trait, then // find all the impls of that trait. Each of those are // candidates. @@ -343,17 +343,8 @@ impl<'self> LookupContext<'self> { for impl_infos.each |impl_info| { self.push_candidates_from_impl( self.extension_candidates, *impl_info); - } - } - // Look for default methods. - match self.tcx().provided_methods.find(trait_did) { - Some(&methods) => { - self.push_candidates_from_provided_methods( - self.extension_candidates, self_ty, *trait_did, - methods); } - None => {} } } } @@ -580,44 +571,6 @@ impl<'self> LookupContext<'self> { }); } - pub fn push_candidates_from_provided_methods(&self, - candidates: - &mut ~[Candidate], - self_ty: ty::t, - trait_def_id: def_id, - methods: - &mut ~[@ProvidedMethodInfo]) - { - debug!("(pushing candidates from provided methods) considering trait \ - id %d:%d", - trait_def_id.crate, - trait_def_id.node); - - for methods.each |provided_method_info| { - if provided_method_info.method_info.ident != self.m_name { loop; } - - debug!("(pushing candidates from provided methods) adding \ - candidate"); - - let method = ty::method(self.tcx(), - provided_method_info.method_info.did); - - // FIXME #4099 (?) Needs to support generics. - let dummy_substs = substs { - self_r: None, - self_ty: None, - tps: ~[] - }; - - candidates.push(Candidate { - rcvr_ty: self_ty, - rcvr_substs: dummy_substs, - method_ty: method, - origin: method_static(provided_method_info.method_info.did) - }); - } - } - // ______________________________________________________________________ // Candidate selection (see comment at start of file) From 36e3d64c3e6030d661d48bf881ed2fef58170be6 Mon Sep 17 00:00:00 2001 From: Michael Sullivan Date: Wed, 12 Jun 2013 11:05:03 -0700 Subject: [PATCH 3/3] Fix a lot of the handling of default methods and type parameters. Closes #4099, #4102. --- src/librustc/middle/trans/base.rs | 4 +- src/librustc/middle/trans/callee.rs | 77 +++++++++++++++++-- src/librustc/middle/trans/monomorphize.rs | 23 ++---- .../trait-default-method-bound-subst.rs | 4 +- .../trait-default-method-bound-subst2.rs | 2 +- 5 files changed, 84 insertions(+), 26 deletions(-) diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index 43a07821513..b5a298464d7 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -506,9 +506,11 @@ pub fn get_res_dtor(ccx: @CrateContext, did }; assert_eq!(did.crate, ast::local_crate); + let tsubsts = ty::substs { self_r: None, self_ty: None, + tps: /*bad*/ substs.to_owned() }; let (val, _) = monomorphize::monomorphic_fn(ccx, did, - substs, + &tsubsts, None, None, None); diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs index 840bad92b19..52df3abfbb5 100644 --- a/src/librustc/middle/trans/callee.rs +++ b/src/librustc/middle/trans/callee.rs @@ -41,6 +41,7 @@ use middle::trans::meth; use middle::trans::monomorphize; use middle::trans::type_of; use middle::ty; +use middle::subst::Subst; use middle::typeck; use util::ppaux::Repr; @@ -230,11 +231,75 @@ pub fn trans_fn_ref_with_vtables( // Polytype of the function item (may have type params) let fn_tpt = ty::lookup_item_type(tcx, def_id); - // Modify the def_id if this is a default method; we want to be - // monomorphizing the trait's code. - let (def_id, opt_impl_did) = match tcx.provided_method_sources.find(&def_id) { - None => (def_id, None), - Some(source) => (source.method_id, Some(source.impl_id)) + let substs = ty::substs { self_r: None, self_ty: None, + tps: /*bad*/ type_params.to_owned() }; + + + // We need to do a bunch of special handling for default methods. + // We need to modify the def_id and our substs in order to monomorphize + // the function. + let (def_id, opt_impl_did, substs) = match tcx.provided_method_sources.find(&def_id) { + None => (def_id, None, substs), + Some(source) => { + // There are two relevant substitutions when compiling + // default methods. First, there is the substitution for + // the type parameters of the impl we are using and the + // method we are calling. This substitution is the substs + // argument we already have. + // In order to compile a default method, though, we need + // to consider another substitution: the substitution for + // the type parameters on trait; the impl we are using + // implements the trait at some particular type + // parameters, and we need to substitute for those first. + // So, what we need to do is find this substitution and + // compose it with the one we already have. + + // In order to find the substitution for the trait params, + // we look up the impl in the ast map, find its trait_ref + // id, then look up its trait ref. I feel like there + // should be a better way. + let map_node = session::expect( + ccx.sess, + ccx.tcx.items.find_copy(&source.impl_id.node), + || fmt!("couldn't find node while monomorphizing \ + default method: %?", source.impl_id.node)); + let item = match map_node { + ast_map::node_item(item, _) => item, + _ => ccx.tcx.sess.bug("Not an item") + }; + let ast_trait_ref = match copy item.node { + ast::item_impl(_, Some(tr), _, _) => tr, + _ => ccx.tcx.sess.bug("Not an impl with trait_ref") + }; + let trait_ref = ccx.tcx.trait_refs.get(&ast_trait_ref.ref_id); + + // The substs from the trait_ref only substitues for the + // trait parameters. Our substitution also needs to be + // able to substitute for the actual method type + // params. To do this, we figure out how many method + // parameters there are and pad out the substitution with + // substitution for the variables. + let item_ty = ty::lookup_item_type(tcx, source.method_id); + let num_params = item_ty.generics.type_param_defs.len() - + trait_ref.substs.tps.len(); + let id_subst = do vec::from_fn(num_params) |i| { + ty::mk_param(tcx, i, ast::def_id {crate: 0, node: 0}) + }; + // Merge the two substitions together now. + let first_subst = ty::substs {tps: trait_ref.substs.tps + id_subst, + .. trait_ref.substs}; + + // And compose them. + let new_substs = first_subst.subst(tcx, &substs); + debug!("trans_fn_with_vtables - default method: \ + substs = %s, id_subst = %s, trait_subst = %s, \ + first_subst = %s, new_subst = %s", + substs.repr(tcx), + id_subst.repr(tcx), trait_ref.substs.repr(tcx), + first_subst.repr(tcx), new_substs.repr(tcx)); + + (source.method_id, Some(source.impl_id), new_substs) + } }; // Check whether this fn has an inlined copy and, if so, redirect @@ -279,7 +344,7 @@ pub fn trans_fn_ref_with_vtables( assert_eq!(def_id.crate, ast::local_crate); let mut (val, must_cast) = - monomorphize::monomorphic_fn(ccx, def_id, type_params, + monomorphize::monomorphic_fn(ccx, def_id, &substs, vtables, opt_impl_did, Some(ref_id)); if must_cast && ref_id != 0 { // Monotype of the REFERENCE to the function (type params diff --git a/src/librustc/middle/trans/monomorphize.rs b/src/librustc/middle/trans/monomorphize.rs index 866daabff33..35ff3a56df4 100644 --- a/src/librustc/middle/trans/monomorphize.rs +++ b/src/librustc/middle/trans/monomorphize.rs @@ -43,7 +43,7 @@ use syntax::abi::AbiSet; pub fn monomorphic_fn(ccx: @CrateContext, fn_id: ast::def_id, - real_substs: &[ty::t], + real_substs: &ty::substs, vtables: Option, impl_did_opt: Option, ref_id: Option) @@ -61,17 +61,17 @@ pub fn monomorphic_fn(ccx: @CrateContext, impl_did_opt.repr(ccx.tcx), ref_id); - assert!(real_substs.all(|t| !ty::type_needs_infer(*t))); + assert!(real_substs.tps.all(|t| !ty::type_needs_infer(*t))); let _icx = ccx.insn_ctxt("monomorphic_fn"); let mut must_cast = false; - let substs = vec::map(real_substs, |t| { + let substs = vec::map(real_substs.tps, |t| { match normalize_for_monomorphization(ccx.tcx, *t) { Some(t) => { must_cast = true; t } None => *t } }); - for real_substs.each() |s| { assert!(!ty::type_has_params(*s)); } + for real_substs.tps.each() |s| { assert!(!ty::type_has_params(*s)); } for substs.each() |s| { assert!(!ty::type_has_params(*s)); } let param_uses = type_use::type_uses_for(ccx, fn_id, substs.len()); let hash_id = make_mono_id(ccx, fn_id, substs, vtables, impl_did_opt, @@ -145,17 +145,8 @@ pub fn monomorphic_fn(ccx: @CrateContext, ast_map::node_struct_ctor(_, i, pt) => (pt, i.ident, i.span) }; - // Look up the impl type if we're translating a default method. - // XXX: Generics. - let impl_ty_opt; - match impl_did_opt { - None => impl_ty_opt = None, - Some(impl_did) => { - impl_ty_opt = Some(ty::lookup_item_type(ccx.tcx, impl_did).ty); - } - } - - let mono_ty = ty::subst_tps(ccx.tcx, substs, impl_ty_opt, llitem_ty); + let mono_ty = ty::subst_tps(ccx.tcx, substs, + real_substs.self_ty, llitem_ty); let llfty = type_of_fn_from_ty(ccx, mono_ty); ccx.stats.n_monos += 1; @@ -186,7 +177,7 @@ pub fn monomorphic_fn(ccx: @CrateContext, tys: substs, vtables: vtables, type_param_defs: tpt.generics.type_param_defs, - self_ty: impl_ty_opt + self_ty: real_substs.self_ty }); let lldecl = match map_node { diff --git a/src/test/run-pass/trait-default-method-bound-subst.rs b/src/test/run-pass/trait-default-method-bound-subst.rs index dc0af7f7d54..adabafc082a 100644 --- a/src/test/run-pass/trait-default-method-bound-subst.rs +++ b/src/test/run-pass/trait-default-method-bound-subst.rs @@ -8,10 +8,10 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// xfail-test +#[allow(default_methods)]; trait A { - fn g(x: T, y: U) -> (T, U) { (x, y) } + fn g(&self, x: T, y: U) -> (T, U) { (x, y) } } impl A for int { } diff --git a/src/test/run-pass/trait-default-method-bound-subst2.rs b/src/test/run-pass/trait-default-method-bound-subst2.rs index 93cc752527b..dee9782d5ae 100644 --- a/src/test/run-pass/trait-default-method-bound-subst2.rs +++ b/src/test/run-pass/trait-default-method-bound-subst2.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// xfail-test +#[allow(default_methods)]; trait A { fn g(&self, x: T) -> T { x }