auto merge of #10519 : nikomatsakis/rust/issue-8624-borrowck-overly-permissive, r=pnkfelix

See #8624 for details.

r? @pnkfelix

src/libextra/dlist.rs

@@ -225,8 +225,9 @@ impl<T> Deque<T> for DList<T> {
     /// Provide a mutable reference to the back element, or None if the list is empty
     #[inline]
     fn back_mut<'a>(&'a mut self) -> Option<&'a mut T> {
-        let mut tmp = self.list_tail.resolve(); // FIXME: #3511: shouldn't need variable
+        let tmp: Option<&'a mut Node<T>> =
-        tmp.as_mut().map(|tail| &mut tail.value)
+            self.list_tail.resolve(); // FIXME: #3511: shouldn't need variable
+        tmp.map(|tail| &mut tail.value)
     }
 
     /// Add an element first in the list

src/libextra/ringbuf.rs

@@ -198,7 +198,28 @@ impl<T> RingBuf<T> {
 
     /// Front-to-back iterator which returns mutable values.
     pub fn mut_iter<'a>(&'a mut self) -> RingBufMutIterator<'a, T> {
-        RingBufMutIterator{index: 0, rindex: self.nelts, lo: self.lo, elts: self.elts}
+        let start_index = raw_index(self.lo, self.elts.len(), 0);
+        let end_index = raw_index(self.lo, self.elts.len(), self.nelts);
+
+        // Divide up the array
+        if end_index <= start_index {
+            // Items to iterate goes from:
+            //    start_index to self.elts.len()
+            // and then
+            //    0 to end_index
+            let (temp, remaining1) = self.elts.mut_split(start_index);
+            let (remaining2, _) = temp.mut_split(end_index);
+            RingBufMutIterator { remaining1: remaining1,
+                                 remaining2: remaining2,
+                                 nelts: self.nelts }
+        } else {
+            // Items to iterate goes from start_index to end_index:
+            let (empty, elts) = self.elts.mut_split(0);
+            let remaining1 = elts.mut_slice(start_index, end_index);
+            RingBufMutIterator { remaining1: remaining1,
+                                 remaining2: empty,
+                                 nelts: self.nelts }
+        }
     }
 
     /// Back-to-front iterator which returns mutable values.
@@ -207,45 +228,6 @@ impl<T> RingBuf<T> {
     }
 }
 
-macro_rules! iterator {
-    (impl $name:ident -> $elem:ty, $getter:ident) => {
-        impl<'self, T> Iterator<$elem> for $name<'self, T> {
-            #[inline]
-            fn next(&mut self) -> Option<$elem> {
-                if self.index == self.rindex {
-                    return None;
-                }
-                let raw_index = raw_index(self.lo, self.elts.len(), self.index);
-                self.index += 1;
-                Some(self.elts[raw_index] . $getter ())
-            }
-
-            #[inline]
-            fn size_hint(&self) -> (uint, Option<uint>) {
-                let len = self.rindex - self.index;
-                (len, Some(len))
-            }
-        }
-    }
-}
-
-macro_rules! iterator_rev {
-    (impl $name:ident -> $elem:ty, $getter:ident) => {
-        impl<'self, T> DoubleEndedIterator<$elem> for $name<'self, T> {
-            #[inline]
-            fn next_back(&mut self) -> Option<$elem> {
-                if self.index == self.rindex {
-                    return None;
-                }
-                self.rindex -= 1;
-                let raw_index = raw_index(self.lo, self.elts.len(), self.rindex);
-                Some(self.elts[raw_index] . $getter ())
-            }
-        }
-    }
-}
-
-
 /// RingBuf iterator
 pub struct RingBufIterator<'self, T> {
     priv lo: uint,
@@ -253,8 +235,36 @@ pub struct RingBufIterator<'self, T> {
     priv rindex: uint,
     priv elts: &'self [Option<T>],
 }
-iterator!{impl RingBufIterator -> &'self T, get_ref}
+
-iterator_rev!{impl RingBufIterator -> &'self T, get_ref}
+impl<'self, T> Iterator<&'self T> for RingBufIterator<'self, T> {
+    #[inline]
+    fn next(&mut self) -> Option<&'self T> {
+        if self.index == self.rindex {
+            return None;
+        }
+        let raw_index = raw_index(self.lo, self.elts.len(), self.index);
+        self.index += 1;
+        Some(self.elts[raw_index].get_ref())
+    }
+
+    #[inline]
+    fn size_hint(&self) -> (uint, Option<uint>) {
+        let len = self.rindex - self.index;
+        (len, Some(len))
+    }
+}
+
+impl<'self, T> DoubleEndedIterator<&'self T> for RingBufIterator<'self, T> {
+    #[inline]
+    fn next_back(&mut self) -> Option<&'self T> {
+        if self.index == self.rindex {
+            return None;
+        }
+        self.rindex -= 1;
+        let raw_index = raw_index(self.lo, self.elts.len(), self.rindex);
+        Some(self.elts[raw_index].get_ref())
+    }
+}
 
 impl<'self, T> ExactSize<&'self T> for RingBufIterator<'self, T> {}
 
@@ -275,13 +285,49 @@ impl<'self, T> RandomAccessIterator<&'self T> for RingBufIterator<'self, T> {
 
 /// RingBuf mutable iterator
 pub struct RingBufMutIterator<'self, T> {
