Allow rcvrs to be borrowed; check rcvrs in borrowck properly

Author: nikomatsakis

src/rustc/middle/borrowck/check_loans.rs

@@ -145,12 +145,22 @@ impl methods for check_loan_ctxt {
 
     // when we are in a pure context, we check each call to ensure
     // that the function which is invoked is itself pure.
-    fn check_pure_callee_or_arg(pc: purity_cause, expr: @ast::expr) {
+    //
+    // note: we take opt_expr and expr_id separately because for
+    // overloaded operators the callee has an id but no expr.
+    // annoying.
+    fn check_pure_callee_or_arg(pc: purity_cause,
+                                opt_expr: option<@ast::expr>,
+                                callee_id: ast::node_id,
+                                callee_span: span) {
         let tcx = self.tcx();
 
-        #debug["check_pure_callee_or_arg(pc=%?, expr=%s, ty=%s)",
-               pc, pprust::expr_to_str(expr),
-               ty_to_str(self.tcx(), tcx.ty(expr))];
+        #debug["check_pure_callee_or_arg(pc=%?, expr=%?, \
+                callee_id=%d, ty=%s)",
+               pc,
+               opt_expr.map({|e| pprust::expr_to_str(e)}),
+               callee_id,
+               ty_to_str(self.tcx(), ty::node_id_to_type(tcx, callee_id))];
 
         // Purity rules: an expr B is a legal callee or argument to a
         // call within a pure function A if at least one of the
