Check whether loans conflict with old loans or with themselves.

Along the way, convert from dvec-of-dvec representation to track loans in scope
to just a single flattened list.  It's more convenient.

Fixes #3765. r+ pcwalton.

5 files changed, 165 insertions, 94 deletions

diff --git a/src/rustc/middle/borrowck.rs b/src/rustc/middle/borrowck.rs
index e2f7ba20642..02fd2998f4d 100644
--- a/src/rustc/middle/borrowck.rs
+++ b/src/rustc/middle/borrowck.rs
@@ -383,7 +383,7 @@ impl bckerr : cmp::Eq {
 type bckres<T> = Result<T, bckerr>;
 
 /// a complete record of a loan that was granted
-type loan = {lp: @loan_path, cmt: cmt, mutbl: ast::mutability};
+struct Loan {lp: @loan_path, cmt: cmt, mutbl: ast::mutability}
 
 /// maps computed by `gather_loans` that are then used by `check_loans`
 ///
@@ -392,7 +392,7 @@ type loan = {lp: @loan_path, cmt: cmt, mutbl: ast::mutability};
 /// - `pure_map`: map from block/expr that must be pure to the error message
 ///   that should be reported if they are not pure
 type req_maps = {
-    req_loan_map: HashMap<ast::node_id, @DVec<@DVec<loan>>>,
+    req_loan_map: HashMap<ast::node_id, @DVec<Loan>>,
     pure_map: HashMap<ast::node_id, bckerr>
 };
 
@@ -582,6 +582,11 @@ impl borrowck_ctxt {
                                            method_map: self.method_map};
         mc.mut_to_str(mutbl)
     }
+
+    fn loan_to_repr(loan: &Loan) -> ~str {
+        fmt!("Loan(lp=%?, cmt=%s, mutbl=%?)",
+             loan.lp, self.cmt_to_repr(loan.cmt), loan.mutbl)
+    }
 }
 
 // The inherent mutability of a component is its default mutability
diff --git a/src/rustc/middle/borrowck/check_loans.rs b/src/rustc/middle/borrowck/check_loans.rs
index 6a9195b4509..7f95d44fd3b 100644
--- a/src/rustc/middle/borrowck/check_loans.rs
+++ b/src/rustc/middle/borrowck/check_loans.rs
@@ -131,18 +131,15 @@ impl check_loan_ctxt {
         }
     }
 