-    priv lo: uint,
+    priv remaining1: &'self mut [Option<T>],
-    priv index: uint,
+    priv remaining2: &'self mut [Option<T>],
-    priv rindex: uint,
+    priv nelts: uint,
-    priv elts: &'self mut [Option<T>],
+}
+
+impl<'self, T> Iterator<&'self mut T> for RingBufMutIterator<'self, T> {
+    #[inline]
+    fn next(&mut self) -> Option<&'self mut T> {
+        if self.nelts == 0 {
+            return None;
+        }
+        let r = if self.remaining1.len() > 0 {
+            &mut self.remaining1
+        } else {
+            assert!(self.remaining2.len() > 0);
+            &mut self.remaining2
+        };
+        self.nelts -= 1;
+        Some(r.mut_shift_ref().get_mut_ref())
+    }
+
+    #[inline]
+    fn size_hint(&self) -> (uint, Option<uint>) {
+        (self.nelts, Some(self.nelts))
+    }
+}
+
+impl<'self, T> DoubleEndedIterator<&'self mut T> for RingBufMutIterator<'self, T> {
+    #[inline]
+    fn next_back(&mut self) -> Option<&'self mut T> {
+        if self.nelts == 0 {
+            return None;
+        }
+        let r = if self.remaining2.len() > 0 {
+            &mut self.remaining2
+        } else {
+            assert!(self.remaining1.len() > 0);
+            &mut self.remaining1
+        };
+        self.nelts -= 1;
+        Some(r.mut_pop_ref().get_mut_ref())
+    }
 }
