auto merge of #14947 : zwarich/rust/check-loans-not-restrictions, r=nikomatsakis

Now that features like `const` are gone, we can remove the concept of restrictions from borrowck and just track loans and their restricted paths.

Author: bors

src/librustc/middle/borrowck/check_loans.rs

@@ -155,6 +155,12 @@ enum UseError {
     UseWhileBorrowed(/*loan*/Rc<LoanPath>, /*loan*/Span)
 }
 
+fn compatible_borrow_kinds(borrow_kind1: ty::BorrowKind,
+                           borrow_kind2: ty::BorrowKind)
+                           -> bool {
+    borrow_kind1 == ty::ImmBorrow && borrow_kind2 == ty::ImmBorrow
+}
+
 impl<'a> CheckLoanCtxt<'a> {
     pub fn tcx(&self) -> &'a ty::ctxt { self.bccx.tcx }
 
@@ -189,29 +195,75 @@ impl<'a> CheckLoanCtxt<'a> {
         })
     }
 
-    pub fn each_in_scope_restriction(&self,
-                                     scope_id: ast::NodeId,
-                                     loan_path: &LoanPath,
-                                     op: |&Loan, &Restriction| -> bool)
-                                     -> bool {
-        //! Iterates through all the in-scope restrictions for the
-        //! given `loan_path`
+    fn each_in_scope_loan_affecting_path(&self,
+                                         scope_id: ast::NodeId,
+                                         loan_path: &LoanPath,
+                                         op: |&Loan| -> bool)
+                                         -> bool {
+        //! Iterates through all of the in-scope loans affecting `loan_path`,
+        //! calling `op`, and ceasing iteration if `false` is returned.
 
-        self.each_in_scope_loan(scope_id, |loan| {
-            debug!("each_in_scope_restriction found loan: {:?}",
-                   loan.repr(self.tcx()));
+        // First, we check for a loan restricting the path P being used. This
+        // accounts for borrows of P but also borrows of subpaths, like P.a.b.
+        // Consider the following example:
+        //
+        //     let x = &mut a.b.c; // Restricts a, a.b, and a.b.c
+        //     let y = a;          // Conflicts with restriction
 
+        let cont = self.each_in_scope_loan(scope_id, |loan| {
             let mut ret = true;
-            for restr in loan.restrictions.iter() {
-                if *restr.loan_path == *loan_path {
-                    if !op(loan, restr) {
+            for restr_path in loan.restricted_paths.iter() {
+                if **restr_path == *loan_path {
+                    if !op(loan) {
                         ret = false;
                         break;
                     }
                 }
             }
             ret
-        })
+        });
+
+        if !cont {
+            return false;
+        }
+
+        // Next, we must check for *loans* (not restrictions) on the path P or
+        // any base path. This rejects examples like the following:
+        //
+        //     let x = &mut a.b;
+        //     let y = a.b.c;
+        //
+        // Limiting this search to *loans* and not *restrictions* means that
+        // examples like the following continue to work:
+        //
+        //     let x = &mut a.b;
+        //     let y = a.c;
+
+        let mut loan_path = loan_path;
+        loop {
+            match *loan_path {
+                LpVar(_) => {
+                    break;
+                }
+                LpExtend(ref lp_base, _, _) => {
+                    loan_path = &**lp_base;
+                }
+            }
+
+            let cont = self.each_in_scope_loan(scope_id, |loan| {
+                if *loan.loan_path == *loan_path {
+                    op(loan)
+                } else {
+                    true
+                }
+            });
+
+            if !cont {
+                return false;
+            }
+        }
+
+        return true;
     }
 
     pub fn loans_generated_by(&self, scope_id: ast::NodeId) -> Vec<uint> {
@@ -288,26 +340,12 @@ impl<'a> CheckLoanCtxt<'a> {
                loan1.repr(self.tcx()),
                loan2.repr(self.tcx()));
 
-        // Restrictions that would cause the new loan to be illegal:
-        let illegal_if = match loan2.kind {
-            // Look for restrictions against mutation. These are
-            // generated by all other borrows.
-            ty::MutBorrow => RESTR_MUTATE,
-
-            // Look for restrictions against freezing (immutable borrows).
-            // These are generated by `&mut` borrows.
-            ty::ImmBorrow => RESTR_FREEZE,
-
-            // No matter how the data is borrowed (as `&`, as `&mut`,
-            // or as `&unique imm`) it will always generate a
-            // restriction against mutating the data. So look for those.
-            ty::UniqueImmBorrow => RESTR_MUTATE,
-        };
-        debug!("illegal_if={:?}", illegal_if);
+        if compatible_borrow_kinds(loan1.kind, loan2.kind) {
+            return true;
+        }
 
-        for restr in loan1.restrictions.iter() {
-            if !restr.set.intersects(illegal_if) { continue; }
-            if restr.loan_path != loan2.loan_path { continue; }
+        for restr_path in loan1.restricted_paths.iter() {
+            if *restr_path != loan2.loan_path { continue; }
 
             let old_pronoun = if new_loan.loan_path == old_loan.loan_path {
                 "it".to_string()
@@ -534,63 +572,16 @@ impl<'a> CheckLoanCtxt<'a> {
 
         let mut ret = UseOk;
 
-        // First, we check for a restriction on the path P being used. This
-        // accounts for borrows of P but also borrows of subpaths, like P.a.b.
-        // Consider the following example:
-        //
-        //     let x = &mut a.b.c; // Restricts a, a.b, and a.b.c
-        //     let y = a;          // Conflicts with restriction
-
-        self.each_in_scope_restriction(expr_id, use_path, |loan, _restr| {
-            if incompatible(loan.kind, borrow_kind) {
+        self.each_in_scope_loan_affecting_path(expr_id, use_path, |loan| {
+            if !compatible_borrow_kinds(loan.kind, borrow_kind) {
                 ret = UseWhileBorrowed(loan.loan_path.clone(), loan.span);
                 false
             } else {
                 true
             }
         });
 
-        // Next, we must check for *loans* (not restrictions) on the path P or
-        // any base path. This rejects examples like the following:
-        //
-        //     let x = &mut a.b;
-        //     let y = a.b.c;
-        //
-        // Limiting this search to *loans* and not *restrictions* means that
-        // examples like the following continue to work:
-        //
-        //     let x = &mut a.b;
-        //     let y = a.c;
-
-        let mut loan_path = use_path;
-        loop {
-            self.each_in_scope_loan(expr_id, |loan| {
-                if *loan.loan_path == *loan_path &&
-                   incompatible(loan.kind, borrow_kind) {
-                    ret = UseWhileBorrowed(loan.loan_path.clone(), loan.span);
-                    false
-                } else {
-                    true
-                }
-            });
-
-            match *loan_path {
-                LpVar(_) => {
-                    break;
-                }
-                LpExtend(ref lp_base, _, _) => {
-                    loan_path = &**lp_base;
-                }
-            }
-        }
-
         return ret;
-
-        fn incompatible(borrow_kind1: ty::BorrowKind,
-                        borrow_kind2: ty::BorrowKind)
-                        -> bool {
-            borrow_kind1 != ty::ImmBorrow || borrow_kind2 != ty::ImmBorrow
-        }
     }
 
     fn check_if_path_is_moved(&self,
@@ -668,11 +659,9 @@ impl<'a> CheckLoanCtxt<'a> {
         // and aliasing restrictions:
         if assignee_cmt.mutbl.is_mutable() {
             if check_for_aliasable_mutable_writes(self, assignment_span, assignee_cmt.clone()) {
-                if mode != euv::Init &&
-                    check_for_assignment_to_restricted_or_frozen_location(
-                        self, assignment_id, assignment_span, assignee_cmt.clone())
-                {
-                    // Safe, but record for lint pass later:
+                if mode != euv::Init {
+                    check_for_assignment_to_borrowed_path(
+                        self, assignment_id, assignment_span, assignee_cmt.clone());
                     mark_variable_as_used_mut(self, assignee_cmt);
                 }
             }
@@ -807,138 +796,24 @@ impl<'a> CheckLoanCtxt<'a> {
             }
         }
 
-        fn check_for_assignment_to_restricted_or_frozen_location(
+        fn check_for_assignment_to_borrowed_path(
             this: &CheckLoanCtxt,
             assignment_id: ast::NodeId,
             assignment_span: Span,
-            assignee_cmt: mc::cmt) -> bool
+            assignee_cmt: mc::cmt)
         {
             //! Check for assignments that violate the terms of an
             //! outstanding loan.
 
             let loan_path = match opt_loan_path(&assignee_cmt) {
                 Some(lp) => lp,
-                None => { return true; /* no loan path, can't be any loans */ }
+                None => { return; /* no loan path, can't be any loans */ }
             };
 
-            // Start by searching for an assignment to a *restricted*
-            // location. Here is one example of the kind of error caught
-            // by this check:
-            //
-            //    let mut v = ~[1, 2, 3];
-            //    let p = &v;
-            //    v = ~[4];
-            //
-            // In this case, creating `p` triggers a RESTR_MUTATE
-            // restriction on the path `v`.
-            //
-            // Here is a second, more subtle example:
-            //
-            //    let mut v = ~[1, 2, 3];
-            //    let p = &const v[0];
-            //    v[0] = 4;                   // OK
-            //    v[1] = 5;                   // OK
-            //    v = ~[4, 5, 3];             // Error
-            //
-            // In this case, `p` is pointing to `v[0]`, and it is a
-            // `const` pointer in any case. So the first two
-            // assignments are legal (and would be permitted by this
-            // check). However, the final assignment (which is
-            // logically equivalent) is forbidden, because it would
-            // cause the existing `v` array to be freed, thus
-            // invalidating `p`. In the code, this error results
-            // because `gather_loans::restrictions` adds a
-            // `RESTR_MUTATE` restriction whenever the contents of an
-            // owned pointer are borrowed, and hence while `v[*]` is not
-            // restricted from being written, `v` is.
-            let cont = this.each_in_scope_restriction(assignment_id,
-                                                      &*loan_path,
-                                                      |loan, restr| {
-                if restr.set.intersects(RESTR_MUTATE) {
-                    this.report_illegal_mutation(assignment_span, &*loan_path, loan);
-                    false
-                } else {
-                    true
-                }
+            this.each_in_scope_loan_affecting_path(assignment_id, &*loan_path, |loan| {
+                this.report_illegal_mutation(assignment_span, &*loan_path, loan);
+                false
             });
-
-            if !cont { return false }
-
-            // The previous code handled assignments to paths that
-            // have been restricted. This covers paths that have been
-            // directly lent out and their base paths, but does not
-            // cover random extensions of those paths. For example,
-            // the following program is not declared illegal by the
-            // previous check:
-            //
-            //    let mut v = ~[1, 2, 3];
-            //    let p = &v;
-            //    v[0] = 4; // declared error by loop below, not code above
-            //
-            // The reason that this passes the previous check whereas
-            // an assignment like `v = ~[4]` fails is because the assignment
-            // here is to `v[*]`, and the existing restrictions were issued
-            // for `v`, not `v[*]`.
-            //
-            // So in this loop, we walk back up the loan path so long
-            // as the mutability of the path is dependent on a super
-            // path, and check that the super path was not lent out as
-            // mutable or immutable (a const loan is ok).
-            //
-            // Mutability of a path can be dependent on the super path
-            // in two ways. First, it might be inherited mutability.
-            // Second, the pointee of an `&mut` pointer can only be
-            // mutated if it is found in an unaliased location, so we
-            // have to check that the owner location is not borrowed.
-            //
-            // Note that we are *not* checking for any and all
-            // restrictions.  We are only interested in the pointers
-            // that the user created, whereas we add restrictions for
-            // all kinds of paths that are not directly aliased. If we checked
-            // for all restrictions, and not just loans, then the following
-            // valid program would be considered illegal:
-            //
-            //    let mut v = ~[1, 2, 3];
-            //    let p = &const v[0];
-            //    v[1] = 5; // ok
-            //
-            // Here the restriction that `v` not be mutated would be misapplied
-            // to block the subpath `v[1]`.
-            let full_loan_path = loan_path.clone();
-            let mut loan_path = loan_path;
-            loop {
-                loan_path = match *loan_path {
-                    // Peel back one layer if, for `loan_path` to be
-                    // mutable, `lp_base` must be mutable. This occurs
-                    // with inherited mutability, owned pointers and
-                    // `&mut` pointers.
-                    LpExtend(ref lp_base, mc::McInherited, _) |
-                    LpExtend(ref lp_base, _, LpDeref(mc::OwnedPtr)) |
-                    LpExtend(ref lp_base, _, LpDeref(mc::GcPtr)) |
-                    LpExtend(ref lp_base, _, LpDeref(mc::BorrowedPtr(ty::MutBorrow, _))) => {
-                        lp_base.clone()
-                    }
-
-                    // Otherwise stop iterating
-                    LpExtend(_, mc::McDeclared, _) |
-                    LpExtend(_, mc::McImmutable, _) |
-                    LpVar(_) => {
-                        return true;
-                    }
-                };
-
-                // Check for a non-const loan of `loan_path`
-                let cont = this.each_in_scope_loan(assignment_id, |loan| {
-                    if loan.loan_path == loan_path {
-                        this.report_illegal_mutation(assignment_span, &*full_loan_path, loan);
-                        false
-                    } else {
-                        true
-                    }
-                });
-
-                if !cont { return false }
-            }
         }
     }