Make by-val explicit self actually work. Closes #2585.

7 files changed, 129 insertions, 64 deletions

diff --git a/src/rustc/middle/astencode.rs b/src/rustc/middle/astencode.rs
index a25fe287222..342113a17b1 100644
--- a/src/rustc/middle/astencode.rs
+++ b/src/rustc/middle/astencode.rs
@@ -412,7 +412,9 @@ trait read_method_map_entry_helper {
 impl ebml::ebml_deserializer: read_method_map_entry_helper {
     fn read_method_map_entry(xcx: extended_decode_ctxt) -> method_map_entry {
         let mme = deserialize_method_map_entry(self);
-        {derefs: mme.derefs, origin: mme.origin.tr(xcx)}
+        {derefs: mme.derefs,
+         self_mode: mme.self_mode,
+         origin: mme.origin.tr(xcx)}
     }
 }
 
diff --git a/src/rustc/middle/kind.rs b/src/rustc/middle/kind.rs
index c055c460e09..35675b9102b 100644
--- a/src/rustc/middle/kind.rs
+++ b/src/rustc/middle/kind.rs
@@ -215,6 +215,51 @@ fn check_block(b: blk, cx: ctx, v: visit::vt<ctx>) {
 
 fn check_expr(e: @expr, cx: ctx, v: visit::vt<ctx>) {
     debug!{"kind::check_expr(%s)", expr_to_str(e)};
+
+    // Handle any kind bounds on type parameters
+    do option::iter(cx.tcx.node_type_substs.find(e.id)) |ts| {
+        let bounds = match check e.node {
+          expr_path(_) => {
+            let did = ast_util::def_id_of_def(cx.tcx.def_map.get(e.id));
+            ty::lookup_item_type(cx.tcx, did).bounds
+          }
+          _ => {
+            // Type substitions should only occur on paths and
+            // method calls, so this needs to be a method call.
+            match cx.method_map.get(e.id).origin {
+              typeck::method_static(did) => {
+                // n.b.: When we encode class/impl methods, the bounds
+                // that we encode include both the class/impl bounds
+                // and then the method bounds themselves...
+                ty::lookup_item_type(cx.tcx, did).bounds
+              }
+              typeck::method_param({trait_id:trt_id,
+                                    method_num:n_mth, _}) |
+              typeck::method_trait(trt_id, n_mth) => {
+                // ...trait methods bounds, in contrast, include only the
+                // method bounds, so we must preprend the tps from the
+                // trait itself.  This ought to be harmonized.
+                let trt_bounds =
+                    ty::lookup_item_type(cx.tcx, trt_id).bounds;
+                let mth = ty::trait_methods(cx.tcx, trt_id)[n_mth];
+                @(vec::append(*trt_bounds, *mth.tps))
+              }
+            }
+          }
+        };
+        if vec::len(ts) != vec::len(*bounds) {
+            // Fail earlier to make debugging easier
+            fail fmt!("Internal error: in kind::check_expr, length \
+                       mismatch between actual and declared bounds: actual = \
+                        %s (%u tys), declared = %? (%u tys)",
+                      tys_to_str(cx.tcx, ts), ts.len(),
+                      *bounds, (*bounds).len());
+        }
+        do vec::iter2(ts, *bounds) |ty, bound| {
+            check_bounds(cx, e.id, e.span, ty, bound)
+        }
+    }
+
     match e.node {
       expr_assign(_, ex) |
       expr_unary(box(_), ex) | expr_unary(uniq(_), ex) |
@@ -266,45 +311,12 @@ fn check_expr(e: @expr, cx: ctx, v: visit::vt<ctx>) {
             i += 1u;
         }
       }
-      expr_path(_) | expr_field(_, _, _) => {
-        do option::iter(cx.tcx.node_type_substs.find(e.id)) |ts| {
-            let bounds = match check e.node {
-              expr_path(_) => {
-                let did = ast_util::def_id_of_def(cx.tcx.def_map.get(e.id));
-                ty::lookup_item_type(cx.tcx, did).bounds
-              }
-              expr_field(base, _, _) => {
-                match cx.method_map.get(e.id).origin {
-                  typeck::method_static(did) => {
-                    // n.b.: When we encode class/impl methods, the bounds
-                    // that we encode include both the class/impl bounds
-                    // and then the method bounds themselves...
-                    ty::lookup_item_type(cx.tcx, did).bounds
-                  }
-                  typeck::method_param({trait_id:trt_id,
-                                        method_num:n_mth, _}) |
-                  typeck::method_trait(trt_id, n_mth) => {
-                    // ...trait methods bounds, in contrast, include only the
-                    // method bounds, so we must preprend the tps from the
-                    // trait itself.  This ought to be harmonized.
-                    let trt_bounds =
-                        ty::lookup_item_type(cx.tcx, trt_id).bounds;
-                    let mth = ty::trait_methods(cx.tcx, trt_id)[n_mth];
-                    @(vec::append(*trt_bounds, *mth.tps))
-                  }
-                }
-              }
-            };
-            if vec::len(ts) != vec::len(*bounds) {
-              // Fail earlier to make debugging easier
-              fail fmt!{"Internal error: in kind::check_expr, length \
-                  mismatch between actual and declared bounds: actual = \
-                  %s (%u tys), declared = %? (%u tys)",
-                  tys_to_str(cx.tcx, ts), ts.len(), *bounds, (*bounds).len()};
-            }
-            do vec::iter2(ts, *bounds) |ty, bound| {
-                check_bounds(cx, e.id, e.span, ty, bound)
-            }
+      expr_field(lhs, _, _) => {
+        // If this is a method call with a by-val argument, we need
+        // to check the copy
+        match cx.method_map.find(e.id) {
+          some({self_mode: by_copy, _}) => maybe_copy(cx, lhs),
+          _ => ()
         }
       }
       expr_repeat(element, count_expr, _) => {
diff --git a/src/rustc/middle/trans/base.rs b/src/rustc/middle/trans/base.rs
index d4eb0ff806b..0e068f68312 100644
--- a/src/rustc/middle/trans/base.rs
+++ b/src/rustc/middle/trans/base.rs
@@ -1984,7 +1984,7 @@ type lval_result = {bcx: block, val: ValueRef, kind: lval_kind};
 enum callee_env {
     null_env,
     is_closure,
-    self_env(ValueRef, ty::t, option<ValueRef>),
+    self_env(ValueRef, ty::t, option<ValueRef>, ast::rmode),
 }
 type lval_maybe_callee = {bcx: block,
                           val: ValueRef,
@@ -2447,7 +2447,9 @@ fn lookup_discriminant(ccx: @crate_ctxt, vid: ast::def_id) -> ValueRef {
     }
 }
 
-fn cast_self(cx: block, slf: val_self_pair) -> ValueRef {
+// This shouldn't exist. We should cast self *once*, but right now this
+// conflicts with default methods.
+fn cast_self(cx: block, slf: val_self_data) -> ValueRef {
     PointerCast(cx, slf.v, T_ptr(type_of(cx.ccx(), slf.t)))
 }
 
@@ -3201,7 +3203,7 @@ fn trans_call_inner(
           null_env => {
             llvm::LLVMGetUndef(T_opaque_box_ptr(ccx))
           }
-          self_env(e, _, _) => {
+          self_env(e, _, _, _) => {
             PointerCast(bcx, e, T_opaque_box_ptr(ccx))
           }
           is_closure => {
@@ -3225,6 +3227,13 @@ fn trans_call_inner(
 
         let llretslot = args_res.retslot;
 
+        // Now that the arguments have finished evaluating, we need to revoke
+        // the cleanup for the self argument, if it exists
+        match f_res.env {
+          self_env(e, _, _, ast::by_copy) => revoke_clean(bcx, e),
+          _ => (),
+        }
+
         /* If the block is terminated,
         then one or more of the args has
         type _|_. Since that means it diverges, the code
@@ -4635,7 +4644,10 @@ fn create_llargs_for_fn_args(cx: fn_ctxt,
     let mut arg_n = first_real_arg;
     match ty_self {
       impl_self(tt) => {
-        cx.llself = some({v: cx.llenv, t: tt});
+        cx.llself = some({v: cx.llenv, t: tt, is_owned: false});
+      }
+      impl_owned_self(tt) => {
+        cx.llself = some({v: cx.llenv, t: tt, is_owned: true});
       }
       no_self => ()
     }
@@ -4662,6 +4674,21 @@ fn copy_args_to_allocas(fcx: fn_ctxt, bcx: block, args: ~[ast::arg],
         tcx.sess.bug(~"someone forgot\
                 to document an invariant in copy_args_to_allocas!");
     };
+
+    match fcx.llself {
+      some(copy slf) => {
+        // We really should do this regardless of whether self is owned,
+        // but it doesn't work right with default method impls yet.
+        if slf.is_owned {
+            let self_val = PointerCast(bcx, slf.v,
+                                       T_ptr(type_of(bcx.ccx(), slf.t)));
+            fcx.llself = some({v: self_val with slf});
+            add_clean(bcx, self_val, slf.t);
+        }
+      }
+      _ => {}
+    }
+
     for vec::each(arg_tys) |arg| {
         let id = args[arg_n].id;
         let argval = match fcx.llargs.get(id) {
@@ -4705,7 +4732,7 @@ fn tie_up_header_blocks(fcx: fn_ctxt, lltop: BasicBlockRef) {
     Br(raw_block(fcx, false, fcx.llloadenv), lltop);
 }
 
-enum self_arg { impl_self(ty::t), no_self, }
+enum self_arg { impl_self(ty::t), impl_owned_self(ty::t), no_self, }
 
 // trans_closure: Builds an LLVM function out of a source function.
 // If the function closes over its environment a closure will be
@@ -4897,7 +4924,7 @@ fn trans_class_ctor(ccx: @crate_ctxt, path: path, decl: ast::fn_decl,
   }
 
   // note we don't want to take *or* drop self.
-  fcx.llself = some({v: selfptr, t: rslt_ty});
+  fcx.llself = some({v: selfptr, t: rslt_ty, is_owned: false});
 
   // Translate the body of the ctor
   bcx = trans_block(bcx_top, body, ignore);
diff --git a/src/rustc/middle/trans/common.rs b/src/rustc/middle/trans/common.rs
index 7930b596217..8414995181f 100644
--- a/src/rustc/middle/trans/common.rs
+++ b/src/rustc/middle/trans/common.rs
@@ -141,7 +141,7 @@ type crate_ctxt = {
      mut do_not_commit_warning_issued: bool};
 
 // Types used for llself.
-type val_self_pair = {v: ValueRef, t: ty::t};
+type val_self_data = {v: ValueRef, t: ty::t, is_owned: bool};
 
 enum local_val { local_mem(ValueRef), local_imm(ValueRef), }
 
@@ -177,7 +177,7 @@ type fn_ctxt = @{
     mut llreturn: BasicBlockRef,
     // The 'self' value currently in use in this function, if there
     // is one.
-    mut llself: option<val_self_pair>,
+    mut llself: option<val_self_data>,
     // The a value alloca'd for calls to upcalls.rust_personality. Used when
     // outputting the resume instruction.
     mut personality: option<ValueRef>,
diff --git a/src/rustc/middle/trans/impl.rs b/src/rustc/middle/trans/impl.rs
index 1d7771b57d5..07923695183 100644
--- a/src/rustc/middle/trans/impl.rs
+++ b/src/rustc/middle/trans/impl.rs
@@ -35,12 +35,11 @@ fn trans_impl(ccx: @crate_ctxt, path: path, name: ast::ident,
               ast::sty_uniq(_) => {
                 impl_self(ty::mk_imm_uniq(ccx.tcx, self_ty))
               }
-              // XXX: Is this right at all?
               ast::sty_region(*) => {
                 impl_self(ty::mk_imm_ptr(ccx.tcx, self_ty))
               }
               ast::sty_value => {
-                ccx.sess.unimpl(~"by value self type not implemented");
+                impl_owned_self(self_ty)
               }
               ast::sty_by_ref => { impl_self(self_ty) }
             };
@@ -54,18 +53,22 @@ fn trans_impl(ccx: @crate_ctxt, path: path, name: ast::ident,
     }
 }
 
-fn trans_self_arg(bcx: block, base: @ast::expr, derefs: uint) -> result {
+fn trans_self_arg(bcx: block, base: @ast::expr,
+                  mentry: typeck::method_map_entry) -> result {
     let _icx = bcx.insn_ctxt("impl::trans_self_arg");
     let basety = expr_ty(bcx, base);
-    let m_by_ref = ast::expl(ast::by_ref);
+    let mode = ast::expl(mentry.self_mode);
     let mut temp_cleanups = ~[];
-    let result = trans_arg_expr(bcx, {mode: m_by_ref, ty: basety},
+    let result = trans_arg_expr(bcx, {mode: mode, ty: basety},
                                 T_ptr(type_of::type_of(bcx.ccx(), basety)),
-                                base, temp_cleanups, none, derefs);
+                                base, temp_cleanups, none, mentry.derefs);
 
     // by-ref self argument should not require cleanup in the case of
     // other arguments failing:
-    assert temp_cleanups == ~[];
+    //assert temp_cleanups == ~[];
+    //do vec::iter(temp_cleanups) |c| {
+    //    revoke_clean(bcx, c)
+    //}
 
     return result;
 }
@@ -76,8 +79,11 @@ fn trans_method_callee(bcx: block, callee_id: ast::node_id,
     let _icx = bcx.insn_ctxt("impl::trans_method_callee");
     match mentry.origin {
       typeck::method_static(did) => {
-        let {bcx, val} = trans_self_arg(bcx, self, mentry.derefs);
-        {env: self_env(val, node_id_type(bcx, self.id), none)
+
+
+        let {bcx, val} = trans_self_arg(bcx, self, mentry);
+        {env: self_env(val, node_id_type(bcx, self.id), none,
+                       mentry.self_mode)
          with lval_static_fn(bcx, did, callee_id)}
       }
       typeck::method_param({trait_id:trait_id, method_num:off,
@@ -85,7 +91,7 @@ fn trans_method_callee(bcx: block, callee_id: ast::node_id,
         match check bcx.fcx.param_substs {
           some(substs) => {
             let vtbl = find_vtable_in_fn_ctxt(substs, p, b);
-            trans_monomorphized_callee(bcx, callee_id, self, mentry.derefs,
+            trans_monomorphized_callee(bcx, callee_id, self, mentry,
                                        trait_id, off, vtbl)
           }
         }
@@ -184,7 +190,8 @@ fn method_ty_param_count(ccx: @crate_ctxt, m_id: ast::def_id,
 }
 
 fn trans_monomorphized_callee(bcx: block, callee_id: ast::node_id,
-                              base: @ast::expr, derefs: uint,
+                              base: @ast::expr,
+                              mentry: typeck::method_map_entry,
                               trait_id: ast::def_id, n_method: uint,
                               vtbl: typeck::vtable_origin)
     -> lval_maybe_callee {
@@ -200,10 +207,11 @@ fn trans_monomorphized_callee(bcx: block, callee_id: ast::node_id,
             = vec::append(impl_substs,
                           vec::tailn(node_substs,
                                      node_substs.len() - n_m_tps));
-        let {bcx, val} = trans_self_arg(bcx, base, derefs);
+        let {bcx, val} = trans_self_arg(bcx, base, mentry);
         let lval = lval_static_fn_inner(bcx, mth_id, callee_id, ty_substs,
                                         some(sub_origins));
-        {env: self_env(val, node_id_type(bcx, base.id), none),
+        {env: self_env(val, node_id_type(bcx, base.id),
+                       none, mentry.self_mode),
          val: PointerCast(bcx, lval.val, T_ptr(type_of_fn_from_ty(
              ccx, node_id_type(bcx, callee_id))))
          with lval}
@@ -230,7 +238,9 @@ fn trans_trait_callee(bcx: block, val: ValueRef,
     let llbox = Load(bcx, GEPi(bcx, val, ~[0u, 1u]));
     // FIXME[impl] I doubt this is alignment-safe (#2534)
     let self = GEPi(bcx, llbox, ~[0u, abi::box_field_body]);
-    let env = self_env(self, ty::mk_opaque_box(bcx.tcx()), some(llbox));
+    let env = self_env(self, ty::mk_opaque_box(bcx.tcx()), some(llbox),
+                       // XXX: is this bogosity?
+                       ast::by_ref);
     let llfty = type_of::type_of_fn_from_ty(ccx, callee_ty);
     let vtable = PointerCast(bcx, vtable,
                              T_ptr(T_array(T_ptr(llfty), n_method + 1u)));
diff --git a/src/rustc/middle/typeck.rs b/src/rustc/middle/typeck.rs
index 2d2bf15c5a2..ded6a2b41f8 100644
--- a/src/rustc/middle/typeck.rs
+++ b/src/rustc/middle/typeck.rs
@@ -113,6 +113,9 @@ type method_map_entry = {
     // number of derefs that are required on the receiver
     derefs: uint,
 
+    // the mode by which the self parameter needs to be passed
+    self_mode: ast::rmode,
+
     // method details being invoked
     origin: method_origin
 };
diff --git a/src/rustc/middle/typeck/check/method.rs b/src/rustc/middle/typeck/check/method.rs
index 261e6eae994..a885fa0b5c0 100644
--- a/src/rustc/middle/typeck/check/method.rs
+++ b/src/rustc/middle/typeck/check/method.rs
@@ -5,7 +5,7 @@ import middle::resolve3::{Impl, MethodInfo};
 import middle::ty::{mk_box, mk_rptr, mk_uniq};
 import syntax::ast::{def_id,
                      sty_static, sty_box, sty_by_ref, sty_region, sty_uniq};
-import syntax::ast::{sty_value};
+import syntax::ast::{sty_value, by_ref, by_copy};
 import syntax::ast_map;
 import syntax::ast_map::node_id_to_str;
 import syntax::ast_util::{dummy_sp, new_def_hash};
@@ -56,6 +56,13 @@ fn transform_self_type_for_method
     }
 }
 
+fn get_mode_from_self_type(self_type: ast::self_ty_) -> ast::rmode {
+    match self_type {
+      sty_value => by_copy,
+      _ => by_ref
+    }
+}
+
 struct lookup {
     let fcx: @fn_ctxt;
     let expr: @ast::expr;
@@ -478,6 +485,8 @@ struct lookup {
                          n_tps_m: m.n_tps,
                          fty: fty,
                          entry: {derefs: self.derefs,
+                                 self_mode: get_mode_from_self_type(
+                                     m.self_type),
                                  origin: method_static(m.did)},
                          mode: mode});
                     self.candidate_impls.insert(im.did, ());
@@ -506,7 +515,9 @@ struct lookup {
              rcvr_ty: self.self_ty,
              n_tps_m: (*m.tps).len(),
              fty: fty,
-             entry: {derefs: self.derefs, origin: origin},
+             entry: {derefs: self.derefs,
+                     self_mode: get_mode_from_self_type(m.self_ty),
+                     origin: origin},
              mode: subtyping_mode});
     }