Skip to content

Commit

Permalink
Check whether loans conflict with old loans or with themselves.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@nikomatsakis
nikomatsakis committed Oct 15, 2012
1 parent 0643466 commit 2a1aa9f
Show file tree
Hide file tree
Showing 5 changed files with 165 additions and 94 deletions.
9 changes: 7 additions & 2 deletions src/rustc/middle/borrowck.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`
///
Expand All @@ -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>
};

Expand Down Expand Up @@ -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
Expand Down
85 changes: 52 additions & 33 deletions src/rustc/middle/borrowck/check_loans.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
}
}
Expand All @@ -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; }
Expand Down Expand Up @@ -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)));
}
}
}
Expand Down
95 changes: 56 additions & 39 deletions src/rustc/middle/borrowck/gather_loans.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {

Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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) => {
Expand Down Expand Up @@ -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)
}
}
Expand Down
45 changes: 25 additions & 20 deletions src/rustc/middle/borrowck/loan.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -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));
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down
Loading

0 comments on commit 2a1aa9f

Please sign in to comment.