From 87fa38910eb78c116cd34b2524edd9c1cfe97228 Mon Sep 17 00:00:00 2001 From: Marijn Haverbeke Date: Thu, 15 Sep 2011 14:53:34 +0200 Subject: [PATCH] Make storing returned references in a by-reference local work fn f(a: {x: str}) -> &str { ret a.x; } fn main() { let x = {x: "hi"}; let &y = f(x); // Look ma, no copy! log_err y; } Issue #918. --- src/comp/middle/alias.rs | 28 +++++++++++++++++++++--- src/comp/middle/trans.rs | 47 +++++++++++++++++++--------------------- src/comp/middle/ty.rs | 11 ---------- 3 files changed, 47 insertions(+), 39 deletions(-) diff --git a/src/comp/middle/alias.rs b/src/comp/middle/alias.rs index 8ad3a4ca9342..fe3f153fcd5d 100644 --- a/src/comp/middle/alias.rs +++ b/src/comp/middle/alias.rs @@ -182,13 +182,35 @@ fn add_bindings_for_let(cx: ctx, &bs: [binding], loc: @ast::local) { (loc.span, "can not move into a by-reference binding"); } let root = expr_root(cx.tcx, init.expr, false); + let outer_ds = *root.ds; let root_var = path_def_id(cx, root.ex); - // FIXME also allow by-ref function calls - if is_none(root_var) { + let is_temp = is_none(root_var); + if is_temp { + alt root.ex.node { + ast::expr_call(f, args) { + let fty = ty::type_autoderef(cx.tcx, ty::expr_ty(cx.tcx, f)); + let ret_style = ty::ty_fn_ret_style(cx.tcx, fty); + if ast_util::ret_by_ref(ret_style) { + // FIXME pick right arg + let arg_root = expr_root(cx.tcx, args[0], false); + root_var = path_def_id(cx, arg_root.ex); + if !is_none(root_var) { + is_temp = false; + if ret_style == ast::return_ref(true) { + outer_ds = [@{mut: true with *arg_root.ds[0]}]; + } + outer_ds = *arg_root.ds + outer_ds; + } + } + } + _ {} + } + } + if is_temp { cx.tcx.sess.span_err(loc.span, "a reference binding can't be \ rooted in a temporary"); } - for proot in *pattern_roots(cx.tcx, *root.ds, loc.node.pat) { + for proot in *pattern_roots(cx.tcx, outer_ds, loc.node.pat) { let bnd = mk_binding(cx, proot.id, proot.span, root_var, inner_mut(proot.ds)); // Don't implicitly copy explicit references diff --git a/src/comp/middle/trans.rs b/src/comp/middle/trans.rs index c341a2a8ee78..dafb91c25d73 100644 --- a/src/comp/middle/trans.rs +++ b/src/comp/middle/trans.rs @@ -2731,7 +2731,7 @@ fn trans_for_each(cx: @block_ctxt, local: @ast::local, seq: @ast::expr, ast::expr_call(f, args) { let pair = create_real_fn_pair(cx, iter_body_llty, lliterbody, llenv.ptr); - r = trans_call(cx, f, some(pair), args, seq.id); + r = trans_call(cx, f, some(pair), args, seq.id).res; ret rslt(r.bcx, C_nil()); } } @@ -3089,6 +3089,12 @@ fn trans_lval_gen(cx: @block_ctxt, e: @ast::expr) -> lval_result { } } } + ast::expr_call(f, args) { + let {res: {bcx, val}, by_ref} = + trans_call(cx, f, none, args, e.id); + if by_ref { ret lval_mem(bcx, val); } + else { ret lval_val(bcx, val); } + } _ { ret {res: trans_expr(cx, e), is_mem: false, @@ -3623,7 +3629,7 @@ fn trans_args(cx: @block_ctxt, llenv: ValueRef, gen: option::t, fn trans_call(in_cx: @block_ctxt, f: @ast::expr, lliterbody: option::t, args: [@ast::expr], - id: ast::node_id) -> result { + id: ast::node_id) -> {res: result, by_ref: bool} { // NB: 'f' isn't necessarily a function; it might be an entire self-call // expression because of the hack that allows us to process self-calls // with trans_call. @@ -3690,8 +3696,7 @@ fn trans_call(in_cx: @block_ctxt, f: @ast::expr, none. { if !ty::type_is_nil(bcx_tcx(cx), ret_ty) { if by_ref { - let retptr = Load(bcx, llretslot); - retval = load_if_immediate(bcx, retptr, ret_ty); + retval = Load(bcx, llretslot); } else { retval = load_if_immediate(bcx, llretslot, ret_ty); // Retval doesn't correspond to anything really tangible @@ -3720,7 +3725,7 @@ fn trans_call(in_cx: @block_ctxt, f: @ast::expr, Br(bcx, next_cx.llbb); bcx = next_cx; } - ret rslt(bcx, retval); + ret {res: rslt(bcx, retval), by_ref: by_ref}; } fn invoke(bcx: @block_ctxt, llfn: ValueRef, @@ -3887,9 +3892,6 @@ fn trans_expr_out(cx: @block_ctxt, e: @ast::expr, output: out_method) -> // Fixme Fill in cx.sp alt e.node { ast::expr_lit(lit) { ret trans_lit(cx, *lit); } - ast::expr_unary(op, x) { - if op != ast::deref { ret trans_unary(cx, op, x, e.id); } - } ast::expr_binary(op, x, y) { ret trans_binary(cx, op, x, y); } ast::expr_if(cond, thn, els) { ret with_out_method(bind trans_if(cx, cond, thn, els, _), cx, e.id, @@ -4044,9 +4046,6 @@ fn trans_expr_out(cx: @block_ctxt, e: @ast::expr, output: out_method) -> ret rslt(bcx, C_nil()); } ast::expr_bind(f, args) { ret trans_bind(cx, f, args, e.id); } - ast::expr_call(f, args) { - ret trans_call(cx, f, none::, args, e.id); - } ast::expr_cast(val, _) { ret trans_cast(cx, val, e.id); } ast::expr_vec(args, _) { ret tvec::trans_vec(cx, args, e.id); } ast::expr_rec(args, base) { ret trans_rec(cx, args, base, e.id); } @@ -4092,21 +4091,18 @@ fn trans_expr_out(cx: @block_ctxt, e: @ast::expr, output: out_method) -> ast::expr_anon_obj(anon_obj) { ret trans_anon_obj(cx, e.span, anon_obj, e.id); } - _ { - // The expression is an lvalue. Fall through. - assert (ty::is_lval(e)); - // make sure it really is and that we - // didn't forget to add a case for a new expr! + ast::expr_call(_, _) | ast::expr_field(_, _) | ast::expr_index(_, _) | + ast::expr_path(_) | ast::expr_unary(ast::deref., _) { + let t = ty::expr_ty(bcx_tcx(cx), e); + let sub = trans_lval(cx, e); + let v = sub.res.val; + if sub.is_mem { v = load_if_immediate(sub.res.bcx, v, t); } + ret rslt(sub.res.bcx, v); + } + ast::expr_unary(op, x) { + ret trans_unary(cx, op, x, e.id); } } - // lval cases fall through to trans_lval and then - // possibly load the result (if it's non-structural). - - let t = ty::expr_ty(bcx_tcx(cx), e); - let sub = trans_lval(cx, e); - let v = sub.res.val; - if sub.is_mem { v = load_if_immediate(sub.res.bcx, v, t); } - ret rslt(sub.res.bcx, v); } fn with_out_method(work: fn(out_method) -> result, cx: @block_ctxt, @@ -4517,7 +4513,8 @@ fn init_ref_local(bcx: @block_ctxt, local: @ast::local) -> @block_ctxt { let init_expr = option::get(local.node.init).expr; let val = trans_lval(bcx, init_expr); assert val.is_mem; - ret trans_alt::bind_irrefutable_pat(bcx, local.node.pat, val.res.val, + ret trans_alt::bind_irrefutable_pat(val.res.bcx, local.node.pat, + val.res.val, bcx.fcx.lllocals, false); } diff --git a/src/comp/middle/ty.rs b/src/comp/middle/ty.rs index eeea7d4358f0..6fbe21e60de0 100644 --- a/src/comp/middle/ty.rs +++ b/src/comp/middle/ty.rs @@ -50,7 +50,6 @@ export fm_general; export get_element_type; export hash_ty; export idx_nil; -export is_lval; export is_binopable; export is_pred_ty; export lookup_item_type; @@ -1713,16 +1712,6 @@ fn sort_methods(meths: [method]) -> [method] { ret std::sort::merge_sort::(bind method_lteq(_, _), meths); } -fn is_lval(expr: @ast::expr) -> bool { - alt expr.node { - ast::expr_field(_, _) { ret true; } - ast::expr_index(_, _) { ret true; } - ast::expr_path(_) { ret true; } - ast::expr_unary(ast::deref., _) { ret true; } - _ { ret false; } - } -} - fn occurs_check_fails(tcx: ctxt, sp: option::t, vid: int, rt: t) -> bool { if !type_contains_vars(tcx, rt) {