-iterator!{impl RingBufMutIterator -> &'self mut T, get_mut_ref}
-iterator_rev!{impl RingBufMutIterator -> &'self mut T, get_mut_ref}
 
 impl<'self, T> ExactSize<&'self mut T> for RingBufMutIterator<'self, T> {}
 
@@ -667,6 +713,21 @@ mod tests {
         assert_eq!(d.rev_iter().collect::<~[&int]>(), ~[&4,&3,&2,&1,&0,&6,&7,&8]);
     }
 
+    #[test]
+    fn test_mut_rev_iter_wrap() {
+        let mut d = RingBuf::with_capacity(3);
+        assert!(d.mut_rev_iter().next().is_none());
+
+        d.push_back(1);
+        d.push_back(2);
+        d.push_back(3);
+        assert_eq!(d.pop_front(), Some(1));
+        d.push_back(4);
+
+        assert_eq!(d.mut_rev_iter().map(|x| *x).collect::<~[int]>(),
+                   ~[4, 3, 2]);
+    }
+
     #[test]
     fn test_mut_iter() {
         let mut d = RingBuf::new();

src/librustc/middle/borrowck/doc.rs

@@ -233,7 +233,7 @@ the lifetime of the value being borrowed. This pass is also
 responsible for inserting root annotations to keep managed values
 alive and for dynamically freezing `@mut` boxes.
 
-3. `RESTRICTIONS(LV, ACTIONS) = RS`: This pass checks and computes the
+3. `RESTRICTIONS(LV, LT, ACTIONS) = RS`: This pass checks and computes the
 restrictions to maintain memory safety. These are the restrictions
 that will go into the final loan. We'll discuss in more detail below.
 
@@ -451,7 +451,7 @@ the scope `LT`.
 
 The final rules govern the computation of *restrictions*, meaning that
 we compute the set of actions that will be illegal for the life of the
-loan. The predicate is written `RESTRICTIONS(LV, ACTIONS) =
+loan. The predicate is written `RESTRICTIONS(LV, LT, ACTIONS) =
 RESTRICTION*`, which can be read "in order to prevent `ACTIONS` from
 occuring on `LV`, the restrictions `RESTRICTION*` must be respected
 for the lifetime of the loan".
@@ -459,9 +459,9 @@ for the lifetime of the loan".
 Note that there is an initial set of restrictions: these restrictions
 are computed based on the kind of borrow:
 
-    &mut LV =>   RESTRICTIONS(LV, MUTATE|CLAIM|FREEZE)
+    &mut LV =>   RESTRICTIONS(LV, LT, MUTATE|CLAIM|FREEZE)
-    &LV =>       RESTRICTIONS(LV, MUTATE|CLAIM)
+    &LV =>       RESTRICTIONS(LV, LT, MUTATE|CLAIM)
-    &const LV => RESTRICTIONS(LV, [])
+    &const LV => RESTRICTIONS(LV, LT, [])
 
 The reasoning here is that a mutable borrow must be the only writer,
 therefore it prevents other writes (`MUTATE`), mutable borrows
@@ -474,7 +474,7 @@ moved out from under it, so no actions are forbidden.
 
 The simplest case is a borrow of a local variable `X`:
 
-    RESTRICTIONS(X, ACTIONS) = (X, ACTIONS)            // R-Variable
+    RESTRICTIONS(X, LT, ACTIONS) = (X, ACTIONS)            // R-Variable
 
 In such cases we just record the actions that are not permitted.
 
@@ -483,8 +483,8 @@ In such cases we just record the actions that are not permitted.
 Restricting a field is the same as restricting the owner of that
 field:
 
-    RESTRICTIONS(LV.f, ACTIONS) = RS, (LV.f, ACTIONS)  // R-Field
+    RESTRICTIONS(LV.f, LT, ACTIONS) = RS, (LV.f, ACTIONS)  // R-Field
-      RESTRICTIONS(LV, ACTIONS) = RS
+      RESTRICTIONS(LV, LT, ACTIONS) = RS
 
 The reasoning here is as follows. If the field must not be mutated,
 then you must not mutate the owner of the field either, since that
@@ -504,9 +504,9 @@ must prevent the owned pointer `LV` from being mutated, which means
 that we always add `MUTATE` and `CLAIM` to the restriction set imposed
 on `LV`:
 
-    RESTRICTIONS(*LV, ACTIONS) = RS, (*LV, ACTIONS)    // R-Deref-Send-Pointer
+    RESTRICTIONS(*LV, LT, ACTIONS) = RS, (*LV, ACTIONS)    // R-Deref-Send-Pointer
       TYPE(LV) = ~Ty
-      RESTRICTIONS(LV, ACTIONS|MUTATE|CLAIM) = RS
+      RESTRICTIONS(LV, LT, ACTIONS|MUTATE|CLAIM) = RS
 
 ### Restrictions for loans of immutable managed/borrowed pointees
 
@@ -519,7 +519,7 @@ restricting that path. Therefore, the rule for `&Ty` and `@Ty`
 pointers always returns an empty set of restrictions, and it only
 permits restricting `MUTATE` and `CLAIM` actions:
 
-    RESTRICTIONS(*LV, ACTIONS) = []                    // R-Deref-Imm-Borrowed
+    RESTRICTIONS(*LV, LT, ACTIONS) = []                    // R-Deref-Imm-Borrowed
       TYPE(LV) = &Ty or @Ty
       ACTIONS subset of [MUTATE, CLAIM]
 
@@ -546,7 +546,7 @@ Because moves from a `&const` or `@const` lvalue are never legal, it
 is not necessary to add any restrictions at all to the final
 result.
 
-    RESTRICTIONS(*LV, []) = []                         // R-Deref-Freeze-Borrowed
+    RESTRICTIONS(*LV, LT, []) = []                         // R-Deref-Freeze-Borrowed
       TYPE(LV) = &const Ty or @const Ty
 
 ### Restrictions for loans of mutable borrowed pointees
@@ -581,83 +581,149 @@ an `&mut` pointee from being mutated, claimed, or frozen, as occurs
 whenever the `&mut` pointee `*LV` is reborrowed as mutable or
 immutable:
 
-    RESTRICTIONS(*LV, ACTIONS) = RS, (*LV, ACTIONS)    // R-Deref-Mut-Borrowed-1
+    RESTRICTIONS(*LV, LT, ACTIONS) = RS, (*LV, ACTIONS)    // R-Deref-Mut-Borrowed-1
-      TYPE(LV) = &mut Ty
+      TYPE(LV) = &LT' mut Ty
-      RESTRICTIONS(LV, MUTATE|CLAIM|ALIAS) = RS
+      LT <= LT'                                            // (1)
+      RESTRICTIONS(LV, LT, MUTATE|CLAIM|ALIAS) = RS        // (2)
 
-The main interesting part of the rule is the final line, which
+There are two interesting parts to this rule:
-requires that the `&mut` *pointer* `LV` be restricted from being
-mutated, claimed, or aliased. The goal of these restrictions is to
-ensure that, not considering the pointer that will result from this
-borrow, `LV` remains the *sole pointer with mutable access* to `*LV`.
 
-Restrictions against mutations and claims are necessary because if the
+1. The lifetime of the loan (`LT`) cannot exceed the lifetime of the
-pointer in `LV` were to be somehow copied or moved to a different
+   `&mut` pointer (`LT'`). The reason for this is that the `&mut`
-location, then the restriction issued for `*LV` would not apply to the
+   pointer is guaranteed to be the only legal way to mutate its
-new location. Note that because `&mut` values are non-copyable, a
+   pointee -- but only for the lifetime `LT'`.  After that lifetime,
-simple attempt to move the base pointer will fail due to the
+   the loan on the pointee expires and hence the data may be modified
-(implicit) restriction against moves:
+   by its owner again. This implies that we are only able to guarantee that
+   the pointee will not be modified or aliased for a maximum of `LT'`.
 
-    // src/test/compile-fail/borrowck-move-mut-base-ptr.rs
+   Here is a concrete example of a bug this rule prevents:
-    fn foo(t0: &mut int) {
-        let p: &int = &*t0; // Freezes `*t0`
-        let t1 = t0;        //~ ERROR cannot move out of `t0`
-        *t1 = 22;
-    }
 
-However, the additional restrictions against mutation mean that even a
+       // Test region-reborrow-from-shorter-mut-ref.rs:
-clever attempt to use a swap to circumvent the type system will
+       fn copy_pointer<'a,'b,T>(x: &'a mut &'b mut T) -> &'b mut T {
-encounter an error:
+           &mut **p // ERROR due to clause (1)
+       }
+       fn main() {
+           let mut x = 1;
+           let mut y = &mut x; // <-'b-----------------------------+
+           //      +-'a--------------------+                       |
+           //      v                       v                       |
+           let z = copy_borrowed_ptr(&mut y); // y is lent         |
+           *y += 1; // Here y==z, so both should not be usable...  |
+           *z += 1; // ...and yet they would be, but for clause 1. |
+       } <---------------------------------------------------------+
 
-    // src/test/compile-fail/borrowck-swap-mut-base-ptr.rs
+2. The final line recursively requires that the `&mut` *pointer* `LV`
-    fn foo<'a>(mut t0: &'a mut int,
+   be restricted from being mutated, claimed, or aliased (not just the
-               mut t1: &'a mut int) {
+   pointee). The goal of these restrictions is to ensure that, not
-        let p: &int = &*t0;     // Freezes `*t0`
+   considering the pointer that will result from this borrow, `LV`
-        swap(&mut t0, &mut t1); //~ ERROR cannot borrow `t0`
+   remains the *sole pointer with mutable access* to `*LV`.
-        *t1 = 22;
-    }
 
-The restriction against *aliasing* (and, in turn, freezing) is
+   Restrictions against claims are necessary because if the pointer in
-necessary because, if an alias were of `LV` were to be produced, then
+   `LV` were to be somehow copied or moved to a different location,
-`LV` would no longer be the sole path to access the `&mut`
+   then the restriction issued for `*LV` would not apply to the new
-pointee. Since we are only issuing restrictions against `*LV`, these
+   location. Note that because `&mut` values are non-copyable, a
-other aliases would be unrestricted, and the result would be
+   simple attempt to move the base pointer will fail due to the
-unsound. For example:
+   (implicit) restriction against moves:
+
+       // src/test/compile-fail/borrowck-move-mut-base-ptr.rs
+       fn foo(t0: &mut int) {
+           let p: &int = &*t0; // Freezes `*t0`
+           let t1 = t0;        //~ ERROR cannot move out of `t0`
+           *t1 = 22;
+       }
+
+   However, the additional restrictions against claims mean that even
+   a clever attempt to use a swap to circumvent the type system will
+   encounter an error:
+
+       // src/test/compile-fail/borrowck-swap-mut-base-ptr.rs
+       fn foo<'a>(mut t0: &'a mut int,
+                  mut t1: &'a mut int) {
+           let p: &int = &*t0;     // Freezes `*t0`
+           swap(&mut t0, &mut t1); //~ ERROR cannot borrow `t0`
+           *t1 = 22;
+       }
+
+   The restriction against *aliasing* (and, in turn, freezing) is
+   necessary because, if an alias were of `LV` were to be produced,
+   then `LV` would no longer be the sole path to access the `&mut`
+   pointee. Since we are only issuing restrictions against `*LV`,
+   these other aliases would be unrestricted, and the result would be
+   unsound. For example:
 
     // src/test/compile-fail/borrowck-alias-mut-base-ptr.rs
     fn foo(t0: &mut int) {
         let p: &int = &*t0; // Freezes `*t0`
         let q: &const &mut int = &const t0; //~ ERROR cannot borrow `t0`
-        **q = 22; // (*)
+        **q = 22;
     }
 
