Skip to content

Commit

Permalink
Get cross crate default methods working.
Browse files Browse the repository at this point in the history
This fixes the large number of problems that prevented cross crate
methods from ever working. It also fixes a couple lingering bugs with
polymorphic default methods and cleans up some of the code paths.

Closes #4102. Closes #4103.
  • Loading branch information
msullivan committed Jun 20, 2013
1 parent 6759ce4 commit 1a8969f
Show file tree
Hide file tree
Showing 10 changed files with 255 additions and 217 deletions.
2 changes: 1 addition & 1 deletion src/librustc/metadata/csearch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ pub fn get_impl_trait(tcx: ty::ctxt,
pub fn get_impl_method(cstore: @mut cstore::CStore,
def: ast::def_id,
mname: ast::ident)
-> ast::def_id {
-> Option<ast::def_id> {
let cdata = cstore::get_crate_data(cstore, def.crate);
decoder::get_impl_method(cstore.intr, cdata, def.node, mname)
}
Expand Down
41 changes: 7 additions & 34 deletions src/librustc/metadata/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ pub fn get_impl_trait(cdata: cmd,
}

pub fn get_impl_method(intr: @ident_interner, cdata: cmd, id: ast::node_id,
name: ast::ident) -> ast::def_id {
name: ast::ident) -> Option<ast::def_id> {
let items = reader::get_doc(reader::Doc(cdata.data), tag_items);
let mut found = None;
for reader::tagged_docs(find_item(id, items), tag_item_impl_method)
Expand All @@ -425,7 +425,7 @@ pub fn get_impl_method(intr: @ident_interner, cdata: cmd, id: ast::node_id,
found = Some(translate_def_id(cdata, m_did));
}
}
found.get()
found
}

pub fn get_symbol(data: @~[u8], id: ast::node_id) -> ~str {
Expand Down Expand Up @@ -755,40 +755,13 @@ pub fn get_provided_trait_methods(intr: @ident_interner, cdata: cmd,
let item = lookup_item(id, data);
let mut result = ~[];

for reader::tagged_docs(item, tag_item_trait_method) |mth| {
if item_method_sort(mth) != 'p' { loop; }

let did = item_def_id(mth, cdata);

let type_param_defs =
item_ty_param_defs(mth, tcx, cdata,
tag_items_data_item_ty_param_bounds);
let name = item_name(intr, mth);
let ty = doc_type(mth, tcx, cdata);
for reader::tagged_docs(item, tag_item_trait_method) |mth_id| {
let did = item_def_id(mth_id, cdata);
let mth = lookup_item(did.node, data);

let fty = match ty::get(ty).sty {
ty::ty_bare_fn(ref f) => copy *f,
_ => {
tcx.diag.handler().bug("get_provided_trait_methods(): id \
has non-function type");
}
};
if item_method_sort(mth) != 'p' { loop; }

let transformed_self_ty = doc_transformed_self_ty(mth, tcx, cdata);
let explicit_self = get_explicit_self(mth);

let ty_method = ty::Method::new(
name,
ty::Generics {
type_param_defs: type_param_defs,
region_param: None
},
transformed_self_ty,
fty,
explicit_self,
ast::public,
did
);
let ty_method = get_method(intr, cdata, did.node, tcx);
let provided_trait_method_info = ProvidedTraitMethodInfo {
ty: ty_method,
def_id: did
Expand Down
51 changes: 13 additions & 38 deletions src/librustc/middle/trans/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ use middle::trans::type_of;
use middle::ty;
use middle::subst::Subst;
use middle::typeck;
use middle::typeck::coherence::make_substs_for_receiver_types;
use util::ppaux::Repr;

use core::vec;
Expand Down Expand Up @@ -253,50 +254,24 @@ pub fn trans_fn_ref_with_vtables(
// 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};
let trait_ref = ty::impl_trait_ref(tcx, source.impl_id)
.expect("could not find trait_ref for impl with \
default methods");
let method = ty::method(tcx, source.method_id);

// And compose them.
// Compute the first substitution
let first_subst = make_substs_for_receiver_types(
tcx, source.impl_id, trait_ref, method);

// 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, \
substs = %s, trait_subst = %s, \
first_subst = %s, new_subst = %s",
substs.repr(tcx),
id_subst.repr(tcx), trait_ref.substs.repr(tcx),
substs.repr(tcx), trait_ref.substs.repr(tcx),
first_subst.repr(tcx), new_substs.repr(tcx));


(source.method_id, Some(source.impl_id), new_substs)
}
};
Expand Down
15 changes: 10 additions & 5 deletions src/librustc/middle/trans/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,16 @@ pub fn maybe_instantiate_inline(ccx: @mut CrateContext, fn_id: ast::def_id,
csearch::found(ast::ii_method(impl_did, mth)) => {
ccx.stats.n_inlines += 1;
ccx.external.insert(fn_id, Some(mth.id));
let impl_tpt = ty::lookup_item_type(ccx.tcx, impl_did);
let num_type_params =
impl_tpt.generics.type_param_defs.len() +
mth.generics.ty_params.len();
if translate && num_type_params == 0 {
// If this is a default method, we can't look up the
// impl type. But we aren't going to translate anyways, so don't.
if !translate { return local_def(mth.id); }

let impl_tpt = ty::lookup_item_type(ccx.tcx, impl_did);
let num_type_params =
impl_tpt.generics.type_param_defs.len() +
mth.generics.ty_params.len();

if num_type_params == 0 {
let llfn = get_item_val(ccx, mth.id);
let path = vec::append(
ty::item_path(ccx.tcx, impl_did),
Expand Down
89 changes: 33 additions & 56 deletions src/librustc/middle/trans/meth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,71 +383,48 @@ pub fn method_with_name_or_default(ccx: @mut CrateContext,
name: ast::ident) -> ast::def_id {
let imp = ccx.impl_method_cache.find_copy(&(impl_id, name));
match imp {
Some(m) => m,
None => {
let imp = 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), _
}, _) => {
let did = method_from_methods(*ms, name);
if did.is_some() {
did.get()
} else {
// Look for a default method
let pmm = ccx.tcx.provided_methods;
match pmm.find(&impl_id) {
Some(pmis) => {
for pmis.each |pmi| {
if pmi.method_info.ident == name {
debug!("pmi.method_info.did = %?", pmi.method_info.did);
return pmi.method_info.did;
}
}
fail!()
}
None => fail!()
}
}
}
_ => fail!("method_with_name")
}
} else {
csearch::get_impl_method(ccx.sess.cstore, impl_id, name)
};
Some(m) => return m,
None => {}
}

ccx.impl_method_cache.insert((impl_id, name), imp);
// None of this feels like it should be the best way to do this.
let mut did = 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) },
_ => fail!("method_with_name")
}
} else {
csearch::get_impl_method(ccx.sess.cstore, impl_id, name)
};

imp
if did.is_none() {
// Look for a default method
let pmm = ccx.tcx.provided_methods;
match pmm.find(&impl_id) {
Some(pmis) => {
for pmis.each |pmi| {
if pmi.method_info.ident == name {
debug!("pmi.method_info.did = %?",
pmi.method_info.did);
did = Some(pmi.method_info.did);
}
}
}
None => {}
}
}

let imp = did.expect("could not find method while translating");
ccx.impl_method_cache.insert((impl_id, name), imp);
imp
}

pub fn method_ty_param_count(ccx: &CrateContext, m_id: ast::def_id,
i_id: ast::def_id) -> uint {
debug!("method_ty_param_count: m_id: %?, i_id: %?", m_id, i_id);
if m_id.crate == ast::local_crate {
match ccx.tcx.items.find(&m_id.node) {
Some(&ast_map::node_method(m, _, _)) => m.generics.ty_params.len(),
None => {
match ccx.tcx.provided_method_sources.find(&m_id) {
Some(source) => {
method_ty_param_count(
ccx, source.method_id, source.impl_id)
}
None => fail!()
}
}
Some(&ast_map::node_trait_method(@ast::provided(@ref m),
_, _)) => {
m.generics.ty_params.len()
}
ref e => fail!("method_ty_param_count %?", *e)
}
} else {
csearch::get_type_param_count(ccx.sess.cstore, m_id) -
csearch::get_type_param_count(ccx.sess.cstore, i_id)
}
ty::method(ccx.tcx, m_id).generics.type_param_defs.len()
}

pub fn trans_monomorphized_callee(bcx: block,
Expand Down
4 changes: 4 additions & 0 deletions src/librustc/middle/typeck/check/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,10 @@ impl<'self> LookupContext<'self> {
if !self.impl_dups.insert(impl_info.did) {
return; // already visited
}
debug!("push_candidates_from_impl: %s %s %s",
self.m_name.repr(self.tcx()),
impl_info.ident.repr(self.tcx()),
impl_info.methods.map(|m| m.ident).repr(self.tcx()));

let idx = {
match impl_info.methods.position(|m| m.ident == self.m_name) {
Expand Down
Loading

9 comments on commit 1a8969f

@bors
Copy link
Contributor

@bors bors commented on 1a8969f Jun 21, 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@1a8969f

@bors
Copy link
Contributor

@bors bors commented on 1a8969f Jun 21, 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 = 1a8969f into auto

@bors
Copy link
Contributor

@bors bors commented on 1a8969f Jun 21, 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 = 1a8969f merged ok, testing candidate = 7560ae07

@bors
Copy link
Contributor

@bors bors commented on 1a8969f Jun 21, 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 1a8969f Jun 21, 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@1a8969f

@bors
Copy link
Contributor

@bors bors commented on 1a8969f Jun 21, 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 = 1a8969f into auto

@bors
Copy link
Contributor

@bors bors commented on 1a8969f Jun 21, 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 = 1a8969f merged ok, testing candidate = ba05af7

@bors
Copy link
Contributor

@bors bors commented on 1a8969f Jun 21, 2013

Choose a reason for hiding this comment

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

fast-forwarding master to auto = ba05af7

Please sign in to comment.