From 2f77212e0c37ee31df12373622974977ba771a04 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Mon, 16 Jun 2014 15:40:20 -0700 Subject: Remove comments that no longer apply after the removal of `const` --- src/librustc/middle/borrowck/check_loans.rs | 42 +++-------------------------- 1 file changed, 4 insertions(+), 38 deletions(-) (limited to 'src') diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 11144118047..077bb99e21b 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -829,28 +829,8 @@ impl<'a> CheckLoanCtxt<'a> { // 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. + // In this case, creating `p` restricts the mutation of `v`. + let cont = this.each_in_scope_restriction(assignment_id, &*loan_path, |loan, restr| { @@ -882,28 +862,14 @@ impl<'a> CheckLoanCtxt<'a> { // // 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). + // path, and check that the super path was not borrowed. // // 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 { -- cgit 1.4.1-3-g733a5 From 2d3f1225342760bcf986d0a7d3945ddacce47ab2 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Mon, 16 Jun 2014 15:40:20 -0700 Subject: Remove a pointless check for intersection with RESTR_MUTATE Now that all loans restrict mutation, there's no point in checking for intersection with RESTR_MUTATE. --- src/librustc/middle/borrowck/check_loans.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 077bb99e21b..969a8c139a4 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -833,13 +833,9 @@ impl<'a> CheckLoanCtxt<'a> { 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 - } + |loan, _restr| { + this.report_illegal_mutation(assignment_span, &*loan_path, loan); + false }); if !cont { return false } -- cgit 1.4.1-3-g733a5 From d2ca7465aa0b589b9a3e84423f5073da33bf0bb3 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Mon, 16 Jun 2014 15:40:20 -0700 Subject: Remove an unused closure parameter Remove the unused &Restriction parameter of each_in_scope_restriction's op parameter. --- src/librustc/middle/borrowck/check_loans.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 969a8c139a4..3c8516a5706 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -192,7 +192,7 @@ impl<'a> CheckLoanCtxt<'a> { pub fn each_in_scope_restriction(&self, scope_id: ast::NodeId, loan_path: &LoanPath, - op: |&Loan, &Restriction| -> bool) + op: |&Loan| -> bool) -> bool { //! Iterates through all the in-scope restrictions for the //! given `loan_path` @@ -204,7 +204,7 @@ impl<'a> CheckLoanCtxt<'a> { let mut ret = true; for restr in loan.restrictions.iter() { if *restr.loan_path == *loan_path { - if !op(loan, restr) { + if !op(loan) { ret = false; break; } @@ -541,7 +541,7 @@ impl<'a> CheckLoanCtxt<'a> { // 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| { + self.each_in_scope_restriction(expr_id, use_path, |loan| { if incompatible(loan.kind, borrow_kind) { ret = UseWhileBorrowed(loan.loan_path.clone(), loan.span); false @@ -833,7 +833,7 @@ impl<'a> CheckLoanCtxt<'a> { let cont = this.each_in_scope_restriction(assignment_id, &*loan_path, - |loan, _restr| { + |loan| { this.report_illegal_mutation(assignment_span, &*loan_path, loan); false }); -- cgit 1.4.1-3-g733a5 From 6ca8dbfaedc9d37b9b40fd562755a358e7810266 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Mon, 16 Jun 2014 15:40:20 -0700 Subject: Make a new compatible_borrow_kinds helper function Move the `incompatible` helper function from analyze_restrictions_on_use to the file scope and invert its meaning to account for future uses. --- src/librustc/middle/borrowck/check_loans.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 3c8516a5706..9061d2d38d3 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -155,6 +155,12 @@ enum UseError { UseWhileBorrowed(/*loan*/Rc, /*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 } @@ -542,7 +548,7 @@ impl<'a> CheckLoanCtxt<'a> { // let y = a; // Conflicts with restriction self.each_in_scope_restriction(expr_id, use_path, |loan| { - if incompatible(loan.kind, borrow_kind) { + if !compatible_borrow_kinds(loan.kind, borrow_kind) { ret = UseWhileBorrowed(loan.loan_path.clone(), loan.span); false } else { @@ -566,7 +572,7 @@ impl<'a> CheckLoanCtxt<'a> { loop { self.each_in_scope_loan(expr_id, |loan| { if *loan.loan_path == *loan_path && - incompatible(loan.kind, borrow_kind) { + !compatible_borrow_kinds(loan.kind, borrow_kind) { ret = UseWhileBorrowed(loan.loan_path.clone(), loan.span); false } else { @@ -585,12 +591,6 @@ impl<'a> CheckLoanCtxt<'a> { } 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, -- cgit 1.4.1-3-g733a5 From 6849362f383c9c21a72cea5377c28596061c6c13 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Mon, 16 Jun 2014 15:40:20 -0700 Subject: Remove the last actual usage of RestrictionSet Switch to checking BorrowKind values of loans instead of their RestrictionSet values. This was the last code that made a decision based on a RestrictionSet. --- src/librustc/middle/borrowck/check_loans.rs | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) (limited to 'src') diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 9061d2d38d3..20c0f8f7866 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -294,25 +294,11 @@ 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; } let old_pronoun = if new_loan.loan_path == old_loan.loan_path { -- cgit 1.4.1-3-g733a5 From 99347591956958ab62074a41a83b4a62587c2ee6 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Mon, 16 Jun 2014 15:40:20 -0700 Subject: Remove an unused bkcerr_code constructor parameter The RestrictionSet parameter of the err_borrowed_pointer_too_short constructor isn't actually used, so it can be removed. --- src/librustc/middle/borrowck/gather_loans/restrictions.rs | 4 ++-- src/librustc/middle/borrowck/mod.rs | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/librustc/middle/borrowck/gather_loans/restrictions.rs b/src/librustc/middle/borrowck/gather_loans/restrictions.rs index 7c1f5937472..94d88915b56 100644 --- a/src/librustc/middle/borrowck/gather_loans/restrictions.rs +++ b/src/librustc/middle/borrowck/gather_loans/restrictions.rs @@ -133,7 +133,7 @@ impl<'a> RestrictionsContext<'a> { cause: self.cause, cmt: cmt_base, code: err_borrowed_pointer_too_short( - self.loan_region, lt, restrictions)}); + self.loan_region, lt)}); return Safe; } Safe @@ -148,7 +148,7 @@ impl<'a> RestrictionsContext<'a> { cause: self.cause, cmt: cmt_base, code: err_borrowed_pointer_too_short( - self.loan_region, lt, restrictions)}); + self.loan_region, lt)}); return Safe; } diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs index 44d722c2094..aded2974c12 100644 --- a/src/librustc/middle/borrowck/mod.rs +++ b/src/librustc/middle/borrowck/mod.rs @@ -310,8 +310,7 @@ impl Repr for RestrictionSet { pub enum bckerr_code { err_mutbl, err_out_of_scope(ty::Region, ty::Region), // superscope, subscope - err_borrowed_pointer_too_short( - ty::Region, ty::Region, RestrictionSet), // loan, ptr + err_borrowed_pointer_too_short(ty::Region, ty::Region), // loan, ptr } // Combination of an error code and the categorization of the expression @@ -711,7 +710,7 @@ impl<'a> BorrowckCtxt<'a> { suggestion); } - err_borrowed_pointer_too_short(loan_scope, ptr_scope, _) => { + err_borrowed_pointer_too_short(loan_scope, ptr_scope) => { let descr = match opt_loan_path(&err.cmt) { Some(lp) => { format!("`{}`", self.loan_path_to_str(&*lp)) -- cgit 1.4.1-3-g733a5 From 59309e0d9bf3eba0f4219062b451ff10cbdf0e2b Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Mon, 16 Jun 2014 15:40:20 -0700 Subject: Remove RestrictionSet Now that RestrictionSet is no longer being used for anything meaningful, it can be removed, along with any other associated functions and RestrictionSet fields of other types. --- src/librustc/middle/borrowck/gather_loans/mod.rs | 17 +------ .../middle/borrowck/gather_loans/restrictions.rs | 45 ++++++------------ src/librustc/middle/borrowck/mod.rs | 53 +--------------------- 3 files changed, 17 insertions(+), 98 deletions(-) (limited to 'src') diff --git a/src/librustc/middle/borrowck/gather_loans/mod.rs b/src/librustc/middle/borrowck/gather_loans/mod.rs index ac94b735640..a9420a3c409 100644 --- a/src/librustc/middle/borrowck/gather_loans/mod.rs +++ b/src/librustc/middle/borrowck/gather_loans/mod.rs @@ -259,7 +259,7 @@ impl<'a> GatherLoanCtxt<'a> { // loan is safe. let restr = restrictions::compute_restrictions( self.bccx, borrow_span, cause, - cmt.clone(), loan_region, self.restriction_set(req_kind)); + cmt.clone(), loan_region); // Create the loan record (if needed). let loan = match restr { @@ -390,21 +390,6 @@ impl<'a> GatherLoanCtxt<'a> { } } - fn restriction_set(&self, req_kind: ty::BorrowKind) -> RestrictionSet { - match req_kind { - // If borrowing data as immutable, no mutation allowed: - ty::ImmBorrow => RESTR_MUTATE, - - // If borrowing data as mutable, no mutation nor other - // borrows allowed: - ty::MutBorrow => RESTR_MUTATE | RESTR_FREEZE, - - // If borrowing data as unique imm, no mutation nor other - // borrows allowed: - ty::UniqueImmBorrow => RESTR_MUTATE | RESTR_FREEZE, - } - } - pub fn mark_loan_path_as_mutated(&self, loan_path: &LoanPath) { //! For mutable loans of content whose mutability derives //! from a local variable, mark the mutability decl as necessary. diff --git a/src/librustc/middle/borrowck/gather_loans/restrictions.rs b/src/librustc/middle/borrowck/gather_loans/restrictions.rs index 94d88915b56..de875dfff3e 100644 --- a/src/librustc/middle/borrowck/gather_loans/restrictions.rs +++ b/src/librustc/middle/borrowck/gather_loans/restrictions.rs @@ -30,8 +30,7 @@ pub fn compute_restrictions(bccx: &BorrowckCtxt, span: Span, cause: euv::LoanCause, cmt: mc::cmt, - loan_region: ty::Region, - restr: RestrictionSet) -> RestrictionResult { + loan_region: ty::Region) -> RestrictionResult { let ctxt = RestrictionsContext { bccx: bccx, span: span, @@ -39,7 +38,7 @@ pub fn compute_restrictions(bccx: &BorrowckCtxt, loan_region: loan_region, }; - ctxt.restrict(cmt, restr) + ctxt.restrict(cmt) } /////////////////////////////////////////////////////////////////////////// @@ -54,11 +53,8 @@ struct RestrictionsContext<'a> { impl<'a> RestrictionsContext<'a> { fn restrict(&self, - cmt: mc::cmt, - restrictions: RestrictionSet) -> RestrictionResult { - debug!("restrict(cmt={}, restrictions={})", - cmt.repr(self.bccx.tcx), - restrictions.repr(self.bccx.tcx)); + cmt: mc::cmt) -> RestrictionResult { + debug!("restrict(cmt={})", cmt.repr(self.bccx.tcx)); match cmt.cat.clone() { mc::cat_rvalue(..) => { @@ -75,19 +71,14 @@ impl<'a> RestrictionsContext<'a> { mc::cat_upvar(ty::UpvarId {var_id: local_id, ..}, _) => { // R-Variable let lp = Rc::new(LpVar(local_id)); - SafeIf(lp.clone(), vec!(Restriction { - loan_path: lp, - set: restrictions - })) + SafeIf(lp.clone(), vec!(Restriction { loan_path: lp })) } mc::cat_downcast(cmt_base) => { // When we borrow the interior of an enum, we have to // ensure the enum itself is not mutated, because that // could cause the type of the memory to change. - self.restrict( - cmt_base, - restrictions | RESTR_MUTATE) + self.restrict(cmt_base) } mc::cat_interior(cmt_base, i) => { @@ -96,8 +87,8 @@ impl<'a> RestrictionsContext<'a> { // Overwriting the base would not change the type of // the memory, so no additional restrictions are // needed. - let result = self.restrict(cmt_base, restrictions); - self.extend(result, cmt.mutbl, LpInterior(i), restrictions) + let result = self.restrict(cmt_base); + self.extend(result, cmt.mutbl, LpInterior(i)) } mc::cat_deref(cmt_base, _, pk @ mc::OwnedPtr) | @@ -112,10 +103,8 @@ impl<'a> RestrictionsContext<'a> { // same, because this could be the last ref. // Eventually we should make these non-special and // just rely on Deref implementation. - let result = self.restrict( - cmt_base, - restrictions | RESTR_MUTATE); - self.extend(result, cmt.mutbl, LpDeref(pk), restrictions) + let result = self.restrict(cmt_base); + self.extend(result, cmt.mutbl, LpDeref(pk)) } mc::cat_copied_upvar(..) | // FIXME(#2152) allow mutation of upvars @@ -152,8 +141,8 @@ impl<'a> RestrictionsContext<'a> { return Safe; } - let result = self.restrict(cmt_base, restrictions); - self.extend(result, cmt.mutbl, LpDeref(pk), restrictions) + let result = self.restrict(cmt_base); + self.extend(result, cmt.mutbl, LpDeref(pk)) } mc::cat_deref(_, _, mc::UnsafePtr(..)) => { @@ -162,7 +151,7 @@ impl<'a> RestrictionsContext<'a> { } mc::cat_discr(cmt_base, _) => { - self.restrict(cmt_base, restrictions) + self.restrict(cmt_base) } } } @@ -170,16 +159,12 @@ impl<'a> RestrictionsContext<'a> { fn extend(&self, result: RestrictionResult, mc: mc::MutabilityCategory, - elem: LoanPathElem, - restrictions: RestrictionSet) -> RestrictionResult { + elem: LoanPathElem) -> RestrictionResult { match result { Safe => Safe, SafeIf(base_lp, mut base_vec) => { let lp = Rc::new(LpExtend(base_lp, mc, elem)); - base_vec.push(Restriction { - loan_path: lp.clone(), - set: restrictions - }); + base_vec.push(Restriction { loan_path: lp.clone() }); SafeIf(lp, base_vec) } } diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs index aded2974c12..77b57dfc593 100644 --- a/src/librustc/middle/borrowck/mod.rs +++ b/src/librustc/middle/borrowck/mod.rs @@ -21,7 +21,6 @@ use middle::ty; use util::ppaux::{note_and_explain_region, Repr, UserString}; use std::cell::{Cell}; -use std::ops::{BitOr, BitAnd}; use std::rc::Rc; use std::gc::{Gc, GC}; use std::string::String; @@ -250,56 +249,8 @@ pub fn opt_loan_path(cmt: &mc::cmt) -> Option> { } } -/////////////////////////////////////////////////////////////////////////// -// Restrictions -// -// Borrowing an lvalue often results in *restrictions* that limit what -// can be done with this lvalue during the scope of the loan: -// -// - `RESTR_MUTATE`: The lvalue may not be modified or `&mut` borrowed. -// - `RESTR_FREEZE`: `&` borrows of the lvalue are forbidden. -// -// In addition, no value which is restricted may be moved. Therefore, -// restrictions are meaningful even if the RestrictionSet is empty, -// because the restriction against moves is implied. - pub struct Restriction { loan_path: Rc, - set: RestrictionSet -} - -#[deriving(PartialEq)] -pub struct RestrictionSet { - bits: u32 -} - -#[allow(dead_code)] // potentially useful -pub static RESTR_EMPTY: RestrictionSet = RestrictionSet {bits: 0b0000}; -pub static RESTR_MUTATE: RestrictionSet = RestrictionSet {bits: 0b0001}; -pub static RESTR_FREEZE: RestrictionSet = RestrictionSet {bits: 0b0010}; - -impl RestrictionSet { - pub fn intersects(&self, restr: RestrictionSet) -> bool { - (self.bits & restr.bits) != 0 - } -} - -impl BitOr for RestrictionSet { - fn bitor(&self, rhs: &RestrictionSet) -> RestrictionSet { - RestrictionSet {bits: self.bits | rhs.bits} - } -} - -impl BitAnd for RestrictionSet { - fn bitand(&self, rhs: &RestrictionSet) -> RestrictionSet { - RestrictionSet {bits: self.bits & rhs.bits} - } -} - -impl Repr for RestrictionSet { - fn repr(&self, _tcx: &ty::ctxt) -> String { - format!("RestrictionSet(0x{:x})", self.bits as uint) - } } /////////////////////////////////////////////////////////////////////////// @@ -832,9 +783,7 @@ impl Repr for Loan { impl Repr for Restriction { fn repr(&self, tcx: &ty::ctxt) -> String { - (format!("Restriction({}, {:x})", - self.loan_path.repr(tcx), - self.set.bits as uint)).to_string() + (format!("Restriction({})", self.loan_path.repr(tcx))).to_string() } } -- cgit 1.4.1-3-g733a5 From e018fc36d17685c31d6b31acf17a9a8f2f73f8d9 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Mon, 16 Jun 2014 15:40:20 -0700 Subject: Remove Restriction The Restriction struct now consists of a single Rc field, so it can be replaced with Rc. --- src/librustc/middle/borrowck/check_loans.rs | 8 ++++---- src/librustc/middle/borrowck/gather_loans/mod.rs | 4 ++-- src/librustc/middle/borrowck/gather_loans/restrictions.rs | 6 +++--- src/librustc/middle/borrowck/mod.rs | 14 ++------------ 4 files changed, 11 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 20c0f8f7866..70cb06e2a30 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -208,8 +208,8 @@ impl<'a> CheckLoanCtxt<'a> { loan.repr(self.tcx())); let mut ret = true; - for restr in loan.restrictions.iter() { - if *restr.loan_path == *loan_path { + for restr_path in loan.restricted_paths.iter() { + if **restr_path == *loan_path { if !op(loan) { ret = false; break; @@ -298,8 +298,8 @@ impl<'a> CheckLoanCtxt<'a> { return true; } - for restr in loan1.restrictions.iter() { - 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() diff --git a/src/librustc/middle/borrowck/gather_loans/mod.rs b/src/librustc/middle/borrowck/gather_loans/mod.rs index a9420a3c409..89f304513ff 100644 --- a/src/librustc/middle/borrowck/gather_loans/mod.rs +++ b/src/librustc/middle/borrowck/gather_loans/mod.rs @@ -268,7 +268,7 @@ impl<'a> GatherLoanCtxt<'a> { return; } - restrictions::SafeIf(loan_path, restrictions) => { + restrictions::SafeIf(loan_path, restricted_paths) => { let loan_scope = match loan_region { ty::ReScope(id) => id, ty::ReFree(ref fr) => fr.scope_id, @@ -314,7 +314,7 @@ impl<'a> GatherLoanCtxt<'a> { gen_scope: gen_scope, kill_scope: kill_scope, span: borrow_span, - restrictions: restrictions, + restricted_paths: restricted_paths, cause: cause, } } diff --git a/src/librustc/middle/borrowck/gather_loans/restrictions.rs b/src/librustc/middle/borrowck/gather_loans/restrictions.rs index de875dfff3e..5b3e1ac0b2c 100644 --- a/src/librustc/middle/borrowck/gather_loans/restrictions.rs +++ b/src/librustc/middle/borrowck/gather_loans/restrictions.rs @@ -23,7 +23,7 @@ use std::rc::Rc; pub enum RestrictionResult { Safe, - SafeIf(Rc, Vec) + SafeIf(Rc, Vec>) } pub fn compute_restrictions(bccx: &BorrowckCtxt, @@ -71,7 +71,7 @@ impl<'a> RestrictionsContext<'a> { mc::cat_upvar(ty::UpvarId {var_id: local_id, ..}, _) => { // R-Variable let lp = Rc::new(LpVar(local_id)); - SafeIf(lp.clone(), vec!(Restriction { loan_path: lp })) + SafeIf(lp.clone(), vec!(lp)) } mc::cat_downcast(cmt_base) => { @@ -164,7 +164,7 @@ impl<'a> RestrictionsContext<'a> { Safe => Safe, SafeIf(base_lp, mut base_vec) => { let lp = Rc::new(LpExtend(base_lp, mc, elem)); - base_vec.push(Restriction { loan_path: lp.clone() }); + base_vec.push(lp.clone()); SafeIf(lp, base_vec) } } diff --git a/src/librustc/middle/borrowck/mod.rs b/src/librustc/middle/borrowck/mod.rs index 77b57dfc593..18cd0b1326d 100644 --- a/src/librustc/middle/borrowck/mod.rs +++ b/src/librustc/middle/borrowck/mod.rs @@ -181,7 +181,7 @@ pub struct Loan { index: uint, loan_path: Rc, kind: ty::BorrowKind, - restrictions: Vec, + restricted_paths: Vec>, gen_scope: ast::NodeId, kill_scope: ast::NodeId, span: Span, @@ -249,10 +249,6 @@ pub fn opt_loan_path(cmt: &mc::cmt) -> Option> { } } -pub struct Restriction { - loan_path: Rc, -} - /////////////////////////////////////////////////////////////////////////// // Errors @@ -777,13 +773,7 @@ impl Repr for Loan { self.kind, self.gen_scope, self.kill_scope, - self.restrictions.repr(tcx))).to_string() - } -} - -impl Repr for Restriction { - fn repr(&self, tcx: &ty::ctxt) -> String { - (format!("Restriction({})", self.loan_path.repr(tcx))).to_string() + self.restricted_paths.repr(tcx))).to_string() } } -- cgit 1.4.1-3-g733a5 From ba203c5c5de5af9941dc5be4b7c200db69b3c4d4 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Mon, 16 Jun 2014 15:40:20 -0700 Subject: Add a new each_in_scope_loan_affecting_path helper function Add a helper function that generalizes the loan path restriction strategy used by analyze_restrictions_on_use. --- src/librustc/middle/borrowck/check_loans.rs | 114 ++++++++++++++++++---------- 1 file changed, 72 insertions(+), 42 deletions(-) (limited to 'src') diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 70cb06e2a30..131ed8dbba6 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -220,6 +220,77 @@ impl<'a> CheckLoanCtxt<'a> { }) } + 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. + + // 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_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 { //! Returns a vector of the loans that are generated as //! we encounter `scope_id`. @@ -526,14 +597,7 @@ 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| { + 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 @@ -542,40 +606,6 @@ impl<'a> CheckLoanCtxt<'a> { } }); - // 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 && - !compatible_borrow_kinds(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; } -- cgit 1.4.1-3-g733a5 From 702ef1b7215e310a0c5e2e761bef922f36dd5f65 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Mon, 16 Jun 2014 15:40:20 -0700 Subject: Call mark_variable_as_used_mut even after assignment errors It doesn't actually matter if we call mark_variable_as_used_mut when the assignment was invalid, since the variable was still used mutably. --- src/librustc/middle/borrowck/check_loans.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 131ed8dbba6..ac8b0867f7a 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -684,11 +684,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 && + 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: + self, assignment_id, assignment_span, assignee_cmt.clone()); mark_variable_as_used_mut(self, assignee_cmt); } } -- cgit 1.4.1-3-g733a5 From 178c4fbccbc59fc7c554cb3cda33413d7d455366 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Mon, 16 Jun 2014 15:40:20 -0700 Subject: Remove an unused return value The only caller of check_for_assignment_to_restricted_or_frozen_location isn't checking its return value, so we can remove it and simplify the internal logic of the function. --- src/librustc/middle/borrowck/check_loans.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index ac8b0867f7a..42712206c01 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -825,14 +825,14 @@ impl<'a> CheckLoanCtxt<'a> { 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* @@ -852,7 +852,7 @@ impl<'a> CheckLoanCtxt<'a> { false }); - if !cont { return false } + if !cont { return; } // The previous code handled assignments to paths that // have been restricted. This covers paths that have been @@ -899,12 +899,12 @@ impl<'a> CheckLoanCtxt<'a> { LpExtend(_, mc::McDeclared, _) | LpExtend(_, mc::McImmutable, _) | LpVar(_) => { - return true; + return; } }; // Check for a non-const loan of `loan_path` - let cont = this.each_in_scope_loan(assignment_id, |loan| { + 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 @@ -912,8 +912,6 @@ impl<'a> CheckLoanCtxt<'a> { true } }); - - if !cont { return false } } } } -- cgit 1.4.1-3-g733a5 From 69f4839b926d6435df83cd80945d9f843cf60e27 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Mon, 16 Jun 2014 15:40:20 -0700 Subject: Always check assigned loan paths to the top of the path Currently, check_for_assignment_to_restricted_or_frozen_location bails out early when looking for loaned base paths when it hits an McDeclared or McImmutable extension. With the current type system, this is actually irrelevant, since mutation can only occur given a unique mutable access path, forcing the same requirement on each base path. --- src/librustc/middle/borrowck/check_loans.rs | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 42712206c01..d36eb6751b1 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -870,34 +870,16 @@ impl<'a> CheckLoanCtxt<'a> { // 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 borrowed. - // - // 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. + // So in this loop, we walk back up the path and look for + // loans, not restrictions. 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, _))) => { + LpExtend(ref lp_base, _, _) => { lp_base.clone() } - - // Otherwise stop iterating - LpExtend(_, mc::McDeclared, _) | - LpExtend(_, mc::McImmutable, _) | LpVar(_) => { return; } -- cgit 1.4.1-3-g733a5 From a924d740df600e2a71656ed477711b0f2a68c722 Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Mon, 16 Jun 2014 15:40:21 -0700 Subject: Switch to each_in_scope_loan_affecting_path The last remaining use of each_in_scope_restriction in check_for_assignment_to_restricted_or_frozen_location is using the pattern captured by each_in_scope_loan_affecting_path, so it can be removed. --- src/librustc/middle/borrowck/check_loans.rs | 83 +---------------------------- 1 file changed, 1 insertion(+), 82 deletions(-) (limited to 'src') diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index d36eb6751b1..5c18001930e 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -195,31 +195,6 @@ impl<'a> CheckLoanCtxt<'a> { }) } - pub fn each_in_scope_restriction(&self, - scope_id: ast::NodeId, - loan_path: &LoanPath, - op: |&Loan| -> bool) - -> bool { - //! Iterates through all the in-scope restrictions for the - //! given `loan_path` - - self.each_in_scope_loan(scope_id, |loan| { - debug!("each_in_scope_restriction found loan: {:?}", - loan.repr(self.tcx())); - - let mut ret = true; - for restr_path in loan.restricted_paths.iter() { - if **restr_path == *loan_path { - if !op(loan) { - ret = false; - break; - } - } - } - ret - }) - } - fn each_in_scope_loan_affecting_path(&self, scope_id: ast::NodeId, loan_path: &LoanPath, @@ -835,66 +810,10 @@ impl<'a> CheckLoanCtxt<'a> { 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` restricts the mutation of `v`. - - let cont = this.each_in_scope_restriction(assignment_id, - &*loan_path, - |loan| { + 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; } - - // 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 path and look for - // loans, not restrictions. - - let full_loan_path = loan_path.clone(); - let mut loan_path = loan_path; - loop { - loan_path = match *loan_path { - LpExtend(ref lp_base, _, _) => { - lp_base.clone() - } - LpVar(_) => { - return; - } - }; - - // Check for a non-const loan of `loan_path` - 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 - } - }); - } } } -- cgit 1.4.1-3-g733a5 From 480cd6fb9052e3db33e01c94e1193f80801b292f Mon Sep 17 00:00:00 2001 From: Cameron Zwarich Date: Mon, 16 Jun 2014 15:40:21 -0700 Subject: Rename check_for_assignment_to_restricted_or_frozen_location Rename check_for_assignment_to_restricted_or_frozen_location to check_for_assignment_to_borrowed_path. --- src/librustc/middle/borrowck/check_loans.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/librustc/middle/borrowck/check_loans.rs b/src/librustc/middle/borrowck/check_loans.rs index 5c18001930e..166d069880f 100644 --- a/src/librustc/middle/borrowck/check_loans.rs +++ b/src/librustc/middle/borrowck/check_loans.rs @@ -660,7 +660,7 @@ impl<'a> CheckLoanCtxt<'a> { 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( + check_for_assignment_to_borrowed_path( self, assignment_id, assignment_span, assignee_cmt.clone()); mark_variable_as_used_mut(self, assignee_cmt); } @@ -796,7 +796,7 @@ 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, -- cgit 1.4.1-3-g733a5