-Note that the current rules also report an error at the assignment in
+The current rules could use some correction:
-`(*)`, because we only permit `&mut` poiners to be assigned if they
-are located in a non-aliasable location. However, I do not believe
-this restriction is strictly necessary. It was added, I believe, to
-discourage `&mut` from being placed in aliasable locations in the
-first place. One (desirable) side-effect of restricting aliasing on
-`LV` is that borrowing an `&mut` pointee found inside an aliasable
-pointee yields an error:
 
-    // src/test/compile-fail/borrowck-borrow-mut-base-ptr-in-aliasable-loc:
+1. Issue #10520. Now that the swap operator has been removed, I do not
-    fn foo(t0: & &mut int) {
+   believe the restriction against mutating `LV` is needed, and in
-        let t1 = t0;
+   fact it prevents some useful patterns. For example, the following
-        let p: &int = &**t0; //~ ERROR cannot borrow an `&mut` in a `&` pointer
+   function will fail to compile:
-        **t1 = 22; // (*)
-    }
 
-Here at the line `(*)` you will also see the error I referred to
+       fn mut_shift_ref<'a,T>(x: &mut &'a mut [T]) -> &'a mut T {
-above, which I do not believe is strictly necessary.
+           // `mut_split` will restrict mutation against *x:
+           let (head, tail) = (*x).mut_split(1);
+
+           // Hence mutating `*x` yields an error here:
+           *x = tail;
+           &mut head[0]
+       }
+
+   Note that this function -- which adjusts the slice `*x` in place so
+   that it no longer contains the head element and then returns a
+   pointer to that element separately -- is perfectly valid. It is
+   currently implemented using unsafe code. I believe that now that
+   the swap operator is removed from the language, we could liberalize
+   the rules and make this function be accepted normally. The idea
+   would be to have the assignment to `*x` kill the loans of `*x` and
+   its subpaths -- after all, those subpaths are no longer accessible
+   through `*x`, since it has been overwritten with a new value. Thus
+   those subpaths are only accessible through prior existing borrows
+   of `*x`, if any. The danger of the *swap* operator was that it
+   allowed `*x` to be mutated without making the subpaths of `*x`
+   inaccessible: worse, they became accessible through a new path (I
+   suppose that we could support swap, too, if needed, by moving the
+   loans over to the new path).
+
+   Note: the `swap()` function doesn't pose the same danger as the
+   swap operator because it requires taking `&mut` refs to invoke it.
+
+2. Issue #9629. The current rules correctly prohibit `&mut` pointees
+   from being assigned unless they are in a unique location. However,
+   we *also* prohibit `&mut` pointees from being frozen. This prevents
+   compositional patterns, like this one:
+
+       struct BorrowedMap<'a> {
+           map: &'a mut HashMap
+       }
+
+   If we have a pointer `x:&BorrowedMap`, we can't freeze `x.map`,
+   and hence can't call `find` etc on it. But that's silly, since
+   fact that the `&mut` exists in frozen data implies that it
+   will not be mutable by anyone. For example, this program nets an
+   error:
+
+       fn main() {
+           let a = &mut 2;
+           let b = &a;
+           *a += 1; // ERROR: cannot assign to `*a` because it is borrowed
+       }
+
+   (Naturally `&mut` reborrows from an `&&mut` pointee should be illegal.)
 
 The second rule for `&mut` handles the case where we are not adding
 any restrictions (beyond the default of "no move"):
 
