Skip to content

Commit

Permalink
Fix a lot of the handling of default methods and type parameters. Closes
Browse files Browse the repository at this point in the history
  • Loading branch information
msullivan committed Jun 12, 2013
1 parent 5835158 commit 36e3d64
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 26 deletions.
4 changes: 3 additions & 1 deletion src/librustc/middle/trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
77 changes: 71 additions & 6 deletions src/librustc/middle/trans/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
23 changes: 7 additions & 16 deletions src/librustc/middle/trans/monomorphize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeck::vtable_res>,
impl_did_opt: Option<ast::def_id>,
ref_id: Option<ast::node_id>)
Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions src/test/run-pass/trait-default-method-bound-subst.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> {
fn g<U>(x: T, y: U) -> (T, U) { (x, y) }
fn g<U>(&self, x: T, y: U) -> (T, U) { (x, y) }
}

impl A<int> for int { }
Expand Down
2 changes: 1 addition & 1 deletion src/test/run-pass/trait-default-method-bound-subst2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> {
fn g(&self, x: T) -> T { x }
Expand Down

5 comments on commit 36e3d64

@bors
Copy link
Contributor

@bors bors commented on 36e3d64 Jun 12, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from graydon
at msullivan@36e3d64

@bors
Copy link
Contributor

@bors bors commented on 36e3d64 Jun 12, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging msullivan/rust/default-methods = 36e3d64 into auto

@bors
Copy link
Contributor

@bors bors commented on 36e3d64 Jun 12, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

msullivan/rust/default-methods = 36e3d64 merged ok, testing candidate = 84bed97

@bors
Copy link
Contributor

@bors bors commented on 36e3d64 Jun 13, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bors
Copy link
Contributor

@bors bors commented on 36e3d64 Jun 13, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding incoming to auto = 84bed97

Please sign in to comment.