From 5e26808141592354a4c9ab28a7e2190e9acf8942 Mon Sep 17 00:00:00 2001 From: Michael Sullivan Date: Wed, 19 Jun 2013 12:50:12 -0700 Subject: [PATCH 1/4] Get rid of terrible way for iterating over provided methods. --- src/librustc/metadata/csearch.rs | 7 +---- src/librustc/metadata/decoder.rs | 13 +++------- src/librustc/middle/ty.rs | 6 ++--- src/librustc/middle/typeck/coherence.rs | 34 +++++-------------------- 4 files changed, 14 insertions(+), 46 deletions(-) diff --git a/src/librustc/metadata/csearch.rs b/src/librustc/metadata/csearch.rs index e1cd7caa19c1..bfc5d512b370 100644 --- a/src/librustc/metadata/csearch.rs +++ b/src/librustc/metadata/csearch.rs @@ -24,11 +24,6 @@ use syntax::ast; use syntax::ast_map; use syntax::diagnostic::expect; -pub struct ProvidedTraitMethodInfo { - ty: ty::Method, - def_id: ast::def_id -} - pub struct StaticMethodInfo { ident: ast::ident, def_id: ast::def_id, @@ -134,7 +129,7 @@ pub fn get_trait_method_def_ids(cstore: @mut cstore::CStore, pub fn get_provided_trait_methods(tcx: ty::ctxt, def: ast::def_id) - -> ~[ProvidedTraitMethodInfo] { + -> ~[@ty::Method] { let cstore = tcx.cstore; let cdata = cstore::get_crate_data(cstore, def.crate); decoder::get_provided_trait_methods(cstore.intr, cdata, def.node, tcx) diff --git a/src/librustc/metadata/decoder.rs b/src/librustc/metadata/decoder.rs index 714164d6a120..824f67b074c2 100644 --- a/src/librustc/metadata/decoder.rs +++ b/src/librustc/metadata/decoder.rs @@ -14,7 +14,7 @@ use core::prelude::*; use metadata::cstore::crate_metadata; use metadata::common::*; -use metadata::csearch::{ProvidedTraitMethodInfo, StaticMethodInfo}; +use metadata::csearch::StaticMethodInfo; use metadata::csearch; use metadata::cstore; use metadata::decoder; @@ -752,7 +752,7 @@ pub fn get_trait_method_def_ids(cdata: cmd, pub fn get_provided_trait_methods(intr: @ident_interner, cdata: cmd, id: ast::node_id, tcx: ty::ctxt) -> - ~[ProvidedTraitMethodInfo] { + ~[@ty::Method] { let data = cdata.data; let item = lookup_item(id, data); let mut result = ~[]; @@ -763,13 +763,8 @@ pub fn get_provided_trait_methods(intr: @ident_interner, cdata: cmd, if item_method_sort(mth) != 'p' { loop; } - let ty_method = get_method(intr, cdata, did.node, tcx); - let provided_trait_method_info = ProvidedTraitMethodInfo { - ty: ty_method, - def_id: did - }; - - vec::push(&mut result, provided_trait_method_info); + vec::push(&mut result, + @get_method(intr, cdata, did.node, tcx)); } return result; diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 328b331ea63e..a7a69d51de26 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -3649,7 +3649,7 @@ pub fn def_has_ty_params(def: ast::def) -> bool { } } -pub fn provided_trait_methods(cx: ctxt, id: ast::def_id) -> ~[ast::ident] { +pub fn provided_trait_methods(cx: ctxt, id: ast::def_id) -> ~[@Method] { if is_local(id) { match cx.items.find(&id.node) { Some(&ast_map::node_item(@ast::item { @@ -3657,13 +3657,13 @@ pub fn provided_trait_methods(cx: ctxt, id: ast::def_id) -> ~[ast::ident] { _ }, _)) => match ast_util::split_trait_methods(*ms) { - (_, p) => p.map(|method| method.ident) + (_, p) => p.map(|m| method(cx, ast_util::local_def(m.id))) }, _ => cx.sess.bug(fmt!("provided_trait_methods: %? is not a trait", id)) } } else { - csearch::get_provided_trait_methods(cx, id).map(|ifo| ifo.ty.ident) + csearch::get_provided_trait_methods(cx, id) } } diff --git a/src/librustc/middle/typeck/coherence.rs b/src/librustc/middle/typeck/coherence.rs index ae62e768ea2b..bd99a8e150b7 100644 --- a/src/librustc/middle/typeck/coherence.rs +++ b/src/librustc/middle/typeck/coherence.rs @@ -333,7 +333,8 @@ impl CoherenceChecker { let impl_poly_type = ty::lookup_item_type(tcx, impl_id); - for self.each_provided_trait_method(trait_ref.def_id) |trait_method| { + let provided = ty::provided_trait_methods(tcx, trait_ref.def_id); + for provided.iter().advance |trait_method| { // Synthesize an ID. let new_id = parse::next_node_id(tcx.sess.parse_sess); let new_did = local_def(new_id); @@ -347,7 +348,7 @@ impl CoherenceChecker { impl_id, trait_ref, new_did, - trait_method); + *trait_method); debug!("new_method_ty=%s", new_method_ty.repr(tcx)); @@ -526,29 +527,6 @@ impl CoherenceChecker { } } - pub fn each_provided_trait_method(&self, - trait_did: ast::def_id, - f: &fn(x: @ty::Method) -> bool) - -> bool { - // Make a list of all the names of the provided methods. - // XXX: This is horrible. - let mut provided_method_idents = HashSet::new(); - let tcx = self.crate_context.tcx; - let r = ty::provided_trait_methods(tcx, trait_did); - for r.iter().advance |ident| { - provided_method_idents.insert(*ident); - } - - for ty::trait_methods(tcx, trait_did).iter().advance |&method| { - if provided_method_idents.contains(&method.ident) { - if !f(method) { - return false; - } - } - } - return true; - } - pub fn polytypes_unify(&self, polytype_a: ty_param_bounds_and_ty, polytype_b: ty_param_bounds_and_ty) @@ -729,9 +707,9 @@ impl CoherenceChecker { } // Default methods let r = ty::provided_trait_methods(tcx, trait_did); - for r.iter().advance |ident| { - debug!("inserting provided method %s", ident.repr(tcx)); - provided_names.insert(*ident); + for r.iter().advance |method| { + debug!("inserting provided method %s", method.ident.repr(tcx)); + provided_names.insert(method.ident); } let r = ty::trait_methods(tcx, trait_did); From 13e5f0ebdfa7305e8449a585aa8e9837703f1f96 Mon Sep 17 00:00:00 2001 From: Michael Sullivan Date: Wed, 19 Jun 2013 18:03:02 -0700 Subject: [PATCH 2/4] Remove some essentially dead code in method handling. --- src/librustc/middle/trans/base.rs | 2 +- src/librustc/middle/trans/meth.rs | 33 ++++------------------- src/librustc/middle/trans/monomorphize.rs | 4 +-- 3 files changed, 8 insertions(+), 31 deletions(-) diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index 0e322c187af2..526e4859415b 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -2110,7 +2110,7 @@ pub fn trans_item(ccx: @mut CrateContext, item: &ast::item) { } ast::item_impl(ref generics, _, _, ref ms) => { meth::trans_impl(ccx, /*bad*/copy *path, item.ident, *ms, - generics, None, item.id); + generics, item.id); } ast::item_mod(ref m) => { trans_mod(ccx, m); diff --git a/src/librustc/middle/trans/meth.rs b/src/librustc/middle/trans/meth.rs index 0b68ae5ee171..92a588700bb7 100644 --- a/src/librustc/middle/trans/meth.rs +++ b/src/librustc/middle/trans/meth.rs @@ -49,13 +49,12 @@ pub fn trans_impl(ccx: @mut CrateContext, name: ast::ident, methods: &[@ast::method], generics: &ast::Generics, - self_ty: Option, id: ast::node_id) { let _icx = push_ctxt("impl::trans_impl"); let tcx = ccx.tcx; - debug!("trans_impl(path=%s, name=%s, self_ty=%s, id=%?)", - path.repr(tcx), name.repr(tcx), self_ty.repr(tcx), id); + debug!("trans_impl(path=%s, name=%s, id=%?)", + path.repr(tcx), name.repr(tcx), id); if !generics.ty_params.is_empty() { return; } let sub_path = vec::append_one(path, path_name(name)); @@ -65,24 +64,10 @@ pub fn trans_impl(ccx: @mut CrateContext, let path = vec::append_one(/*bad*/copy sub_path, path_name(method.ident)); - let param_substs_opt; - match self_ty { - None => param_substs_opt = None, - Some(self_ty) => { - param_substs_opt = Some(@param_substs { - tys: ~[], - vtables: None, - type_param_defs: @~[], - self_ty: Some(self_ty) - }); - } - } - trans_method(ccx, path, *method, - param_substs_opt, - self_ty, + None, llfn, ast_util::local_def(id)); } @@ -98,9 +83,6 @@ Translates a (possibly monomorphized) method body. - `method`: the AST node for the method - `param_substs`: if this is a generic method, the current values for type parameters and so forth, else none -- `base_self_ty`: optionally, the explicit self type for this method. This - will be none if this is not a default method and must always be present - if this is a default method. - `llfn`: the LLVM ValueRef for the method - `impl_id`: the node ID of the impl this method is inside */ @@ -108,7 +90,6 @@ pub fn trans_method(ccx: @mut CrateContext, path: path, method: &ast::method, param_substs: Option<@param_substs>, - base_self_ty: Option, llfn: ValueRef, impl_id: ast::def_id) { // figure out how self is being passed @@ -119,18 +100,14 @@ pub fn trans_method(ccx: @mut CrateContext, _ => { // determine the (monomorphized) type that `self` maps to for // this method - let self_ty = match base_self_ty { - None => ty::node_id_to_type(ccx.tcx, method.self_id), - Some(provided_self_ty) => provided_self_ty, - }; + let self_ty = ty::node_id_to_type(ccx.tcx, method.self_id); let self_ty = match param_substs { None => self_ty, Some(@param_substs {tys: ref tys, _}) => { ty::subst_tps(ccx.tcx, *tys, None, self_ty) } }; - debug!("calling trans_fn with base_self_ty %s, self_ty %s", - base_self_ty.repr(ccx.tcx), + debug!("calling trans_fn with self_ty %s", self_ty.repr(ccx.tcx)); match method.explicit_self.node { ast::sty_value => { diff --git a/src/librustc/middle/trans/monomorphize.rs b/src/librustc/middle/trans/monomorphize.rs index 4f4bbf84a72b..1ffe26e3affe 100644 --- a/src/librustc/middle/trans/monomorphize.rs +++ b/src/librustc/middle/trans/monomorphize.rs @@ -233,14 +233,14 @@ pub fn monomorphic_fn(ccx: @mut CrateContext, Some(override_impl_did) => impl_did = override_impl_did } - meth::trans_method(ccx, pt, mth, psubsts, None, d, impl_did); + meth::trans_method(ccx, pt, mth, psubsts, d, impl_did); d } ast_map::node_trait_method(@ast::provided(mth), _, pt) => { let d = mk_lldecl(); set_inline_hint_if_appr(/*bad*/copy mth.attrs, d); debug!("monomorphic_fn impl_did_opt is %?", impl_did_opt); - meth::trans_method(ccx, /*bad*/copy *pt, mth, psubsts, None, d, + meth::trans_method(ccx, /*bad*/copy *pt, mth, psubsts, d, impl_did_opt.get()); d } From 3c867114525068275bd403ba5b1918d507a38933 Mon Sep 17 00:00:00 2001 From: Michael Sullivan Date: Fri, 21 Jun 2013 12:38:10 -0700 Subject: [PATCH 3/4] Fix some tests in rustpkg to not pollute the build directory. Closes #7278. --- src/librustpkg/tests.rs | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/librustpkg/tests.rs b/src/librustpkg/tests.rs index 973b960008be..19293e31167a 100644 --- a/src/librustpkg/tests.rs +++ b/src/librustpkg/tests.rs @@ -268,12 +268,12 @@ fn output_file_name(workspace: &Path, short_name: &str) -> Path { workspace.push(fmt!("%s%s", short_name, os::EXE_SUFFIX)) } -fn touch_source_file(workspace: &Path, short_name: &str) { +fn touch_source_file(workspace: &Path, pkgid: &PkgId) { use conditions::bad_path::cond; - let pkg_src_dir = workspace.push("src").push(short_name); - let contents = os::list_dir(&pkg_src_dir); + let pkg_src_dir = workspace.push("src").push(pkgid.to_str()); + let contents = os::list_dir_path(&pkg_src_dir); for contents.iter().advance |p| { - if Path(copy *p).filetype() == Some(~".rs") { + if p.filetype() == Some(~".rs") { // should be able to do this w/o a process if run::process_output("touch", [p.to_str()]).status != 0 { let _ = cond.raise((copy pkg_src_dir, ~"Bad path")); @@ -287,20 +287,19 @@ fn touch_source_file(workspace: &Path, short_name: &str) { fn frob_source_file(workspace: &Path, pkgid: &PkgId) { use conditions::bad_path::cond; let pkg_src_dir = workspace.push("src").push(pkgid.to_str()); - let contents = os::list_dir(&pkg_src_dir); + let contents = os::list_dir_path(&pkg_src_dir); let mut maybe_p = None; for contents.iter().advance |p| { - if Path(copy *p).filetype() == Some(~".rs") { + if p.filetype() == Some(~".rs") { maybe_p = Some(p); break; } } match maybe_p { Some(p) => { - let p = Path(copy *p); - let w = io::buffered_file_writer(&p); + let w = io::file_writer(*p, &[io::Append]); match w { - Err(s) => { let _ = cond.raise((p, fmt!("Bad path: %s", s))); } + Err(s) => { let _ = cond.raise((copy **p, fmt!("Bad path: %s", s))); } Ok(w) => w.write_line("") } } @@ -615,7 +614,7 @@ fn do_rebuild_dep_dates_change() { let workspace = create_local_package_with_dep(&p_id, &dep_id); command_line_test([~"build", ~"foo"], &workspace); let bar_date = datestamp(&lib_output_file_name(&workspace, "build", "bar")); - touch_source_file(&workspace, "bar"); + touch_source_file(&workspace, &dep_id); command_line_test([~"build", ~"foo"], &workspace); let new_bar_date = datestamp(&lib_output_file_name(&workspace, "build", "bar")); assert!(new_bar_date > bar_date); From 663f29818310c7aad3b1501fe8eac6ca2379e037 Mon Sep 17 00:00:00 2001 From: Michael Sullivan Date: Mon, 24 Jun 2013 10:34:18 -0700 Subject: [PATCH 4/4] Get rid of cast on every self reference. --- src/librustc/middle/trans/base.rs | 27 +++++++++++++-------------- src/librustc/middle/trans/expr.rs | 8 +------- src/librustc/middle/trans/meth.rs | 4 ++-- 3 files changed, 16 insertions(+), 23 deletions(-) diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index 526e4859415b..ddf12f0df035 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -1683,21 +1683,20 @@ pub fn copy_args_to_allocas(fcx: fn_ctxt, match fcx.llself { Some(slf) => { - // We really should do this regardless of whether self is owned, but - // it doesn't work right with default method impls yet. (FIXME: #2794) - if slf.is_owned { - let self_val = if datum::appropriate_mode(slf.t).is_by_value() { - let tmp = BitCast(bcx, slf.v, type_of(bcx.ccx(), slf.t)); - let alloc = alloc_ty(bcx, slf.t); - Store(bcx, tmp, alloc); - alloc - } else { - PointerCast(bcx, slf.v, type_of(bcx.ccx(), slf.t).ptr_to()) - }; + let self_val = if slf.is_owned + && datum::appropriate_mode(slf.t).is_by_value() { + let tmp = BitCast(bcx, slf.v, type_of(bcx.ccx(), slf.t)); + let alloc = alloc_ty(bcx, slf.t); + Store(bcx, tmp, alloc); + alloc + } else { + PointerCast(bcx, slf.v, type_of(bcx.ccx(), slf.t).ptr_to()) + }; - fcx.llself = Some(ValSelfData {v: self_val, ..slf}); - add_clean(bcx, self_val, slf.t); - } + fcx.llself = Some(ValSelfData {v: self_val, ..slf}); + if slf.is_owned { + add_clean(bcx, self_val, slf.t); + } } _ => {} } diff --git a/src/librustc/middle/trans/expr.rs b/src/librustc/middle/trans/expr.rs index 1aebf73b81a8..0e64d7582ac5 100644 --- a/src/librustc/middle/trans/expr.rs +++ b/src/librustc/middle/trans/expr.rs @@ -1051,14 +1051,8 @@ pub fn trans_local_var(bcx: block, def: ast::def) -> Datum { debug!("def_self() reference, self_info.t=%s", self_info.t.repr(bcx.tcx())); - // This cast should not be necessary. We should cast self *once*, - // but right now this conflicts with default methods. - let real_self_ty = monomorphize_type(bcx, self_info.t); - let llselfty = type_of::type_of(bcx.ccx(), real_self_ty).ptr_to(); - - let casted_val = PointerCast(bcx, self_info.v, llselfty); Datum { - val: casted_val, + val: self_info.v, ty: self_info.t, mode: ByRef(ZeroMem) } diff --git a/src/librustc/middle/trans/meth.rs b/src/librustc/middle/trans/meth.rs index 92a588700bb7..96f8a1976a68 100644 --- a/src/librustc/middle/trans/meth.rs +++ b/src/librustc/middle/trans/meth.rs @@ -103,8 +103,8 @@ pub fn trans_method(ccx: @mut CrateContext, let self_ty = ty::node_id_to_type(ccx.tcx, method.self_id); let self_ty = match param_substs { None => self_ty, - Some(@param_substs {tys: ref tys, _}) => { - ty::subst_tps(ccx.tcx, *tys, None, self_ty) + Some(@param_substs {tys: ref tys, self_ty: ref self_sub, _}) => { + ty::subst_tps(ccx.tcx, *tys, *self_sub, self_ty) } }; debug!("calling trans_fn with self_ty %s",