-    RESTRICTIONS(*LV, []) = []                    // R-Deref-Mut-Borrowed-2
+    RESTRICTIONS(*LV, LT, []) = []                    // R-Deref-Mut-Borrowed-2
       TYPE(LV) = &mut Ty
 
 Moving from an `&mut` pointee is never legal, so no special
-restrictions are needed.
+restrictions are needed. This rule is used for `&const` borrows.
 
 ### Restrictions for loans of mutable managed pointees
 
@@ -665,7 +731,7 @@ With `@mut` pointees, we don't make any static guarantees.  But as a
 convenience, we still register a restriction against `*LV`, because
 that way if we *can* find a simple static error, we will:
 
-    RESTRICTIONS(*LV, ACTIONS) = [*LV, ACTIONS]   // R-Deref-Managed-Borrowed
+    RESTRICTIONS(*LV, LT, ACTIONS) = [*LV, ACTIONS]   // R-Deref-Managed-Borrowed
       TYPE(LV) = @mut Ty
 
 # Moves and initialization

src/librustc/middle/borrowck/gather_loans/lifetime.rs

@@ -8,9 +8,10 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-//! This module implements the check that the lifetime of a borrow
+/*!
-//! does not exceed the lifetime of the value being borrowed.
+ * This module implements the check that the lifetime of a borrow
-
+ * does not exceed the lifetime of the value being borrowed.
+ */
 
 use middle::borrowck::*;
 use mc = middle::mem_categorization;
@@ -20,13 +21,15 @@ use syntax::ast;
 use syntax::codemap::Span;
 use util::ppaux::{note_and_explain_region};
 
