wip---work on making rooting work properly

8 files changed, 175 insertions, 117 deletions

diff --git a/src/libcore/num/int-template.rs b/src/libcore/num/int-template.rs
index 85489755fc7..77b4eab1338 100644
--- a/src/libcore/num/int-template.rs
+++ b/src/libcore/num/int-template.rs
@@ -200,6 +200,8 @@ impl Mul<T,T> for T {
     #[inline(always)]
     fn mul(&self, other: &T) -> T { *self * *other }
 }
+
+#[cfg(notest)]
 impl Quot<T,T> for T {
     ///
     /// Returns the integer quotient, truncated towards 0. As this behaviour reflects
@@ -222,6 +224,8 @@ impl Quot<T,T> for T {
     #[inline(always)]
     fn quot(&self, other: &T) -> T { *self / *other }
 }
+
+#[cfg(notest)]
 impl Rem<T,T> for T {
     ///
     /// Returns the integer remainder after division, satisfying:
diff --git a/src/libcore/num/uint-template.rs b/src/libcore/num/uint-template.rs
index f975226cde6..2d204449468 100644
--- a/src/libcore/num/uint-template.rs
+++ b/src/libcore/num/uint-template.rs
@@ -165,10 +165,14 @@ impl Mul<T,T> for T {
     #[inline(always)]
     fn mul(&self, other: &T) -> T { *self * *other }
 }
+
+#[cfg(notest)]
 impl Quot<T,T> for T {
     #[inline(always)]
     fn quot(&self, other: &T) -> T { *self / *other }
 }
+
+#[cfg(notest)]
 impl Rem<T,T> for T {
     #[inline(always)]
     fn rem(&self, other: &T) -> T { *self % *other }
diff --git a/src/libcore/unstable/lang.rs b/src/libcore/unstable/lang.rs
index 0705be7cdef..d0e3c2b0678 100644
--- a/src/libcore/unstable/lang.rs
+++ b/src/libcore/unstable/lang.rs
@@ -10,6 +10,7 @@
 
 //! Runtime calls emitted by the compiler.
 
+use uint;
 use cast::transmute;
 use libc::{c_char, c_uchar, c_void, size_t, uintptr_t, c_int, STDERR_FILENO};
 use managed::raw::BoxRepr;
@@ -22,10 +23,9 @@ use task::rt::rust_get_task;
 #[allow(non_camel_case_types)]
 pub type rust_task = c_void;
 
-#[cfg(target_word_size = "32")]
-pub static FROZEN_BIT: uint = 0x80000000;
-#[cfg(target_word_size = "64")]
-pub static FROZEN_BIT: uint = 0x8000000000000000;
+pub static FROZEN_BIT: uint = 1 << (uint::bits - 1);
+pub static MUT_BIT: uint = 1 << (uint::bits - 2);
+static ALL_BITS: uint = FROZEN_BIT | MUT_BIT;
 
 pub mod rustrt {
     use unstable::lang::rust_task;
@@ -196,21 +196,51 @@ pub unsafe fn borrow_as_imm(a: *u8) {
     (*a).header.ref_count |= FROZEN_BIT;
 }
 
+fn add_borrow_to_task_list(a: *mut BoxRepr, file: *c_char, line: size_t) {
+    do swap_task_borrow_list |borrow_list| {
+        let mut borrow_list = borrow_list;
+        borrow_list.push(BorrowRecord {box: a, file: file, line: line});
+        borrow_list
+    }
+}
+
 #[cfg(not(stage0))]
 #[lang="borrow_as_imm"]
 #[inline(always)]
-pub unsafe fn borrow_as_imm(a: *u8, file: *c_char, line: size_t) {
+pub unsafe fn borrow_as_imm(a: *u8, file: *c_char, line: size_t) -> uint {
     let a: *mut BoxRepr = transmute(a);
-    (*a).header.ref_count |= FROZEN_BIT;
-    if ::rt::env::get().debug_borrows {
-        do swap_task_borrow_list |borrow_list| {
-            let mut borrow_list = borrow_list;
-            borrow_list.push(BorrowRecord {box: a, file: file, line: line});
-            borrow_list
+
+    let ref_count = (*a).header.ref_count;
+    if (ref_count & MUT_BIT) != 0 {
+        fail_borrowed(a, file, line);
+    } else {
+        (*a).header.ref_count |= FROZEN_BIT;
+        if ::rt::env::get().debug_borrows {
+            add_borrow_to_list(a, file, line);
         }
     }
+    ref_count
 }
 
+#[cfg(not(stage0))]
+#[lang="borrow_as_mut"]
+#[inline(always)]
+pub unsafe fn borrow_as_mut(a: *u8, file: *c_char, line: size_t) -> uint {
+    let a: *mut BoxRepr = transmute(a);
+
+    let ref_count = (*a).header.ref_count;
+    if (ref_count & (MUT_BIT|FROZEN_BIT)) != 0 {
+        fail_borrowed(a, file, line);
+    } else {
+        (*a).header.ref_count |= (MUT_BIT|FROZEN_BIT);
+        if ::rt::env::get().debug_borrows {
+            add_borrow_to_list(a, file, line);
+        }
+    }
+    ref_count
+}
+
+#[cfg(stage0)]
 #[lang="return_to_mut"]
 #[inline(always)]
 pub unsafe fn return_to_mut(a: *u8) {
@@ -222,6 +252,21 @@ pub unsafe fn return_to_mut(a: *u8) {
     }
 }
 
+#[cfg(not(stage0))]
+#[lang="return_to_mut"]
+#[inline(always)]
+pub unsafe fn return_to_mut(a: *u8, old_ref_count: uint) {
+    // Sometimes the box is null, if it is conditionally frozen.
+    // See e.g. #4904.
+    if !a.is_null() {
+        let a: *mut BoxRepr = transmute(a);
+
+        let ref_count = (*a).header.ref_count & !ALL_BITS;
+        let old_bits = old_ref_count & ALL_BITS;
+        (*a).header.ref_count = ref_count | old_bits;
+    }
+}
+
 #[cfg(stage0)]
 #[lang="check_not_borrowed"]
 #[inline(always)]
diff --git a/src/librustc/middle/lang_items.rs b/src/librustc/middle/lang_items.rs
index e7261d53952..0266b77997e 100644
--- a/src/librustc/middle/lang_items.rs
+++ b/src/librustc/middle/lang_items.rs
@@ -66,11 +66,12 @@ pub enum LangItem {
     MallocFnLangItem,           // 28
     FreeFnLangItem,             // 29
     BorrowAsImmFnLangItem,      // 30
-    ReturnToMutFnLangItem,      // 31
-    CheckNotBorrowedFnLangItem, // 32
-    StrDupUniqFnLangItem,       // 33
+    BorrowAsMutFnLangItem,      // 31
+    ReturnToMutFnLangItem,      // 32
+    CheckNotBorrowedFnLangItem, // 33
+    StrDupUniqFnLangItem,       // 34
 
-    StartFnLangItem,            // 34
+    StartFnLangItem,            // 35
 }
 
 pub struct LanguageItems {
@@ -128,11 +129,12 @@ pub impl LanguageItems {
             28 => "malloc",
             29 => "free",
             30 => "borrow_as_imm",
-            31 => "return_to_mut",
-            32 => "check_not_borrowed",
-            33 => "strdup_uniq",
+            31 => "borrow_as_mut",
+            32 => "return_to_mut",
+            33 => "check_not_borrowed",
+            34 => "strdup_uniq",
 
-            34 => "start",
+            35 => "start",
 
             _ => "???"
         }
@@ -237,6 +239,9 @@ pub impl LanguageItems {
     pub fn borrow_as_imm_fn(&const self) -> def_id {
         self.items[BorrowAsImmFnLangItem as uint].get()
     }
+    pub fn borrow_as_mut_fn(&const self) -> def_id {
+        self.items[BorrowAsMutFnLangItem as uint].get()
+    }
     pub fn return_to_mut_fn(&const self) -> def_id {
         self.items[ReturnToMutFnLangItem as uint].get()
     }
@@ -292,6 +297,7 @@ fn LanguageItemCollector(crate: @crate,
     item_refs.insert(@~"malloc", MallocFnLangItem as uint);
     item_refs.insert(@~"free", FreeFnLangItem as uint);
     item_refs.insert(@~"borrow_as_imm", BorrowAsImmFnLangItem as uint);
+    item_refs.insert(@~"borrow_as_mut", BorrowAsMutFnLangItem as uint);
     item_refs.insert(@~"return_to_mut", ReturnToMutFnLangItem as uint);
     item_refs.insert(@~"check_not_borrowed",
                      CheckNotBorrowedFnLangItem as uint);
diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs
index 1785feda779..7738e977799 100644
--- a/src/librustc/middle/trans/base.rs
+++ b/src/librustc/middle/trans/base.rs
@@ -991,63 +991,30 @@ pub fn get_landing_pad(bcx: block) -> BasicBlockRef {
     return pad_bcx.llbb;
 }
 
-// Arranges for the value found in `*root_loc` to be dropped once the scope
-// associated with `scope_id` exits.  This is used to keep boxes live when
-// there are extant region pointers pointing at the interior.
-//
-// Note that `root_loc` is not the value itself but rather a pointer to the
-// value.  Generally it in alloca'd value.  The reason for this is that the
-// value is initialized in an inner block but may be freed in some outer
-// block, so an SSA value that is valid in the inner block may not be valid in
-// the outer block.  In fact, the inner block may not even execute.  Rather
-// than generate the full SSA form, we just use an alloca'd value.
-pub fn add_root_cleanup(bcx: block,
-                        root_info: RootInfo,
-                        root_loc: ValueRef,
-                        ty: ty::t) {
-
-    debug!("add_root_cleanup(bcx=%s, \
-                             scope=%d, \
-                             freezes=%?, \
-                             root_loc=%s, \
-                             ty=%s)",
-           bcx.to_str(),
-           root_info.scope,
-           root_info.freeze,
-           val_str(bcx.ccx().tn, root_loc),
-           ppaux::ty_to_str(bcx.ccx().tcx, ty));
-
-    let bcx_scope = find_bcx_for_scope(bcx, root_info.scope);
-    if root_info.freeze.is_some() {
-        add_clean_frozen_root(bcx_scope, root_loc, ty);
-    } else {
-        add_clean_temp_mem(bcx_scope, root_loc, ty);
-    }
-
-    fn find_bcx_for_scope(bcx: block, scope_id: ast::node_id) -> block {
-        let mut bcx_sid = bcx;
-        loop {
-            bcx_sid = match bcx_sid.node_info {
-              Some(NodeInfo { id, _ }) if id == scope_id => {
+pub fn find_bcx_for_scope(bcx: block, scope_id: ast::node_id) -> block {
+    let mut bcx_sid = bcx;
+    loop {
+        bcx_sid = match bcx_sid.node_info {
+            Some(NodeInfo { id, _ }) if id == scope_id => {
                 return bcx_sid
               }
 
-              // NOTE This is messier than it ought to be and not really right
-              Some(NodeInfo { callee_id: Some(id), _ }) if id == scope_id => {
-                return bcx_sid
-              }
+                // NOTE This is messier than it ought to be and not really right
+                Some(NodeInfo { callee_id: Some(id), _ }) if id == scope_id => {
+                    return bcx_sid
+                }
 
-              _ => {
-                match bcx_sid.parent {
-                  None => bcx.tcx().sess.bug(
-                      fmt!("no enclosing scope with id %d", scope_id)),
-                  Some(bcx_par) => bcx_par
+                _ => {
+                    match bcx_sid.parent {
+                        None => bcx.tcx().sess.bug(
+                            fmt!("no enclosing scope with id %d", scope_id)),
+                        Some(bcx_par) => bcx_par
+                    }
                 }
-              }
             }
         }
     }
-}
+
 
 pub fn do_spill(bcx: block, v: ValueRef, t: ty::t) -> ValueRef {
     if ty::type_is_bot(t) {
diff --git a/src/librustc/middle/trans/common.rs b/src/librustc/middle/trans/common.rs
index 2ebd696dbfd..d0da8a2e1ee 100644
--- a/src/librustc/middle/trans/common.rs
+++ b/src/librustc/middle/trans/common.rs
@@ -467,28 +467,40 @@ pub fn add_clean_temp_mem(bcx: block, val: ValueRef, t: ty::t) {
         scope_clean_changed(scope_info);
     }
 }
-pub fn add_clean_frozen_root(bcx: block, val: ValueRef, t: ty::t) {
-    debug!("add_clean_frozen_root(%s, %s, %s)",
-           bcx.to_str(), val_str(bcx.ccx().tn, val),
-           t.repr(bcx.tcx()));
-    let (root, rooted) = root_for_cleanup(bcx, val, t);
-    let cleanup_type = cleanup_type(bcx.tcx(), t);
+pub fn add_clean_return_to_mut(bcx: block,
+                               frozen_val_ref: ValueRef,
+                               bits_val_ref: ValueRef) {
+    //! When an `@mut` has been frozen, we have to
+    //! call the lang-item `return_to_mut` when the
+    //! freeze goes out of scope. We need to pass
+    //! in both the value which was frozen (`frozen_val`) and
+    //! the value (`bits_val_ref`) which was returned when the
+    //! box was frozen initially. Here, both `frozen_val_ref` and
+    //! `bits_val_ref` are in fact pointers to stack slots.
+
+    debug!("add_clean_return_to_mut(%s, %s, %s)",
+           bcx.to_str(),
+           val_str(bcx.ccx().tn, frozen_val_ref),
+           val_str(bcx.ccx().tn, bits_val_ref));
     do in_scope_cx(bcx) |scope_info| {
         scope_info.cleanups.push(
-            clean_temp(val, |bcx| {
-                let bcx = callee::trans_lang_call(
-                    bcx,
-                    bcx.tcx().lang_items.return_to_mut_fn(),
-                    ~[
-                        build::Load(bcx,
-                                    build::PointerCast(bcx,
-                                                       root,
-                                                       T_ptr(T_ptr(T_i8()))))
-                    ],
-                    expr::Ignore
-                );
-                glue::drop_ty_root(bcx, root, rooted, t)
-            }, cleanup_type));
+            clean_temp(
+                frozen_val_ref,
+                |bcx| {
+                    callee::trans_lang_call(
+                        bcx,
+                        bcx.tcx().lang_items.return_to_mut_fn(),
+                        ~[
+                            build::Load(bcx,
+                                        build::PointerCast(bcx,
+                                                           frozen_val_ref,
+                                                           T_ptr(T_ptr(T_i8())))),
+                            build::Load(bcx, bits_val_ref)
+                        ],
+                        expr::Ignore
+                    )
+                },
+                normal_exit_only));
         scope_clean_changed(scope_info);
     }
 }
diff --git a/src/librustc/middle/trans/datum.rs b/src/librustc/middle/trans/datum.rs
index ae71a3e6c22..94bff65843b 100644
--- a/src/librustc/middle/trans/datum.rs
+++ b/src/librustc/middle/trans/datum.rs
@@ -87,7 +87,7 @@
 
 use lib;
 use lib::llvm::ValueRef;
-use middle::borrowck::{RootInfo, root_map_key};
+use middle::borrowck::{RootInfo, root_map_key, DynaImm, DynaMut};
 use middle::trans::adt;
 use middle::trans::base::*;
 use middle::trans::build::*;
@@ -517,7 +517,7 @@ pub impl Datum {
         }
     }
 
-    fn root(&self, bcx: block, span: span, root_info: RootInfo) -> block {
+    fn root(&self, mut bcx: block, span: span, root_info: RootInfo) -> block {
         /*!
          *
          * In some cases, borrowck will decide that an @T/@[]/@str
@@ -535,34 +535,53 @@ pub impl Datum {
                      root_info.scope));
         }
 
+        // First, root the datum. Note that we must zero this value,
+        // because sometimes we root on one path but not another.
+        // See e.g. #4904.
         let scratch = scratch_datum(bcx, self.ty, true);
         self.copy_to_datum(bcx, INIT, scratch);
-        add_root_cleanup(bcx, root_info, scratch.val, scratch.ty);
-
-        // If we need to freeze the box, do that now.
-        if root_info.freeze.is_some() {
-            // NOTE distinguish the two kinds of freezing here
-
-            let loc = bcx.sess().parse_sess.cm.lookup_char_pos(span.lo);
-            let line = C_int(bcx.ccx(), loc.line as int);
-            let filename_cstr = C_cstr(bcx.ccx(), @/*bad*/copy loc.file.name);
-            let filename = PointerCast(bcx, filename_cstr, T_ptr(T_i8()));
-
-            callee::trans_lang_call(
-                bcx,
-                bcx.tcx().lang_items.borrow_as_imm_fn(),
-                ~[
-                    Load(bcx,
-                         PointerCast(bcx,
-                                     scratch.val,
-                                     T_ptr(T_ptr(T_i8())))),
-                    filename,
-                    line
-                ],
-                expr::Ignore)
-        } else {
-            bcx
+        let cleanup_bcx = find_bcx_for_scope(bcx, root_info.scope);
+        add_clean_temp_mem(cleanup_bcx, scratch.val, scratch.ty);
+
+        // Now, consider also freezing it.
+        match root_info.freeze {
+            None => {}
+            Some(freeze_kind) => {
+                let loc = bcx.sess().parse_sess.cm.lookup_char_pos(span.lo);
+                let line = C_int(bcx.ccx(), loc.line as int);
+                let filename_cstr = C_cstr(bcx.ccx(), @/*bad*/copy loc.file.name);
+                let filename = PointerCast(bcx, filename_cstr, T_ptr(T_i8()));
+
+                // in this case, we don't have to zero, because
+                // scratch.val will be NULL should the cleanup get
+                // called without the freezing actually occurring, and
+                // return_to_mut checks for this condition.
+                let scratch_bits = scratch_datum(bcx, ty::mk_uint(), false);
+
+                let freeze_did = match freeze_kind {
+                    DynaImm => bcx.tcx().lang_items.borrow_as_imm_fn(),
+                    DynaMut => bcx.tcx().lang_items.borrow_as_mut_fn(),
+                };
+
+                bcx = callee::trans_lang_call(
+                    bcx,
+                    freeze_did,
+                    ~[
+                        Load(bcx,
+                             PointerCast(bcx,
+                                         scratch.val,
+                                         T_ptr(T_ptr(T_i8())))),
+                        filename,
+                        line
+                    ],
+                    expr::SaveIn(scratch_bits.val));
+
+                add_clean_return_to_mut(
+                    cleanup_bcx, scratch.val, scratch_bits.val);
+            }
         }
+
+        bcx
     }
 
     fn perform_write_guard(&self, bcx: block, span: span) -> block {
diff --git a/src/test/compile-fail/borrowck-loan-rcvr-overloaded-op.rs b/src/test/compile-fail/borrowck-loan-rcvr-overloaded-op.rs
index 8c9d3009e5e..2c4ae242fb4 100644
--- a/src/test/compile-fail/borrowck-loan-rcvr-overloaded-op.rs
+++ b/src/test/compile-fail/borrowck-loan-rcvr-overloaded-op.rs
@@ -40,10 +40,11 @@ fn b() {
 
     let q = &mut p;
 
-    p + 3;  // ok for pure fns
+    p + 3;  //~ ERROR cannot borrow `p`
     p.times(3); //~ ERROR cannot borrow `p`
 
-    q.x += 1;
+    *q + 3; // OK to use the new alias `q`
+    q.x += 1; // and OK to mutate it
 }
 
 fn c() {