Skip to content

Remove restrictions from borrowck #14947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jun 17, 2014
Merged

Remove restrictions from borrowck #14947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
2f77212
Remove comments that no longer apply after the removal of `const`
Jun 16, 2014
2d3f122
Remove a pointless check for intersection with RESTR_MUTATE
Jun 16, 2014
d2ca746
Remove an unused closure parameter
Jun 16, 2014
6ca8dbf
Make a new compatible_borrow_kinds helper function
Jun 16, 2014
6849362
Remove the last actual usage of RestrictionSet
Jun 16, 2014
9934759
Remove an unused bkcerr_code constructor parameter
Jun 16, 2014
59309e0
Remove RestrictionSet
Jun 16, 2014
e018fc3
Remove Restriction
Jun 16, 2014
ba203c5
Add a new each_in_scope_loan_affecting_path helper function
Jun 16, 2014
702ef1b
Call mark_variable_as_used_mut even after assignment errors
Jun 16, 2014
178c4fb
Remove an unused return value
Jun 16, 2014
69f4839
Always check assigned loan paths to the top of the path
Jun 16, 2014
a924d74
Switch to each_in_scope_loan_affecting_path
Jun 16, 2014
480cd6f
Rename check_for_assignment_to_restricted_or_frozen_location
Jun 16, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
289 changes: 82 additions & 207 deletions src/librustc/middle/borrowck/check_loans.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how hard it would be to make these into proper iterators. It'd make the code so much nicer.

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> {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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 }
}
}
}

Expand Down
Loading