-    fn walk_loans(scope_id: ast::node_id,
-                  f: fn(v: &loan) -> bool) {
+    fn walk_loans(scope_id: ast::node_id, f: fn(v: &Loan) -> bool) {
         let mut scope_id = scope_id;
         let region_map = self.tcx().region_map;
         let req_loan_map = self.req_maps.req_loan_map;
 
         loop {
-            for req_loan_map.find(scope_id).each |loanss| {
-                for loanss.each |loans| {
-                    for loans.each |loan| {
-                        if !f(loan) { return; }
-                    }
+            for req_loan_map.find(scope_id).each |loans| {
+                for loans.each |loan| {
+                    if !f(loan) { return; }
                 }
             }
 
@@ -155,7 +152,7 @@ impl check_loan_ctxt {
 
     fn walk_loans_of(scope_id: ast::node_id,
                      lp: @loan_path,
-                     f: fn(v: &loan) -> bool) {
+                     f: fn(v: &Loan) -> bool) {
         for self.walk_loans(scope_id) |loan| {
             if loan.lp == lp {
                 if !f(loan) { return; }
@@ -256,36 +253,58 @@ impl check_loan_ctxt {
     }
 
     fn check_for_conflicting_loans(scope_id: ast::node_id) {
-        let new_loanss = match self.req_maps.req_loan_map.find(scope_id) {
+        debug!("check_for_conflicting_loans(scope_id=%?)", scope_id);
+
+        let new_loans = match self.req_maps.req_loan_map.find(scope_id) {
             None => return,
-            Some(loanss) => loanss
+            Some(loans) => loans
         };
 
+        debug!("new_loans has length %?", new_loans.len());
+
         let par_scope_id = self.tcx().region_map.get(scope_id);
         for self.walk_loans(par_scope_id) |old_loan| {
-            for new_loanss.each |new_loans| {
-                for new_loans.each |new_loan| {
-                    if old_loan.lp != new_loan.lp { loop; }
-                    match (old_loan.mutbl, new_loan.mutbl) {
-                      (m_const, _) | (_, m_const) |
-                      (m_mutbl, m_mutbl) | (m_imm, m_imm) => {
-                        /*ok*/
-                      }
-
-                      (m_mutbl, m_imm) | (m_imm, m_mutbl) => {
-                        self.bccx.span_err(
-                            new_loan.cmt.span,
-                            fmt!("loan of %s as %s \
-                                  conflicts with prior loan",
-                                 self.bccx.cmt_to_str(new_loan.cmt),
-                                 self.bccx.mut_to_str(new_loan.mutbl)));
-                        self.bccx.span_note(
-                            old_loan.cmt.span,
-                            fmt!("prior loan as %s granted here",
-                                 self.bccx.mut_to_str(old_loan.mutbl)));
-                      }
-                    }
-                }
+            debug!("old_loan=%?", self.bccx.loan_to_repr(old_loan));
+
+            for new_loans.each |new_loan| {
+                self.report_error_if_loans_conflict(old_loan, new_loan);
+            }
+        }
+
+        let len = new_loans.len();
+        for uint::range(0, len) |i| {
+            let loan_i = new_loans[i];
+            for uint::range(i+1, len) |j| {
+                let loan_j = new_loans[j];
+                self.report_error_if_loans_conflict(&loan_i, &loan_j);
+            }
+        }
+    }
+
+    fn report_error_if_loans_conflict(&self,
+                                      old_loan: &Loan,
+                                      new_loan: &Loan) {
+        if old_loan.lp != new_loan.lp {
+            return;
+        }
+
+        match (old_loan.mutbl, new_loan.mutbl) {
+            (m_const, _) | (_, m_const) |
+            (m_mutbl, m_mutbl) | (m_imm, m_imm) => {
+                /*ok*/
+            }
+
+            (m_mutbl, m_imm) | (m_imm, m_mutbl) => {
+                self.bccx.span_err(
+                    new_loan.cmt.span,
+                    fmt!("loan of %s as %s \
+                          conflicts with prior loan",
+                         self.bccx.cmt_to_str(new_loan.cmt),
+                         self.bccx.mut_to_str(new_loan.mutbl)));
+                self.bccx.span_note(
+                    old_loan.cmt.span,
+                    fmt!("prior loan as %s granted here",
+                         self.bccx.mut_to_str(old_loan.mutbl)));
             }
         }
     }
diff --git a/src/rustc/middle/borrowck/gather_loans.rs b/src/rustc/middle/borrowck/gather_loans.rs
index a2c8f185071..e8d11fd1708 100644
--- a/src/rustc/middle/borrowck/gather_loans.rs
+++ b/src/rustc/middle/borrowck/gather_loans.rs
@@ -213,9 +213,10 @@ fn req_loans_in_expr(ex: @ast::expr,
 }
 
 impl gather_loan_ctxt {
-    fn tcx() -> ty::ctxt { self.bccx.tcx }
+    fn tcx(&self) -> ty::ctxt { self.bccx.tcx }
 
-    fn guarantee_adjustments(expr: @ast::expr,
+    fn guarantee_adjustments(&self,
+                             expr: @ast::expr,
                              adjustment: &ty::AutoAdjustment) {
         debug!("guarantee_adjustments(expr=%s, adjustment=%?)",
                expr_repr(self.tcx(), expr), adjustment);
@@ -256,7 +257,8 @@ impl gather_loan_ctxt {
     // out loans, which will be added to the `req_loan_map`.  This can
     // also entail "rooting" GC'd pointers, which means ensuring
     // dynamically that they are not freed.
-    fn guarantee_valid(cmt: cmt,
+    fn guarantee_valid(&self,
+                       cmt: cmt,
                        req_mutbl: ast::mutability,
                        scope_r: ty::region) {
 
@@ -280,35 +282,12 @@ impl gather_loan_ctxt {
           // it within that scope, the loan will be detected and an
           // error will be reported.
           Some(_) => {
-            match self.bccx.loan(cmt, scope_r, req_mutbl) {
-              Err(e) => { self.bccx.report(e); }
-              Ok(loans) if loans.len() == 0 => {}
-              Ok(loans) => {
-                match scope_r {
-                  ty::re_scope(scope_id) => {
-                    self.add_loans(scope_id, loans);
-
-                    if req_mutbl == m_imm && cmt.mutbl != m_imm {
-                        self.bccx.loaned_paths_imm += 1;
-
-                        if self.tcx().sess.borrowck_note_loan() {
-                            self.bccx.span_note(
-                                cmt.span,
-                                fmt!("immutable loan required"));
-                        }
-                    } else {
-                        self.bccx.loaned_paths_same += 1;
-                    }
+              match self.bccx.loan(cmt, scope_r, req_mutbl) {
+                  Err(e) => { self.bccx.report(e); }
+                  Ok(move loans) => {
+                      self.add_loans(cmt, req_mutbl, scope_r, move loans);
                   }
-                  _ => {
-                    self.bccx.tcx.sess.span_bug(
-                        cmt.span,
-                        fmt!("loans required but scope is scope_region is %s",
-                             region_to_str(self.tcx(), scope_r)));
-                  }
-                }
               }
-            }
           }
 
           // The path is not loanable: in that case, we must try and
@@ -385,7 +364,8 @@ impl gather_loan_ctxt {
     // has type `@mut{f:int}`, this check might fail because `&x.f`
     // reqires an immutable pointer, but `f` lives in (aliased)
     // mutable memory.
-    fn check_mutbl(req_mutbl: ast::mutability,
+    fn check_mutbl(&self,
+                   req_mutbl: ast::mutability,
                    cmt: cmt) -> bckres<preserve_condition> {
         debug!("check_mutbl(req_mutbl=%?, cmt.mutbl=%?)",
                req_mutbl, cmt.mutbl);
@@ -407,21 +387,58 @@ impl gather_loan_ctxt {
         }
     }
 
-    fn add_loans(scope_id: ast::node_id, loans: @DVec<loan>) {
+    fn add_loans(&self,
+                 cmt: cmt,
+                 req_mutbl: ast::mutability,
+                 scope_r: ty::region,
+                 +loans: ~[Loan]) {
+        if loans.len() == 0 {
+            return;
+        }
+
+        let scope_id = match scope_r {
+            ty::re_scope(scope_id) => scope_id,
+            _ => {
+                self.bccx.tcx.sess.span_bug(
+                    cmt.span,
+                    fmt!("loans required but scope is scope_region is %s",
+                         region_to_str(self.tcx(), scope_r)));
+            }
+        };
+
+        self.add_loans_to_scope_id(scope_id, move loans);
+
+        if req_mutbl == m_imm && cmt.mutbl != m_imm {
+            self.bccx.loaned_paths_imm += 1;
+
+            if self.tcx().sess.borrowck_note_loan() {
+                self.bccx.span_note(
+                    cmt.span,
+                    fmt!("immutable loan required"));
+            }
+        } else {
+            self.bccx.loaned_paths_same += 1;
+        }
+    }
+
+    fn add_loans_to_scope_id(&self, scope_id: ast::node_id, +loans: ~[Loan]) {
         debug!("adding %u loans to scope_id %?", loans.len(), scope_id);
         match self.req_maps.req_loan_map.find(scope_id) {
-            Some(l) => {
-                l.push(loans);
+            Some(req_loans) => {
+                req_loans.push_all(loans);
             }
             None => {
-                self.req_maps.req_loan_map.insert(
-                    scope_id, @dvec::from_vec(~[loans]));
+                let dvec = @dvec::from_vec(move loans);
+                self.req_maps.req_loan_map.insert(scope_id, dvec);
             }
         }
     }
 
-    fn gather_pat(discr_cmt: cmt, root_pat: @ast::pat,
-                  arm_id: ast::node_id, alt_id: ast::node_id) {
+    fn gather_pat(&self,
+                  discr_cmt: cmt,
+                  root_pat: @ast::pat,
+                  arm_id: ast::node_id,
+                  alt_id: ast::node_id) {
         do self.bccx.cat_pattern(discr_cmt, root_pat) |cmt, pat| {
             match pat.node {
               ast::pat_ident(bm, _, _) if !self.pat_is_variant(pat) => {
@@ -475,7 +492,7 @@ impl gather_loan_ctxt {
         }
     }
 
-    fn pat_is_variant(pat: @ast::pat) -> bool {
+    fn pat_is_variant(&self, pat: @ast::pat) -> bool {
         pat_util::pat_is_variant(self.bccx.tcx.def_map, pat)
     }
 }
diff --git a/src/rustc/middle/borrowck/loan.rs b/src/rustc/middle/borrowck/loan.rs
index 8d9d7a5796a..5d3ccc39213 100644
--- a/src/rustc/middle/borrowck/loan.rs
+++ b/src/rustc/middle/borrowck/loan.rs
@@ -8,35 +8,37 @@ use result::{Result, Ok, Err};
 impl borrowck_ctxt {
     fn loan(cmt: cmt,
             scope_region: ty::region,
-            mutbl: ast::mutability) -> bckres<@DVec<loan>> {
-        let lc = loan_ctxt_(@{bccx: self,
-                              scope_region: scope_region,
-                              loans: @DVec()});
+            mutbl: ast::mutability) -> bckres<~[Loan]> {
+        let lc = LoanContext {
+            bccx: self,
+            scope_region: scope_region,
+            loans: ~[]
+        };
         match lc.loan(cmt, mutbl) {
-          Ok(()) => {Ok(lc.loans)}
-          Err(e) => {Err(e)}
+          Err(e) => Err(e),
+          Ok(()) => {
+              let LoanContext {loans, _} = move lc;
+              Ok(loans)
+          }
         }
     }
 }
 
-type loan_ctxt_ = {
+struct LoanContext {
     bccx: borrowck_ctxt,
 
     // the region scope for which we must preserve the memory
     scope_region: ty::region,
 
     // accumulated list of loans that will be required
-    loans: @DVec<loan>
-};
-
-enum loan_ctxt {
-    loan_ctxt_(@loan_ctxt_)
+    mut loans: ~[Loan]
 }
 
-impl loan_ctxt {
-    fn tcx() -> ty::ctxt { self.bccx.tcx }
+impl LoanContext {
+    fn tcx(&self) -> ty::ctxt { self.bccx.tcx }
 
-    fn issue_loan(cmt: cmt,
+    fn issue_loan(&self,
+                  cmt: cmt,
                   scope_ub: ty::region,
                   req_mutbl: ast::mutability) -> bckres<()> {
         if self.bccx.is_subregion_of(self.scope_region, scope_ub) {
@@ -57,12 +59,13 @@ impl loan_ctxt {
                 }
             }
 
-            (*self.loans).push({
+            self.loans.push(Loan {
                 // Note: cmt.lp must be Some(_) because otherwise this
                 // loan process does not apply at all.
                 lp: cmt.lp.get(),
                 cmt: cmt,
-                mutbl: req_mutbl});
+                mutbl: req_mutbl
+            });
             return Ok(());
         } else {
             // The loan being requested lives longer than the data
@@ -73,7 +76,7 @@ impl loan_ctxt {
         }
     }
 
-    fn loan(cmt: cmt, req_mutbl: ast::mutability) -> bckres<()> {
+    fn loan(&self, cmt: cmt, req_mutbl: ast::mutability) -> bckres<()> {
         debug!("loan(%s, %s)",
                self.bccx.cmt_to_repr(cmt),
                self.bccx.mut_to_str(req_mutbl));
@@ -144,7 +147,8 @@ impl loan_ctxt {
     // A "stable component" is one where assigning the base of the
     // component cannot cause the component itself to change types.
     // Example: record fields.
-    fn loan_stable_comp(cmt: cmt,
+    fn loan_stable_comp(&self,
+                        cmt: cmt,
                         cmt_base: cmt,
                         req_mutbl: ast::mutability) -> bckres<()> {
         let base_mutbl = match req_mutbl {
@@ -162,7 +166,8 @@ impl loan_ctxt {
     // An "unstable deref" means a deref of a ptr/comp where, if the
     // base of the deref is assigned to, pointers into the result of the
     // deref would be invalidated. Examples: interior of variants, uniques.
-    fn loan_unstable_deref(cmt: cmt,
+    fn loan_unstable_deref(&self,
+                           cmt: cmt,
                            cmt_base: cmt,
                            req_mutbl: ast::mutability) -> bckres<()> {
         // Variant components: the base must be immutable, because
diff --git a/src/test/compile-fail/borrowck-loan-local-as-both-mut-and-imm.rs b/src/test/compile-fail/borrowck-loan-local-as-both-mut-and-imm.rs
new file mode 100644
index 00000000000..54048ed2fd8
--- /dev/null
+++ b/src/test/compile-fail/borrowck-loan-local-as-both-mut-and-imm.rs
@@ -0,0 +1,25 @@
+use core::either::{Either, Left, Right};
+
+    fn f(x: &mut Either<int,float>, y: &Either<int,float>) -> int {
+        match *y {
+            Left(ref z) => {
+                *x = Right(1.0);
+                *z
+            }
+            _ => fail
+        }
+    }
+
+    fn g() {
+        let mut x: Either<int,float> = Left(3);
+        io::println(f(&mut x, &x).to_str()); //~ ERROR conflicts with prior loan
+    }
+
+    fn h() {
+        let mut x: Either<int,float> = Left(3);
+        let y: &Either<int, float> = &x;
+        let z: &mut Either<int, float> = &mut x; //~ ERROR conflicts with prior loan
+        *z = *y;
+    } 
+
+    fn main() {}