From 767374976b71965415ada45cf8e7d9da5ddef43f Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Fri, 24 May 2013 15:33:46 -0400 Subject: [PATCH 1/2] librustc: Always pass self ByRef. --- src/librustc/middle/trans/asm.rs | 3 ++ src/librustc/middle/trans/base.rs | 24 ++++++---------- src/librustc/middle/trans/callee.rs | 33 ++++++++++++++++++---- src/librustc/middle/trans/meth.rs | 8 ++++-- src/librustc/middle/typeck/check/method.rs | 11 ++------ 5 files changed, 47 insertions(+), 32 deletions(-) diff --git a/src/librustc/middle/trans/asm.rs b/src/librustc/middle/trans/asm.rs index 3c263b1c01e..7d0defde435 100644 --- a/src/librustc/middle/trans/asm.rs +++ b/src/librustc/middle/trans/asm.rs @@ -41,6 +41,7 @@ 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, @@ -56,6 +57,7 @@ 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, @@ -77,6 +79,7 @@ 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 a0628bc8e87..6ab228cc172 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -1680,23 +1680,15 @@ pub fn copy_args_to_allocas(fcx: fn_ctxt, let mut bcx = bcx; match fcx.llself { - Some(slf) => { - 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()) - }; + 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}); - fcx.llself = Some(ValSelfData {v: self_val, ..slf}); - if slf.is_owned { - add_clean(bcx, self_val, slf.t); - } - } - _ => {} + if slf.is_owned { + add_clean(bcx, slf.v, slf.t); + } + } + _ => {} } for uint::range(0, arg_tys.len()) |arg_n| { diff --git a/src/librustc/middle/trans/callee.rs b/src/librustc/middle/trans/callee.rs index 064a457c712..78c796329d8 100644 --- a/src/librustc/middle/trans/callee.rs +++ b/src/librustc/middle/trans/callee.rs @@ -63,6 +63,7 @@ pub struct MethodData { llself: ValueRef, self_ty: ty::t, self_mode: ty::SelfMode, + explicit_self: ast::explicit_self_ } pub enum CalleeData { @@ -565,7 +566,8 @@ 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 => { + Method(d) if d.self_mode == ty::ByCopy || + d.explicit_self == ast::sty_value => { revoke_clean(bcx, d.llself); } _ => {} @@ -687,6 +689,7 @@ 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 }, @@ -720,6 +723,7 @@ 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, @@ -727,9 +731,10 @@ 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), self_mode=%?, arg_expr=%s, \ + debug!("trans_arg_expr(formal_arg_ty=(%s), explicit_self=%? 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))); @@ -789,8 +794,26 @@ pub fn trans_arg_expr(bcx: block, val = arg_datum.to_ref_llval(bcx); } DontAutorefArg => { - match self_mode { - ty::ByRef => { + match (self_mode, ex_self) { + (ty::ByRef, ast::sty_value) => { + debug!("by value self with type %s, storing to scratch", + bcx.ty_to_str(arg_datum.ty)); + let scratch = scratch_datum(bcx, arg_datum.ty, false); + + arg_datum.store_to_datum(bcx, + arg_expr.id, + INIT, + scratch); + + // Technically, ownership of val passes to the callee. + // However, we must cleanup should we fail before the + // callee is actually invoked. + scratch.add_clean(bcx); + temp_cleanups.push(scratch.val); + + 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. @@ -801,7 +824,7 @@ pub fn trans_arg_expr(bcx: block, 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/meth.rs b/src/librustc/middle/trans/meth.rs index 4f7ce6381d8..80ae2ca135e 100644 --- a/src/librustc/middle/trans/meth.rs +++ b/src/librustc/middle/trans/meth.rs @@ -110,9 +110,7 @@ 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) - } + ast::sty_value => impl_owned_self(self_ty), _ => { impl_self(self_ty) } @@ -145,6 +143,7 @@ pub fn trans_self_arg(bcx: block, let result = trans_arg_expr(bcx, self_ty, mentry.self_mode, + mentry.explicit_self, base, &mut temp_cleanups, None, @@ -231,6 +230,7 @@ pub fn trans_method_callee(bcx: block, llself: val, self_ty: node_id_type(bcx, this.id), self_mode: mentry.self_mode, + explicit_self: mentry.explicit_self }) } } @@ -453,6 +453,7 @@ pub fn trans_monomorphized_callee(bcx: block, llself: llself_val, self_ty: node_id_type(bcx, base.id), self_mode: mentry.self_mode, + explicit_self: mentry.explicit_self }) } } @@ -692,6 +693,7 @@ pub fn trans_trait_callee_from_llval(bcx: block, llself: llself, self_ty: ty::mk_opaque_box(bcx.tcx()), self_mode: self_mode, + explicit_self: explicit_self /* XXX: Some(llbox) */ }) }; diff --git a/src/librustc/middle/typeck/check/method.rs b/src/librustc/middle/typeck/check/method.rs index 777b11186c6..8c3315e49b4 100644 --- a/src/librustc/middle/typeck/check/method.rs +++ b/src/librustc/middle/typeck/check/method.rs @@ -976,7 +976,9 @@ 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)); - let self_mode = get_mode_from_explicit_self(candidate.method_ty.explicit_self); + // 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; // before we only checked whether self_ty could be a subtype // of rcvr_ty; now we actually make it so (this may cause @@ -1242,10 +1244,3 @@ 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::ByCopy, - _ => ty::ByRef, - } -} From 0aa94ff3c3419fdd58d87ad226d94ee0128f778b Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Wed, 19 Jun 2013 19:49:23 -0400 Subject: [PATCH 2/2] Add test for #5321. --- .../issue-5321-immediates-with-bare-self.rs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 src/test/run-pass/issue-5321-immediates-with-bare-self.rs diff --git a/src/test/run-pass/issue-5321-immediates-with-bare-self.rs b/src/test/run-pass/issue-5321-immediates-with-bare-self.rs new file mode 100644 index 00000000000..7b809c39cb8 --- /dev/null +++ b/src/test/run-pass/issue-5321-immediates-with-bare-self.rs @@ -0,0 +1,25 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +trait Fooable { + fn yes(self); +} + +impl Fooable for uint { + fn yes(self) { + for self.times { + println("yes"); + } + } +} + +fn main() { + 2.yes(); +}