@@ -161,29 +171,35 @@ impl methods for check_loan_ctxt {
         // (c) B is a pure fn;
         // (d) B is not a fn.
 
-        alt expr.node {
-          ast::expr_path(_) if pc == pc_pure_fn {
-            let def = self.tcx().def_map.get(expr.id);
-            let did = ast_util::def_id_of_def(def);
-            let is_fn_arg =
-                did.crate == ast::local_crate &&
-                self.fn_args.contains(did.node);
-            if is_fn_arg { ret; } // case (a) above
-          }
-          ast::expr_fn_block(*) | ast::expr_fn(*) | ast::expr_loop_body(*) {
-            if self.is_stack_closure(expr.id) { ret; } // case (b) above
+        alt opt_expr {
+          some(expr) {
+            alt expr.node {
+              ast::expr_path(_) if pc == pc_pure_fn {
+                let def = self.tcx().def_map.get(expr.id);
+                let did = ast_util::def_id_of_def(def);
+                let is_fn_arg =
+                    did.crate == ast::local_crate &&
+                    self.fn_args.contains(did.node);
+                if is_fn_arg { ret; } // case (a) above
+              }
+              ast::expr_fn_block(*) | ast::expr_fn(*) |
+              ast::expr_loop_body(*) {
+                if self.is_stack_closure(expr.id) { ret; } // case (b) above
+              }
+              _ {}
+            }
           }
-          _ {}
+          none {}
         }
 
-        let expr_ty = tcx.ty(expr);
-        alt ty::get(expr_ty).struct {
+        let callee_ty = ty::node_id_to_type(tcx, callee_id);
+        alt ty::get(callee_ty).struct {
           ty::ty_fn(fn_ty) {
             alt fn_ty.purity {
               ast::pure_fn { ret; } // case (c) above
               ast::impure_fn | ast::unsafe_fn | ast::crust_fn {
                 self.report_purity_error(
-                    pc, expr.span,
+                    pc, callee_span,
                     #fmt["access to %s function",
                          pprust::purity_to_str(fn_ty.purity)]);
               }
@@ -429,6 +445,39 @@ impl methods for check_loan_ctxt {
             ret;
         }
     }
+
+    fn check_call(expr: @ast::expr,
+                  callee: option<@ast::expr>,
+                  callee_id: ast::node_id,
+                  callee_span: span,
+                  args: [@ast::expr]) {
+        alt self.purity(expr.id) {
+          none {}
+          some(pc) {
+            self.check_pure_callee_or_arg(
+                pc, callee, callee_id, callee_span);
+            for args.each { |arg|
+                self.check_pure_callee_or_arg(
+                    pc, some(arg), arg.id, arg.span);
+            }
+          }
+        }
+        let arg_tys =
+            ty::ty_fn_args(
+                ty::node_id_to_type(self.tcx(), callee_id));
+        vec::iter2(args, arg_tys) { |arg, arg_ty|
+            alt ty::resolved_mode(self.tcx(), arg_ty.mode) {
+              ast::by_move {
+                self.check_move_out(arg);
+              }
+              ast::by_mutbl_ref {
+                self.check_assignment(at_mutbl_ref, arg);
+              }
+              ast::by_ref | ast::by_copy | ast::by_val {
+              }
+            }
+        }
+    }
 }
 
 fn check_loans_in_fn(fk: visit::fn_kind, decl: ast::fn_decl, body: ast::blk,
@@ -521,26 +570,24 @@ fn check_loans_in_expr(expr: @ast::expr,
         }
       }
       ast::expr_call(f, args, _) {
-        alt self.purity(expr.id) {
-          none {}
-          some(pc) {
-            self.check_pure_callee_or_arg(pc, f);
-            for args.each { |arg| self.check_pure_callee_or_arg(pc, arg) }
-          }
-        }
-        let arg_tys = ty::ty_fn_args(ty::expr_ty(self.tcx(), f));
-        vec::iter2(args, arg_tys) { |arg, arg_ty|
-            alt ty::resolved_mode(self.tcx(), arg_ty.mode) {
-              ast::by_move {
-                self.check_move_out(arg);
-              }
-              ast::by_mutbl_ref {
-                self.check_assignment(at_mutbl_ref, arg);
-              }
-              ast::by_ref | ast::by_copy | ast::by_val {
-              }
-            }
-        }
+        self.check_call(expr, some(f), f.id, f.span, args);
+      }
+      ast::expr_index(_, rval) |
+      ast::expr_binary(_, _, rval)
+      if self.bccx.method_map.contains_key(expr.id) {
+        self.check_call(expr,
+                        none,
+                        ast_util::op_expr_callee_id(expr),
+                        expr.span,
+                        [rval]);
+      }
+      ast::expr_unary(_, _)
+      if self.bccx.method_map.contains_key(expr.id) {
+        self.check_call(expr,
+                        none,
+                        ast_util::op_expr_callee_id(expr),
+                        expr.span,
+                        []);
       }
       _ { }
     }

src/rustc/middle/borrowck/gather_loans.rs

@@ -112,19 +112,33 @@ fn req_loans_in_expr(ex: @ast::expr,
         }
       }
 
-      ast::expr_field(rcvr, _, _) |
+      ast::expr_index(rcvr, _) |
       ast::expr_binary(_, rcvr, _) |
       ast::expr_unary(_, rcvr) if self.bccx.method_map.contains_key(ex.id) {
         // Receivers in method calls are always passed by ref.
         //
-        // FIXME--this scope is both too large and too small.  We make
-        // the scope the enclosing block, which surely includes any
-        // immediate call (a.b()) but which is too big.  OTOH, in the
-        // case of a naked field `a.b`, the value is copied
-        // anyhow. This is probably best fixed if we address the
-        // syntactic ambiguity.
-
-        // let scope_r = ty::re_scope(ex.id);
+        // Here, in an overloaded operator, the call is this expression,
+        // and hence the scope of the borrow is this call.
+        //
+        // FIXME/NOT REALLY---technically we should check the other
+        // argument and consider the argument mode.  But how annoying.
+        // And this problem when goes away when argument modes are
+        // phased out.  So I elect to leave this undone.
+        let scope_r = ty::re_scope(ex.id);
+        let rcvr_cmt = self.bccx.cat_expr(rcvr);
+        self.guarantee_valid(rcvr_cmt, m_imm, scope_r);
+      }
+
+      ast::expr_field(rcvr, _, _)
+      if self.bccx.method_map.contains_key(ex.id) {
+        // Receivers in method calls are always passed by ref.
+        //
+        // Here, the field a.b is in fact a closure.  Eventually, this
+        // should be an fn&, but for now it's an fn@.  In any case,
+        // the enclosing scope is either the call where it is a rcvr
+        // (if used like `a.b(...)`), the call where it's an argument
+        // (if used like `x(a.b)`), or the block (if used like `let x
+        // = a.b`).
         let scope_r = ty::re_scope(self.tcx().region_map.get(ex.id));
         let rcvr_cmt = self.bccx.cat_expr(rcvr);
         self.guarantee_valid(rcvr_cmt, m_imm, scope_r);

src/rustc/middle/trans/base.rs

@@ -2883,7 +2883,7 @@ fn trans_arg_expr(cx: block, arg: ty::arg, lldestty: TypeRef, e: @ast::expr,
       none { trans_temp_lval(cx, e) }
     };
     #debug("   pre-adaptation value: %s", val_str(lv.bcx.ccx().tn, lv.val));
-    let lv = adapt_borrowed_value(lv, arg, e);
+    let {lv, arg} = adapt_borrowed_value(lv, arg, e);
     let mut bcx = lv.bcx;
     let mut val = lv.val;
     #debug("   adapted value: %s", val_str(bcx.ccx().tn, val));
@@ -2897,6 +2897,8 @@ fn trans_arg_expr(cx: block, arg: ty::arg, lldestty: TypeRef, e: @ast::expr,
     } else if arg_mode == ast::by_ref || arg_mode == ast::by_val {
         let mut copied = false;
         let imm = ty::type_is_immediate(arg.ty);
+        #debug["   arg.ty=%s, imm=%b, arg_mode=%?, lv.kind=%?",
+               ty_to_str(bcx.tcx(), arg.ty), imm, arg_mode, lv.kind];
         if arg_mode == ast::by_ref && lv.kind != owned && imm {
             val = do_spill_noroot(bcx, val);
             copied = true;
@@ -2953,29 +2955,29 @@ fn load_value_from_lval_result(lv: lval_result) -> ValueRef {
     }
 }
 
-fn adapt_borrowed_value(lv: lval_result, _arg: ty::arg,
-                        e: @ast::expr) -> lval_result {
+// when invoking a method, an argument of type @T or ~T can be implicltly
+// converted to an argument of type &T. Similarly, [T] can be converted to
+// [T]/& and so on.  If such a conversion (called borrowing) is necessary,
+// then the borrowings table will have an appropriate entry inserted.  This
+// routine consults this table and performs these adaptations.  It returns a
+// new location for the borrowed result as well as a new type for the argument
+// that reflects the borrowed value and not the original.
+fn adapt_borrowed_value(lv: lval_result, arg: ty::arg,
+                        e: @ast::expr) -> {lv: lval_result,
+                                           arg: ty::arg} {
     let bcx = lv.bcx;
-    if !expr_is_borrowed(bcx, e) { ret lv; }
+    if !expr_is_borrowed(bcx, e) {
+        ret {lv:lv, arg:arg};
+    }
 
     let e_ty = expr_ty(bcx, e);
     alt ty::get(e_ty).struct {
-      ty::ty_box(mt) {
+      ty::ty_uniq(mt) | ty::ty_box(mt) {
         let box_ptr = load_value_from_lval_result(lv);
         let body_ptr = GEPi(bcx, box_ptr, [0u, abi::box_field_body]);
-        ret lval_temp(bcx, body_ptr);
-      }
-
-      ty::ty_uniq(_) {
-        let box_ptr = {
-            alt lv.kind {
-              temporary { lv.val }
-              owned { Load(bcx, lv.val) }
-              owned_imm { lv.val }
-            }
-        };
-        let body_ptr = GEPi(bcx, box_ptr, [0u, abi::box_field_body]);
-        ret lval_temp(bcx, body_ptr);
+        let rptr_ty = ty::mk_rptr(bcx.tcx(), ty::re_static, mt);
+        ret {lv: lval_temp(bcx, body_ptr),
+             arg: {ty: rptr_ty with arg}};
       }
 
       ty::ty_str | ty::ty_vec(_) |
@@ -2999,7 +3001,16 @@ fn adapt_borrowed_value(lv: lval_result, _arg: ty::arg,
 
         Store(bcx, base, GEPi(bcx, p, [0u, abi::slice_elt_base]));
         Store(bcx, len, GEPi(bcx, p, [0u, abi::slice_elt_len]));
-        ret lval_temp(bcx, p);
+
+        // this isn't necessarily the type that rust would assign but it's
+        // close enough for trans purposes, as it will have the same runtime
+        // representation
+        let slice_ty = ty::mk_evec(bcx.tcx(),
+                                   {ty: unit_ty, mutbl: ast::m_imm},
+                                   ty::vstore_slice(ty::re_static));
+
+        ret {lv: lval_temp(bcx, p),
+             arg: {ty: slice_ty with arg}};
       }
 
       _ {