+type R = Result<(),()>;
+
 pub fn guarantee_lifetime(bccx: &BorrowckCtxt,
                           item_scope_id: ast::NodeId,
                           root_scope_id: ast::NodeId,
                           span: Span,
                           cmt: mc::cmt,
                           loan_region: ty::Region,
-                          loan_mutbl: LoanMutability) {
+                          loan_mutbl: LoanMutability) -> R {
     debug!("guarantee_lifetime(cmt={}, loan_region={})",
            cmt.repr(bccx.tcx), loan_region.repr(bccx.tcx));
     let ctxt = GuaranteeLifetimeContext {bccx: bccx,
@@ -36,7 +39,7 @@ pub fn guarantee_lifetime(bccx: &BorrowckCtxt,
                                          loan_mutbl: loan_mutbl,
                                          cmt_original: cmt,
                                          root_scope_id: root_scope_id};
-    ctxt.check(cmt, None);
+    ctxt.check(cmt, None)
 }
 
 ///////////////////////////////////////////////////////////////////////////
@@ -63,7 +66,7 @@ impl<'self> GuaranteeLifetimeContext<'self> {
         self.bccx.tcx
     }
 
-    fn check(&self, cmt: mc::cmt, discr_scope: Option<ast::NodeId>) {
+    fn check(&self, cmt: mc::cmt, discr_scope: Option<ast::NodeId>) -> R {
         //! Main routine. Walks down `cmt` until we find the "guarantor".
 
         match cmt.cat {
@@ -83,6 +86,7 @@ impl<'self> GuaranteeLifetimeContext<'self> {
             }
 
             mc::cat_static_item => {
+                Ok(())
             }
 
             mc::cat_deref(base, derefs, mc::gc_ptr(ptr_mutbl)) => {
@@ -99,10 +103,11 @@ impl<'self> GuaranteeLifetimeContext<'self> {
                 if !omit_root {
                     // L-Deref-Managed-Imm-Compiler-Root
                     // L-Deref-Managed-Mut-Compiler-Root
-                    self.check_root(cmt, base, derefs, ptr_mutbl, discr_scope);
+                    self.check_root(cmt, base, derefs, ptr_mutbl, discr_scope)
                 } else {
                     debug!("omitting root, base={}, base_scope={:?}",
                            base.repr(self.tcx()), base_scope);
+                    Ok(())
                 }
             }
 
@@ -120,7 +125,7 @@ impl<'self> GuaranteeLifetimeContext<'self> {
                 // for one arm.  Therefore, we insert a cat_discr(),
                 // basically a special kind of category that says "if this
                 // value must be dynamically rooted, root it for the scope
-                // `match_id`.
+                // `match_id`".
                 //
                 // As an example, consider this scenario:
                 //
@@ -188,7 +193,7 @@ impl<'self> GuaranteeLifetimeContext<'self> {
                   cmt_base: mc::cmt,
                   derefs: uint,
                   ptr_mutbl: ast::Mutability,
-                  discr_scope: Option<ast::NodeId>) {
+                  discr_scope: Option<ast::NodeId>) -> R {
         debug!("check_root(cmt_deref={}, cmt_base={}, derefs={:?}, ptr_mutbl={:?}, \
                 discr_scope={:?})",
                cmt_deref.repr(self.tcx()),
@@ -201,9 +206,8 @@ impl<'self> GuaranteeLifetimeContext<'self> {
         // that we can root the value, dynamically.
         let root_region = ty::ReScope(self.root_scope_id);
         if !self.bccx.is_subregion_of(self.loan_region, root_region) {
-            self.report_error(
+            return Err(self.report_error(
-                err_out_of_root_scope(root_region, self.loan_region));
+                err_out_of_root_scope(root_region, self.loan_region)));
-            return;
         }
 
         // Extract the scope id that indicates how long the rooting is required
@@ -278,13 +282,16 @@ impl<'self> GuaranteeLifetimeContext<'self> {
         self.bccx.root_map.insert(rm_key, root_info);
 
         debug!("root_key: {:?} root_info: {:?}", rm_key, root_info);
+        Ok(())
     }
 
-    fn check_scope(&self, max_scope: ty::Region) {
+    fn check_scope(&self, max_scope: ty::Region) -> R {
         //! Reports an error if `loan_region` is larger than `valid_scope`
 
         if !self.bccx.is_subregion_of(self.loan_region, max_scope) {
-            self.report_error(err_out_of_scope(max_scope, self.loan_region));
+            Err(self.report_error(err_out_of_scope(max_scope, self.loan_region)))
+        } else {
+            Ok(())
         }
     }
 

src/librustc/middle/borrowck/gather_loans/mod.rs

@@ -447,17 +447,22 @@ impl<'self> GatherLoanCtxt<'self> {
 
         // Check that the lifetime of the borrow does not exceed
         // the lifetime of the data being borrowed.
-        lifetime::guarantee_lifetime(self.bccx, self.item_ub, root_ub,
+        if lifetime::guarantee_lifetime(self.bccx, self.item_ub, root_ub,
-                                     borrow_span, cmt, loan_region, req_mutbl);
+                                        borrow_span, cmt, loan_region,
+                                        req_mutbl).is_err() {
+            return; // reported an error, no sense in reporting more.
+        }
 
         // Check that we don't allow mutable borrows of non-mutable data.
-        check_mutability(self.bccx, borrow_span, cmt, req_mutbl);
+        if check_mutability(self.bccx, borrow_span, cmt, req_mutbl).is_err() {
+            return; // reported an error, no sense in reporting more.
+        }
 
         // Compute the restrictions that are required to enforce the
         // loan is safe.
         let restr = restrictions::compute_restrictions(
             self.bccx, borrow_span,
-            cmt, self.restriction_set(req_mutbl));
+            cmt, loan_region, self.restriction_set(req_mutbl));
 
         // Create the loan record (if needed).
         let loan = match restr {
@@ -554,25 +559,29 @@ impl<'self> GatherLoanCtxt<'self> {
         fn check_mutability(bccx: &BorrowckCtxt,
                             borrow_span: Span,
                             cmt: mc::cmt,
-                            req_mutbl: LoanMutability) {
+                            req_mutbl: LoanMutability) -> Result<(),()> {
             //! Implements the M-* rules in doc.rs.
 
             match req_mutbl {
                 ConstMutability => {
                     // Data of any mutability can be lent as const.
+                    Ok(())
                 }
 
                 ImmutableMutability => {
                     // both imm and mut data can be lent as imm;
                     // for mutable data, this is a freeze
+                    Ok(())
                 }
 
                 MutableMutability => {
                     // Only mutable data can be lent as mutable.
                     if !cmt.mutbl.is_mutable() {
-                        bccx.report(BckError {span: borrow_span,
+                        Err(bccx.report(BckError {span: borrow_span,
-                                              cmt: cmt,
+                                                  cmt: cmt,
-                                              code: err_mutbl(req_mutbl)});
+                                                  code: err_mutbl(req_mutbl)}))
+                    } else {
+                        Ok(())
                     }
                 }
             }

src/librustc/middle/borrowck/gather_loans/restrictions.rs

@@ -8,8 +8,9 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-//! Computes the restrictions that result from a borrow.
+/*!
-
+ * Computes the restrictions that result from a borrow.
+ */
 
 use std::vec;
 use middle::borrowck::*;
@@ -26,11 +27,13 @@ pub enum RestrictionResult {
 pub fn compute_restrictions(bccx: &BorrowckCtxt,
                             span: Span,
                             cmt: mc::cmt,
+                            loan_region: ty::Region,
                             restr: RestrictionSet) -> RestrictionResult {
     let ctxt = RestrictionsContext {
         bccx: bccx,
         span: span,
-        cmt_original: cmt
+        cmt_original: cmt,
+        loan_region: loan_region,
     };
 
     ctxt.restrict(cmt, restr)
@@ -42,7 +45,8 @@ pub fn compute_restrictions(bccx: &BorrowckCtxt,
 struct RestrictionsContext<'self> {
     bccx: &'self BorrowckCtxt,
     span: Span,
-    cmt_original: mc::cmt
+    cmt_original: mc::cmt,
+    loan_region: ty::Region,
 }
 
 impl<'self> RestrictionsContext<'self> {
@@ -169,12 +173,22 @@ impl<'self> RestrictionsContext<'self> {
                 }
             }
 
-            mc::cat_deref(cmt_base, _, pk @ mc::region_ptr(MutMutable, _)) => {
+            mc::cat_deref(cmt_base, _, pk @ mc::region_ptr(MutMutable, lt)) => {
                 // Because an `&mut` pointer does not inherit its
                 // mutability, we can only prevent mutation or prevent
                 // freezing if it is not aliased. Therefore, in such
                 // cases we restrict aliasing on `cmt_base`.
                 if restrictions != RESTR_EMPTY {
+                    if !self.bccx.is_subregion_of(self.loan_region, lt) {
+                        self.bccx.report(
+                            BckError {
+                                span: self.span,
+                                cmt: cmt_base,
+                                code: err_mut_pointer_too_short(
+                                    self.loan_region, lt, restrictions)});
+                        return Safe;
+                    }
+
                     // R-Deref-Mut-Borrowed-1
                     let result = self.restrict(
                         cmt_base,

src/librustc/middle/borrowck/mod.rs

@@ -443,7 +443,8 @@ pub enum bckerr_code {
     err_mutbl(LoanMutability),
     err_out_of_root_scope(ty::Region, ty::Region), // superscope, subscope
     err_out_of_scope(ty::Region, ty::Region), // superscope, subscope
-    err_freeze_aliasable_const
+    err_freeze_aliasable_const,
+    err_mut_pointer_too_short(ty::Region, ty::Region, RestrictionSet), // loan, ptr
 }
 
 // Combination of an error code and the categorization of the expression
@@ -669,6 +670,22 @@ impl BorrowckCtxt {
                 // supposed to be going away.
                 format!("unsafe borrow of aliasable, const value")
             }
+            err_mut_pointer_too_short(_, _, r) => {
+                let descr = match opt_loan_path(err.cmt) {
+                    Some(lp) => format!("`{}`", self.loan_path_to_str(lp)),
+                    None => ~"`&mut` pointer"
+                };
+
+                let tag = if r.intersects(RESTR_ALIAS) {
+                    "its contents are unique"
+                } else {
+                    "its contents are not otherwise mutable"
+                };
+
+                format!("lifetime of {} is too short to guarantee {} \
+                        so they can be safely reborrowed",
+                        descr, tag)
+            }
         }
     }
 
@@ -742,7 +759,24 @@ impl BorrowckCtxt {
                     "...but borrowed value is only valid for ",
                     super_scope,
                     "");
-          }
+            }
+
+            err_mut_pointer_too_short(loan_scope, ptr_scope, _) => {
+                let descr = match opt_loan_path(err.cmt) {
+                    Some(lp) => format!("`{}`", self.loan_path_to_str(lp)),
+                    None => ~"`&mut` pointer"
+                };
+                note_and_explain_region(
+                    self.tcx,
+                    format!("{} would have to be valid for ", descr),
+                    loan_scope,
+                    "...");
+                note_and_explain_region(
+                    self.tcx,
+                    format!("...but {} is only valid for ", descr),
+                    ptr_scope,
+                    "");
+            }
         }
     }
 

src/libstd/vec.rs

@@ -975,6 +975,40 @@ pub trait ImmutableVector<'self, T> {
      * foreign interop.
      */
     fn as_imm_buf<U>(&self, f: |*T, uint| -> U) -> U;
+
+    /**
+     * Returns a mutable reference to the first element in this slice
+     * and adjusts the slice in place so that it no longer contains
+     * that element. O(1).
+     *
+     * Equivalent to:
+     *
+     * ```
+     *     let head = &self[0];
+     *     *self = self.slice_from(1);
+     *     head
+     * ```
+     *
+     * Fails if slice is empty.
+     */
+    fn shift_ref(&mut self) -> &'self T;
+
+    /**
+     * Returns a mutable reference to the last element in this slice
+     * and adjusts the slice in place so that it no longer contains
+     * that element. O(1).
+     *
+     * Equivalent to:
+     *
+     * ```
+     *     let tail = &self[self.len() - 1];
+     *     *self = self.slice_to(self.len() - 1);
+     *     tail
+     * ```
+     *
+     * Fails if slice is empty.
+     */
+    fn pop_ref(&mut self) -> &'self T;
 }
 
 impl<'self,T> ImmutableVector<'self, T> for &'self [T] {
@@ -1146,6 +1180,20 @@ impl<'self,T> ImmutableVector<'self, T> for &'self [T] {
         let s = self.repr();
         f(s.data, s.len)
     }
+
+    fn shift_ref(&mut self) -> &'self T {
+        unsafe {
+            let s: &mut Slice<T> = cast::transmute(self);
+            &*raw::shift_ptr(s)
+        }
+    }
+
+    fn pop_ref(&mut self) -> &'self T {
+        unsafe {
+            let s: &mut Slice<T> = cast::transmute(self);
+            &*raw::pop_ptr(s)
+        }
+    }
 }
 
 /// Extension methods for vectors contain `Eq` elements.
@@ -1864,23 +1912,61 @@ impl<T:Eq> OwnedEqVector<T> for ~[T] {
 pub trait MutableVector<'self, T> {
     /// Return a slice that points into another slice.
     fn mut_slice(self, start: uint, end: uint) -> &'self mut [T];
+
     /**
      * Returns a slice of self from `start` to the end of the vec.
      *
      * Fails when `start` points outside the bounds of self.
      */
     fn mut_slice_from(self, start: uint) -> &'self mut [T];
+
     /**
      * Returns a slice of self from the start of the vec to `end`.
      *
      * Fails when `end` points outside the bounds of self.
      */
     fn mut_slice_to(self, end: uint) -> &'self mut [T];
+
     /// Returns an iterator that allows modifying each value
     fn mut_iter(self) -> VecMutIterator<'self, T>;
+
     /// Returns a reversed iterator that allows modifying each value
     fn mut_rev_iter(self) -> MutRevIterator<'self, T>;
 
+    /**
+     * Returns a mutable reference to the first element in this slice
+     * and adjusts the slice in place so that it no longer contains
+     * that element. O(1).
+     *
+     * Equivalent to:
+     *
+     * ```
+     *     let head = &mut self[0];
+     *     *self = self.mut_slice_from(1);
+     *     head
+     * ```
+     *
+     * Fails if slice is empty.
+     */
+    fn mut_shift_ref(&mut self) -> &'self mut T;
+
+    /**
+     * Returns a mutable reference to the last element in this slice
+     * and adjusts the slice in place so that it no longer contains
+     * that element. O(1).
+     *
+     * Equivalent to:
+     *
+     * ```
+     *     let tail = &mut self[self.len() - 1];
+     *     *self = self.mut_slice_to(self.len() - 1);
+     *     tail
+     * ```
+     *
+     * Fails if slice is empty.
+     */
+    fn mut_pop_ref(&mut self) -> &'self mut T;
+
     /**
      * Swaps two elements in a vector
      *
@@ -1983,6 +2069,20 @@ impl<'self,T> MutableVector<'self, T> for &'self mut [T] {
         self.mut_iter().invert()
     }
 
+    fn mut_shift_ref(&mut self) -> &'self mut T {
+        unsafe {
+            let s: &mut Slice<T> = cast::transmute(self);
+            cast::transmute_mut(&*raw::shift_ptr(s))
+        }
+    }
+
+    fn mut_pop_ref(&mut self) -> &'self mut T {
+        unsafe {
+            let s: &mut Slice<T> = cast::transmute(self);
+            cast::transmute_mut(&*raw::pop_ptr(s))
+        }
+    }
+
     fn swap(self, a: uint, b: uint) {
         unsafe {
             // Can't take two mutable loans from one vector, so instead just cast
@@ -2194,6 +2294,31 @@ pub mod raw {
             })
         })
     }
+
+    /**
+     * Returns a pointer to first element in slice and adjusts
+     * slice so it no longer contains that element. Fails if
+     * slice is empty. O(1).
+     */
+    pub unsafe fn shift_ptr<T>(slice: &mut Slice<T>) -> *T {
+        if slice.len == 0 { fail!("shift on empty slice"); }
+        let head: *T = slice.data;
+        slice.data = ptr::offset(slice.data, 1);
+        slice.len -= 1;
+        head
+    }
+
+    /**
+     * Returns a pointer to last element in slice and adjusts
+     * slice so it no longer contains that element. Fails if
+     * slice is empty. O(1).
+     */
+    pub unsafe fn pop_ptr<T>(slice: &mut Slice<T>) -> *T {
+        if slice.len == 0 { fail!("pop on empty slice"); }
+        let tail: *T = ptr::offset(slice.data, (slice.len - 1) as int);
+        slice.len -= 1;
+        tail
+    }
 }
 
 /// Operations on `[u8]`
@@ -3827,6 +3952,75 @@ mod tests {
         assert!(!empty.ends_with(bytes!("foo")));
         assert!(bytes!("foobar").ends_with(empty));
     }
+
+    #[test]
+    fn test_shift_ref() {
+        let mut x: &[int] = [1, 2, 3, 4, 5];
+        let h = x.shift_ref();
+        assert_eq!(*h, 1);
+        assert_eq!(x.len(), 4);
+        assert_eq!(x[0], 2);
+        assert_eq!(x[3], 5);
+    }
+
+    #[test]
+    #[should_fail]
+    fn test_shift_ref_empty() {
+        let mut x: &[int] = [];
+        x.shift_ref();
+    }
+
+    #[test]
+    fn test_pop_ref() {
+        let mut x: &[int] = [1, 2, 3, 4, 5];
+        let h = x.pop_ref();
+        assert_eq!(*h, 5);
+        assert_eq!(x.len(), 4);
+        assert_eq!(x[0], 1);
+        assert_eq!(x[3], 4);
+    }
+
+    #[test]
+    #[should_fail]
+    fn test_pop_ref_empty() {
+        let mut x: &[int] = [];
+        x.pop_ref();
+    }
+
+
+    #[test]
+    fn test_mut_shift_ref() {
+        let mut x: &mut [int] = [1, 2, 3, 4, 5];
+        let h = x.mut_shift_ref();
+        assert_eq!(*h, 1);
+        assert_eq!(x.len(), 4);
+        assert_eq!(x[0], 2);
+        assert_eq!(x[3], 5);
+    }
+
+    #[test]
+    #[should_fail]
+    fn test_mut_shift_ref_empty() {
+        let mut x: &mut [int] = [];
+        x.mut_shift_ref();
+    }
+
+    #[test]
+    fn test_mut_pop_ref() {
+        let mut x: &mut [int] = [1, 2, 3, 4, 5];
+        let h = x.mut_pop_ref();
+        assert_eq!(*h, 5);
+        assert_eq!(x.len(), 4);
+        assert_eq!(x[0], 1);
+        assert_eq!(x[3], 4);
+    }
+
+    #[test]
+    #[should_fail]
+    fn test_mut_pop_ref_empty() {
+        let mut x: &mut [int] = [];
+        x.mut_pop_ref();
+    }
 }
 
 #[cfg(test)]

src/test/compile-fail/regions-reborrow-from-shorter-mut-ref-mut-ref.rs

@@ -0,0 +1,18 @@
+// Copyright 2012 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Issue #8624. Test for reborrowing with 3 levels, not just two.
+
+fn copy_borrowed_ptr<'a, 'b, 'c>(p: &'a mut &'b mut &'c mut int) -> &'b mut int {
+    &mut ***p //~ ERROR cannot infer an appropriate lifetime
+}
+
+fn main() {
+}

src/test/compile-fail/regions-reborrow-from-shorter-mut-ref.rs

@@ -0,0 +1,25 @@
+// Copyright 2012 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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Issue #8624. Tests that reborrowing the contents of an `&'b mut`
+// pointer which is backed by another `&'a mut` can only be done
+// for `'a` (which must be a sublifetime of `'b`).
+
+fn copy_borrowed_ptr<'a, 'b>(p: &'a mut &'b mut int) -> &'b mut int {
+    &mut **p //~ ERROR lifetime of `p` is too short
+}
+
+fn main() {
+    let mut x = 1;
+    let mut y = &mut x;
+    let z = copy_borrowed_ptr(&mut y);
+    *y += 1;
+    *z += 1;
+}