Skip to content
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.

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

Check whether loans conflict with old loans or with themselves. #3769

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 7 additions & 2 deletions src/rustc/middle/borrowck.rs
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
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
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
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