From 08a5278bb5cd11d7a4f986db627a585bdad878d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Steinbrink?= Date: Tue, 25 Jun 2013 22:01:37 +0200 Subject: [PATCH 1/4] Add missing argument to a debug! call in trans_arg_expr --- src/librustc/middle/trans/callee.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs index 4c07f88f16eba..8ef55f89d8173 100644 --- a/src/librustc/middle/trans/callee.rs +++ b/src/librustc/middle/trans/callee.rs @@ -930,7 +930,7 @@ pub fn trans_arg_expr(bcx: block, ByRef(_) => val = scratch.val, } } else { - debug!("by copy arg with type %s"); + debug!("by copy arg with type %s", bcx.ty_to_str(arg_datum.ty)); match arg_datum.mode { ByRef(_) => val = Load(bcx, arg_datum.val), ByValue => val = arg_datum.val, From 2a40c5db469b5065f64c69ae7874f162ff2669a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Steinbrink?= Date: Sat, 29 Jun 2013 16:43:39 +0200 Subject: [PATCH 2/4] Avoid extra casts for "self" arguments "self" is always passed as an opaque box, so there's no point in using the concrete self type when translating the argument. All it does it causing the value to be casted back to an opaque box right away. --- src/librustc/middle/trans/callee.rs | 9 +-------- src/librustc/middle/trans/meth.rs | 29 +++++++---------------------- 2 files changed, 8 insertions(+), 30 deletions(-) diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs index 8ef55f89d8173..6d139130e2be9 100644 --- a/src/librustc/middle/trans/callee.rs +++ b/src/librustc/middle/trans/callee.rs @@ -615,10 +615,7 @@ pub fn trans_call_inner(in_cx: block, } Method(d) => { // Weird but true: we pass self in the *environment* slot! - let llself = PointerCast(bcx, - d.llself, - Type::opaque_box(ccx).ptr_to()); - (d.llfn, llself) + (d.llfn, d.llself) } Closure(d) => { // Closures are represented as (llfn, llclosure) pair: @@ -944,10 +941,6 @@ pub fn trans_arg_expr(bcx: block, if formal_arg_ty != arg_datum.ty { // this could happen due to e.g. subtyping let llformal_arg_ty = type_of::type_of_explicit_arg(ccx, &formal_arg_ty); - let llformal_arg_ty = match self_mode { - ty::ByRef => llformal_arg_ty.ptr_to(), - ty::ByCopy => llformal_arg_ty, - }; debug!("casting actual type (%s) to match formal (%s)", bcx.val_to_str(val), bcx.llty_str(llformal_arg_ty)); val = PointerCast(bcx, val, llformal_arg_ty); diff --git a/src/librustc/middle/trans/meth.rs b/src/librustc/middle/trans/meth.rs index 0b051662781e0..fa32dd5b93ef4 100644 --- a/src/librustc/middle/trans/meth.rs +++ b/src/librustc/middle/trans/meth.rs @@ -133,8 +133,8 @@ pub fn trans_self_arg(bcx: block, let _icx = push_ctxt("impl::trans_self_arg"); let mut temp_cleanups = ~[]; - // Compute the type of self. - let self_ty = monomorphize_type(bcx, mentry.self_ty); + // self is passed as an opaque box in the environment slot + let self_ty = ty::mk_opaque_box(bcx.tcx()); let result = trans_arg_expr(bcx, self_ty, mentry.self_mode, @@ -576,7 +576,6 @@ pub fn trans_trait_callee_from_llval(bcx: block, let llbox = Load(bcx, GEPi(bcx, llpair, [0u, abi::trt_field_box])); // Munge `llself` appropriately for the type of `self` in the method. - let self_mode; match explicit_self { ast::sty_static => { bcx.tcx().sess.bug("shouldn't see static method here"); @@ -597,12 +596,6 @@ pub fn trans_trait_callee_from_llval(bcx: block, llself = llbox; } } - - let llscratch = alloca(bcx, val_ty(llself)); - Store(bcx, llself, llscratch); - llself = llscratch; - - self_mode = ty::ByRef; } ast::sty_box(_) => { // Bump the reference count on the box. @@ -615,12 +608,6 @@ pub fn trans_trait_callee_from_llval(bcx: block, ty::BoxTraitStore => llself = llbox, _ => bcx.tcx().sess.bug("@self receiver with non-@Trait") } - - let llscratch = alloca(bcx, val_ty(llself)); - Store(bcx, llself, llscratch); - llself = llscratch; - - self_mode = ty::ByRef; } ast::sty_uniq(_) => { // Pass the unique pointer. @@ -628,15 +615,13 @@ pub fn trans_trait_callee_from_llval(bcx: block, ty::UniqTraitStore => llself = llbox, _ => bcx.tcx().sess.bug("~self receiver with non-~Trait") } - - let llscratch = alloca(bcx, val_ty(llself)); - Store(bcx, llself, llscratch); - llself = llscratch; - - self_mode = ty::ByRef; } } + let llscratch = alloca(bcx, val_ty(llself)); + Store(bcx, llself, llscratch); + llself = PointerCast(bcx, llscratch, Type::opaque_box(ccx).ptr_to()); + // Load the function from the vtable and cast it to the expected type. debug!("(translating trait callee) loading method"); let llcallee_ty = type_of_fn_from_ty(ccx, callee_ty); @@ -652,7 +637,7 @@ pub fn trans_trait_callee_from_llval(bcx: block, llfn: mptr, llself: llself, self_ty: ty::mk_opaque_box(bcx.tcx()), - self_mode: self_mode, + self_mode: ty::ByRef, explicit_self: explicit_self /* XXX: Some(llbox) */ }) From ba922a4027f407dd28ab0e699837109ca99ff1ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Steinbrink?= Date: Sat, 29 Jun 2013 17:45:59 +0200 Subject: [PATCH 3/4] Fix handling of temp cleanups for the "self" argument The code that tried to revoke the cleanup for the self argument tried to use "llself" to do so, but the cleanup might actually be registered with a different ValueRef due to e.g. casting. Currently, this is worked around by early revocation of the cleanup for self in trans_self_arg. To handle this correctly, we have to put the ValueRef for the cleanup into the MethodData, so trans_call_inner can use it to revoke the cleanup when it's actually supposed to. --- src/librustc/middle/trans/callee.rs | 8 +++--- src/librustc/middle/trans/meth.rs | 40 +++++++++++++---------------- 2 files changed, 23 insertions(+), 25 deletions(-) diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs index 6d139130e2be9..821f5c7950fea 100644 --- a/src/librustc/middle/trans/callee.rs +++ b/src/librustc/middle/trans/callee.rs @@ -62,6 +62,7 @@ pub struct FnData { pub struct MethodData { llfn: ValueRef, llself: ValueRef, + temp_cleanup: Option, self_ty: ty::t, self_mode: ty::SelfMode, explicit_self: ast::explicit_self_ @@ -646,9 +647,10 @@ pub fn trans_call_inner(in_cx: block, // Now that the arguments have finished evaluating, we need to revoke // the cleanup for the self argument, if it exists match callee.data { - Method(d) if d.self_mode == ty::ByCopy || - d.explicit_self == ast::sty_value => { - revoke_clean(bcx, d.llself); + Method(d) => { + for d.temp_cleanup.iter().advance |&v| { + revoke_clean(bcx, v); + } } _ => {} } diff --git a/src/librustc/middle/trans/meth.rs b/src/librustc/middle/trans/meth.rs index fa32dd5b93ef4..3884a2d03f670 100644 --- a/src/librustc/middle/trans/meth.rs +++ b/src/librustc/middle/trans/meth.rs @@ -129,28 +129,20 @@ pub fn trans_method(ccx: @mut CrateContext, pub fn trans_self_arg(bcx: block, base: @ast::expr, + temp_cleanups: &mut ~[ValueRef], mentry: typeck::method_map_entry) -> Result { let _icx = push_ctxt("impl::trans_self_arg"); - let mut temp_cleanups = ~[]; // self is passed as an opaque box in the environment slot let self_ty = ty::mk_opaque_box(bcx.tcx()); - let result = trans_arg_expr(bcx, - self_ty, - mentry.self_mode, - mentry.explicit_self, - base, - &mut temp_cleanups, - None, - DontAutorefArg); - - // FIXME(#3446)---this is wrong, actually. The temp_cleanups - // should be revoked only after all arguments have been passed. - for temp_cleanups.iter().advance |c| { - revoke_clean(bcx, *c) - } - - return result; + trans_arg_expr(bcx, + self_ty, + mentry.self_mode, + mentry.explicit_self, + base, + temp_cleanups, + None, + DontAutorefArg) } pub fn trans_method_callee(bcx: block, @@ -203,12 +195,14 @@ pub fn trans_method_callee(bcx: block, match origin { typeck::method_static(did) => { let callee_fn = callee::trans_fn_ref(bcx, did, callee_id); - let Result {bcx, val} = trans_self_arg(bcx, this, mentry); + let mut temp_cleanups = ~[]; + let Result {bcx, val} = trans_self_arg(bcx, this, &mut temp_cleanups, mentry); Callee { bcx: bcx, data: Method(MethodData { llfn: callee_fn.llfn, llself: val, + temp_cleanup: temp_cleanups.head_opt().map(|&v| *v), self_ty: node_id_type(bcx, this.id), self_mode: mentry.self_mode, explicit_self: mentry.explicit_self @@ -254,9 +248,8 @@ pub fn trans_method_callee(bcx: block, store, mentry.explicit_self) } - typeck::method_super(*) => { - fail!("method_super should have been handled \ - above") + typeck::method_super(*) => { + fail!("method_super should have been handled above") } } } @@ -413,8 +406,9 @@ pub fn trans_monomorphized_callee(bcx: block, bcx.ccx(), impl_did, mname); // obtain the `self` value: + let mut temp_cleanups = ~[]; let Result {bcx, val: llself_val} = - trans_self_arg(bcx, base, mentry); + trans_self_arg(bcx, base, &mut temp_cleanups, mentry); // create a concatenated set of substitutions which includes // those from the impl and those from the method: @@ -441,6 +435,7 @@ pub fn trans_monomorphized_callee(bcx: block, data: Method(MethodData { llfn: llfn_val, llself: llself_val, + temp_cleanup: temp_cleanups.head_opt().map(|&v| *v), self_ty: node_id_type(bcx, base.id), self_mode: mentry.self_mode, explicit_self: mentry.explicit_self @@ -636,6 +631,7 @@ pub fn trans_trait_callee_from_llval(bcx: block, data: Method(MethodData { llfn: mptr, llself: llself, + temp_cleanup: None, self_ty: ty::mk_opaque_box(bcx.tcx()), self_mode: ty::ByRef, explicit_self: explicit_self From 765a2901d58625f5995c9cf4a32c85b6573e12b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Steinbrink?= Date: Fri, 28 Jun 2013 01:40:27 +0200 Subject: [PATCH 4/4] Avoid double indirection for the "self" arg in methods Currently we pass all "self" arguments by reference, for the pointer variants this means that we end up with double indirection which causes a unnecessary performance hit. The fix itself is pretty straight-forward and just means that "self" needs to be handled like any other argument, except for by-value "self" which still needs to be passed by reference. This is because non-pointer types can't just be stuffed into the environment slot which is used to pass "self". What made things tricky is that there was also a bug in the typechecker where the method map entries are created. For type impls, that stored the base type instead of the actual self-type in the method map, e.g. Foo instead of &Foo for &self. That worked with pass-by-reference, but fails with pass-by-value which needs the real type. Code that makes use of methods seems to be about 10% faster with this change. Also, build times are reduced by about 4%. Fixes #4355, #4402, #5280, #4406 and #7285 --- src/librustc/middle/trans/asm.rs | 3 -- src/librustc/middle/trans/base.rs | 29 +++++++++--------- src/librustc/middle/trans/callee.rs | 33 ++++++++------------ src/librustc/middle/trans/common.rs | 2 +- src/librustc/middle/trans/glue.rs | 14 ++------- src/librustc/middle/trans/inline.rs | 6 ++-- src/librustc/middle/trans/meth.rs | 35 ++++++++++------------ src/librustc/middle/typeck/check/method.rs | 13 +++++--- 8 files changed, 57 insertions(+), 78 deletions(-) diff --git a/src/librustc/middle/trans/asm.rs b/src/librustc/middle/trans/asm.rs index a1d1b737f31f2..81a69dfca248c 100644 --- a/src/librustc/middle/trans/asm.rs +++ b/src/librustc/middle/trans/asm.rs @@ -41,7 +41,6 @@ pub fn trans_inline_asm(bcx: block, ia: &ast::inline_asm) -> block { callee::trans_arg_expr(bcx, expr_ty(bcx, out), ty::ByCopy, - ast::sty_static, out, &mut cleanups, None, @@ -57,7 +56,6 @@ pub fn trans_inline_asm(bcx: block, ia: &ast::inline_asm) -> block { callee::trans_arg_expr(bcx, expr_ty(bcx, e), ty::ByCopy, - ast::sty_static, e, &mut cleanups, None, @@ -79,7 +77,6 @@ pub fn trans_inline_asm(bcx: block, ia: &ast::inline_asm) -> block { callee::trans_arg_expr(bcx, expr_ty(bcx, in), ty::ByCopy, - ast::sty_static, in, &mut cleanups, None, diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index d9fea12134684..60f4096cf9e5f 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -1626,18 +1626,11 @@ pub fn create_llargs_for_fn_args(cx: fn_ctxt, let _icx = push_ctxt("create_llargs_for_fn_args"); match self_arg { - impl_self(tt) => { + impl_self(tt, self_mode) => { cx.llself = Some(ValSelfData { v: cx.llenv, t: tt, - is_owned: false - }); - } - impl_owned_self(tt) => { - cx.llself = Some(ValSelfData { - v: cx.llenv, - t: tt, - is_owned: true + is_copy: self_mode == ty::ByCopy }); } no_self => () @@ -1676,12 +1669,18 @@ pub fn copy_args_to_allocas(fcx: fn_ctxt, match fcx.llself { Some(slf) => { - let self_val = PointerCast(bcx, slf.v, type_of(bcx.ccx(), slf.t).ptr_to()); - fcx.llself = Some(ValSelfData {v: self_val, ..slf}); + let self_val = if slf.is_copy + && 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()) + }; - if slf.is_owned { - add_clean(bcx, slf.v, slf.t); - } + fcx.llself = Some(ValSelfData {v: self_val, ..slf}); + add_clean(bcx, self_val, slf.t); } _ => {} } @@ -1758,7 +1757,7 @@ pub fn tie_up_header_blocks(fcx: fn_ctxt, lltop: BasicBlockRef) { } } -pub enum self_arg { impl_self(ty::t), impl_owned_self(ty::t), no_self, } +pub enum self_arg { impl_self(ty::t, ty::SelfMode), no_self, } // trans_closure: Builds an LLVM function out of a source function. // If the function closes over its environment a closure will be diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs index 821f5c7950fea..ffeb4f2754af2 100644 --- a/src/librustc/middle/trans/callee.rs +++ b/src/librustc/middle/trans/callee.rs @@ -65,7 +65,6 @@ pub struct MethodData { temp_cleanup: Option, self_ty: ty::t, self_mode: ty::SelfMode, - explicit_self: ast::explicit_self_ } pub enum CalleeData { @@ -645,7 +644,7 @@ pub fn trans_call_inner(in_cx: block, // Now that the arguments have finished evaluating, we need to revoke - // the cleanup for the self argument, if it exists + // the cleanup for the self argument match callee.data { Method(d) => { for d.temp_cleanup.iter().advance |&v| { @@ -771,7 +770,6 @@ pub fn trans_args(cx: block, trans_arg_expr(bcx, arg_tys[i], ty::ByCopy, - ast::sty_static, *arg_expr, &mut temp_cleanups, if i == last { ret_flag } else { None }, @@ -805,7 +803,6 @@ pub enum AutorefArg { pub fn trans_arg_expr(bcx: block, formal_arg_ty: ty::t, self_mode: ty::SelfMode, - ex_self: ast::explicit_self_, arg_expr: @ast::expr, temp_cleanups: &mut ~[ValueRef], ret_flag: Option, @@ -813,10 +810,9 @@ pub fn trans_arg_expr(bcx: block, let _icx = push_ctxt("trans_arg_expr"); let ccx = bcx.ccx(); - debug!("trans_arg_expr(formal_arg_ty=(%s), explicit_self=%? self_mode=%?, arg_expr=%s, \ + debug!("trans_arg_expr(formal_arg_ty=(%s), self_mode=%?, arg_expr=%s, \ ret_flag=%?)", formal_arg_ty.repr(bcx.tcx()), - ex_self, self_mode, arg_expr.repr(bcx.tcx()), ret_flag.map(|v| bcx.val_to_str(*v))); @@ -876,9 +872,15 @@ pub fn trans_arg_expr(bcx: block, val = arg_datum.to_ref_llval(bcx); } DontAutorefArg => { - match (self_mode, ex_self) { - (ty::ByRef, ast::sty_value) => { - debug!("by value self with type %s, storing to scratch", + match self_mode { + ty::ByRef => { + // This assertion should really be valid, but because + // the explicit self code currently passes by-ref, it + // does not hold. + // + //assert !bcx.ccx().maps.moves_map.contains_key( + // &arg_expr.id); + debug!("by ref arg with type %s, storing to scratch", bcx.ty_to_str(arg_datum.ty)); let scratch = scratch_datum(bcx, arg_datum.ty, false); @@ -895,18 +897,7 @@ pub fn trans_arg_expr(bcx: block, val = scratch.to_ref_llval(bcx); } - (ty::ByRef, _) => { - // This assertion should really be valid, but because - // the explicit self code currently passes by-ref, it - // does not hold. - // - //assert !bcx.ccx().maps.moves_map.contains_key( - // &arg_expr.id); - debug!("by ref arg with type %s", - bcx.ty_to_str(arg_datum.ty)); - val = arg_datum.to_ref_llval(bcx); - } - (ty::ByCopy, _) => { + ty::ByCopy => { if ty::type_needs_drop(bcx.tcx(), arg_datum.ty) || arg_datum.appropriate_mode().is_by_ref() { debug!("by copy arg with type %s, storing to scratch", diff --git a/src/librustc/middle/trans/common.rs b/src/librustc/middle/trans/common.rs index b255f2ca78c7f..229cacef2fedf 100644 --- a/src/librustc/middle/trans/common.rs +++ b/src/librustc/middle/trans/common.rs @@ -125,7 +125,7 @@ pub type ExternMap = HashMap<@str, ValueRef>; pub struct ValSelfData { v: ValueRef, t: ty::t, - is_owned: bool + is_copy: bool, } // Here `self_ty` is the real type of the self parameter to this method. It diff --git a/src/librustc/middle/trans/glue.rs b/src/librustc/middle/trans/glue.rs index 0172b70546593..9ca174d3f2de2 100644 --- a/src/librustc/middle/trans/glue.rs +++ b/src/librustc/middle/trans/glue.rs @@ -425,12 +425,7 @@ pub fn trans_struct_drop_flag(bcx: block, t: ty::t, v0: ValueRef, dtor_did: ast: // just consist of the environment (self) assert_eq!(params.len(), 1); - // Take a reference to the class (because it's using the Drop trait), - // do so now. - let llval = alloca(bcx, val_ty(v0)); - Store(bcx, v0, llval); - - let self_arg = PointerCast(bcx, llval, params[0]); + let self_arg = PointerCast(bcx, v0, params[0]); let args = ~[self_arg]; Call(bcx, dtor_addr, args); @@ -465,12 +460,7 @@ pub fn trans_struct_drop(mut bcx: block, t: ty::t, v0: ValueRef, dtor_did: ast:: // just consist of the environment (self) assert_eq!(params.len(), 1); - // Take a reference to the class (because it's using the Drop trait), - // do so now. - let llval = alloca(bcx, val_ty(v0)); - Store(bcx, v0, llval); - - let self_arg = PointerCast(bcx, llval, params[0]); + let self_arg = PointerCast(bcx, v0, params[0]); let args = ~[self_arg]; Call(bcx, dtor_addr, args); diff --git a/src/librustc/middle/trans/inline.rs b/src/librustc/middle/trans/inline.rs index 11c02f165b6f7..0d7b5d5ed5afd 100644 --- a/src/librustc/middle/trans/inline.rs +++ b/src/librustc/middle/trans/inline.rs @@ -12,7 +12,7 @@ use core::prelude::*; use metadata::csearch; use middle::astencode; -use middle::trans::base::{push_ctxt,impl_owned_self, impl_self, no_self}; +use middle::trans::base::{push_ctxt, impl_self, no_self}; use middle::trans::base::{trans_item, get_item_val, trans_fn}; use middle::trans::common::*; use middle::ty; @@ -114,8 +114,8 @@ pub fn maybe_instantiate_inline(ccx: @mut CrateContext, fn_id: ast::def_id, debug!("calling inline trans_fn with self_ty %s", ty_to_str(ccx.tcx, self_ty)); match mth.explicit_self.node { - ast::sty_value => impl_owned_self(self_ty), - _ => impl_self(self_ty), + ast::sty_value => impl_self(self_ty, ty::ByRef), + _ => impl_self(self_ty, ty::ByCopy), } } }; diff --git a/src/librustc/middle/trans/meth.rs b/src/librustc/middle/trans/meth.rs index 3884a2d03f670..845ca34ce541b 100644 --- a/src/librustc/middle/trans/meth.rs +++ b/src/librustc/middle/trans/meth.rs @@ -20,6 +20,7 @@ use middle::trans::build::*; use middle::trans::callee::*; use middle::trans::callee; use middle::trans::common::*; +use middle::trans::datum::*; use middle::trans::expr::{SaveIn, Ignore}; use middle::trans::expr; use middle::trans::glue; @@ -107,10 +108,8 @@ pub fn trans_method(ccx: @mut CrateContext, debug!("calling trans_fn with self_ty %s", self_ty.repr(ccx.tcx)); match method.explicit_self.node { - ast::sty_value => impl_owned_self(self_ty), - _ => { - impl_self(self_ty) - } + ast::sty_value => impl_self(self_ty, ty::ByRef), + _ => impl_self(self_ty, ty::ByCopy), } } }; @@ -138,7 +137,6 @@ pub fn trans_self_arg(bcx: block, trans_arg_expr(bcx, self_ty, mentry.self_mode, - mentry.explicit_self, base, temp_cleanups, None, @@ -205,7 +203,6 @@ pub fn trans_method_callee(bcx: block, temp_cleanup: temp_cleanups.head_opt().map(|&v| *v), self_ty: node_id_type(bcx, this.id), self_mode: mentry.self_mode, - explicit_self: mentry.explicit_self }) } } @@ -438,7 +435,6 @@ pub fn trans_monomorphized_callee(bcx: block, temp_cleanup: temp_cleanups.head_opt().map(|&v| *v), self_ty: node_id_type(bcx, base.id), self_mode: mentry.self_mode, - explicit_self: mentry.explicit_self }) } } @@ -568,7 +564,8 @@ pub fn trans_trait_callee_from_llval(bcx: block, // necessary: let mut llself; debug!("(translating trait callee) loading second index from pair"); - let llbox = Load(bcx, GEPi(bcx, llpair, [0u, abi::trt_field_box])); + let llboxptr = GEPi(bcx, llpair, [0u, abi::trt_field_box]); + let llbox = Load(bcx, llboxptr); // Munge `llself` appropriately for the type of `self` in the method. match explicit_self { @@ -580,8 +577,6 @@ pub fn trans_trait_callee_from_llval(bcx: block, called on objects"); } ast::sty_region(*) => { - // As before, we need to pass a pointer to a pointer to the - // payload. match store { ty::BoxTraitStore | ty::UniqTraitStore => { @@ -596,7 +591,7 @@ pub fn trans_trait_callee_from_llval(bcx: block, // Bump the reference count on the box. debug!("(translating trait callee) callee type is `%s`", bcx.ty_to_str(callee_ty)); - bcx = glue::take_ty(bcx, llbox, callee_ty); + glue::incr_refcnt_of_boxed(bcx, llbox); // Pass a pointer to the box. match store { @@ -610,12 +605,15 @@ pub fn trans_trait_callee_from_llval(bcx: block, ty::UniqTraitStore => llself = llbox, _ => bcx.tcx().sess.bug("~self receiver with non-~Trait") } + + zero_mem(bcx, llboxptr, ty::mk_opaque_box(bcx.tcx())); } } - let llscratch = alloca(bcx, val_ty(llself)); - Store(bcx, llself, llscratch); - llself = PointerCast(bcx, llscratch, Type::opaque_box(ccx).ptr_to()); + llself = PointerCast(bcx, llself, Type::opaque_box(ccx).ptr_to()); + let scratch = scratch_datum(bcx, ty::mk_opaque_box(bcx.tcx()), false); + Store(bcx, llself, scratch.val); + scratch.add_clean(bcx); // Load the function from the vtable and cast it to the expected type. debug!("(translating trait callee) loading method"); @@ -630,11 +628,10 @@ pub fn trans_trait_callee_from_llval(bcx: block, bcx: bcx, data: Method(MethodData { llfn: mptr, - llself: llself, - temp_cleanup: None, - self_ty: ty::mk_opaque_box(bcx.tcx()), - self_mode: ty::ByRef, - explicit_self: explicit_self + llself: scratch.to_value_llval(bcx), + temp_cleanup: Some(scratch.val), + self_ty: scratch.ty, + self_mode: ty::ByCopy, /* XXX: Some(llbox) */ }) }; diff --git a/src/librustc/middle/typeck/check/method.rs b/src/librustc/middle/typeck/check/method.rs index 4bebca3c9a8b3..2fb8209ecf2ac 100644 --- a/src/librustc/middle/typeck/check/method.rs +++ b/src/librustc/middle/typeck/check/method.rs @@ -976,9 +976,7 @@ impl<'self> LookupContext<'self> { let fty = ty::mk_bare_fn(tcx, ty::BareFnTy {sig: fn_sig, ..bare_fn_ty}); debug!("after replacing bound regions, fty=%s", self.ty_to_str(fty)); - // FIXME(#7411): We always pass self by-ref since we stuff it in the environment slot. - // Eventually that should not be the case - let self_mode = ty::ByRef; + let self_mode = get_mode_from_explicit_self(candidate.method_ty.explicit_self); // before we only checked whether self_ty could be a subtype // of rcvr_ty; now we actually make it so (this may cause @@ -998,7 +996,7 @@ impl<'self> LookupContext<'self> { self.fcx.write_ty(self.callee_id, fty); self.fcx.write_substs(self.callee_id, all_substs); method_map_entry { - self_ty: candidate.rcvr_ty, + self_ty: rcvr_ty, self_mode: self_mode, explicit_self: candidate.method_ty.explicit_self, origin: candidate.origin, @@ -1253,3 +1251,10 @@ impl<'self> LookupContext<'self> { self.tcx().sess.bug(s) } } + +pub fn get_mode_from_explicit_self(explicit_self: ast::explicit_self_) -> SelfMode { + match explicit_self { + sty_value => ty::ByRef, + _ => ty::ByCopy, + } +}