1
Fork 0

Check whether loans conflict with old loans or with themselves.

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.
This commit is contained in:
Niko Matsakis 2012-10-14 13:39:17 -07:00
parent 0643466f85
commit 2a1aa9fb53
5 changed files with 164 additions and 93 deletions

View file

@ -383,7 +383,7 @@ impl bckerr : cmp::Eq {
type bckres<T> = Result<T, bckerr>; type bckres<T> = Result<T, bckerr>;
/// a complete record of a loan that was granted /// 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` /// maps computed by `gather_loans` that are then used by `check_loans`
/// ///
@ -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 /// - `pure_map`: map from block/expr that must be pure to the error message
/// that should be reported if they are not pure /// that should be reported if they are not pure
type req_maps = { 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> pure_map: HashMap<ast::node_id, bckerr>
}; };
@ -582,6 +582,11 @@ impl borrowck_ctxt {
method_map: self.method_map}; method_map: self.method_map};
mc.mut_to_str(mutbl) 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 // The inherent mutability of a component is its default mutability

View file

@ -131,18 +131,15 @@ impl check_loan_ctxt {
} }
} }
fn walk_loans(scope_id: ast::node_id, fn walk_loans(scope_id: ast::node_id, f: fn(v: &Loan) -> bool) {
f: fn(v: &loan) -> bool) {
let mut scope_id = scope_id; let mut scope_id = scope_id;
let region_map = self.tcx().region_map; let region_map = self.tcx().region_map;
let req_loan_map = self.req_maps.req_loan_map; let req_loan_map = self.req_maps.req_loan_map;
loop { loop {
for req_loan_map.find(scope_id).each |loanss| { for req_loan_map.find(scope_id).each |loans| {
for loanss.each |loans| { for loans.each |loan| {
for loans.each |loan| { if !f(loan) { return; }
if !f(loan) { return; }
}
} }
} }
@ -155,7 +152,7 @@ impl check_loan_ctxt {
fn walk_loans_of(scope_id: ast::node_id, fn walk_loans_of(scope_id: ast::node_id,
lp: @loan_path, lp: @loan_path,
f: fn(v: &loan) -> bool) { f: fn(v: &Loan) -> bool) {
for self.walk_loans(scope_id) |loan| { for self.walk_loans(scope_id) |loan| {
if loan.lp == lp { if loan.lp == lp {
if !f(loan) { return; } if !f(loan) { return; }
@ -256,36 +253,58 @@ impl check_loan_ctxt {
} }
fn check_for_conflicting_loans(scope_id: ast::node_id) { 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, 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); let par_scope_id = self.tcx().region_map.get(scope_id);
for self.walk_loans(par_scope_id) |old_loan| { for self.walk_loans(par_scope_id) |old_loan| {
for new_loanss.each |new_loans| { debug!("old_loan=%?", self.bccx.loan_to_repr(old_loan));
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) => { for new_loans.each |new_loan| {
self.bccx.span_err( self.report_error_if_loans_conflict(old_loan, new_loan);
new_loan.cmt.span, }
fmt!("loan of %s as %s \ }
conflicts with prior loan",
self.bccx.cmt_to_str(new_loan.cmt), let len = new_loans.len();
self.bccx.mut_to_str(new_loan.mutbl))); for uint::range(0, len) |i| {
self.bccx.span_note( let loan_i = new_loans[i];
old_loan.cmt.span, for uint::range(i+1, len) |j| {
fmt!("prior loan as %s granted here", let loan_j = new_loans[j];
self.bccx.mut_to_str(old_loan.mutbl))); 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)));
} }
} }
} }

View file

@ -213,9 +213,10 @@ fn req_loans_in_expr(ex: @ast::expr,
} }
impl gather_loan_ctxt { 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) { adjustment: &ty::AutoAdjustment) {
debug!("guarantee_adjustments(expr=%s, adjustment=%?)", debug!("guarantee_adjustments(expr=%s, adjustment=%?)",
expr_repr(self.tcx(), expr), adjustment); expr_repr(self.tcx(), expr), adjustment);
@ -256,7 +257,8 @@ impl gather_loan_ctxt {
// out loans, which will be added to the `req_loan_map`. This can // out loans, which will be added to the `req_loan_map`. This can
// also entail "rooting" GC'd pointers, which means ensuring // also entail "rooting" GC'd pointers, which means ensuring
// dynamically that they are not freed. // dynamically that they are not freed.
fn guarantee_valid(cmt: cmt, fn guarantee_valid(&self,
cmt: cmt,
req_mutbl: ast::mutability, req_mutbl: ast::mutability,
scope_r: ty::region) { scope_r: ty::region) {
@ -280,35 +282,12 @@ impl gather_loan_ctxt {
// it within that scope, the loan will be detected and an // it within that scope, the loan will be detected and an
// error will be reported. // error will be reported.
Some(_) => { Some(_) => {
match self.bccx.loan(cmt, scope_r, req_mutbl) { match self.bccx.loan(cmt, scope_r, req_mutbl) {
Err(e) => { self.bccx.report(e); } Err(e) => { self.bccx.report(e); }
Ok(loans) if loans.len() == 0 => {} Ok(move loans) => {
Ok(loans) => { self.add_loans(cmt, req_mutbl, scope_r, move 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;
}
} }
_ => {
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 // The path is not loanable: in that case, we must try and
@ -385,7 +364,8 @@ impl gather_loan_ctxt {
// has type `@mut{f:int}`, this check might fail because `&x.f` // has type `@mut{f:int}`, this check might fail because `&x.f`
// reqires an immutable pointer, but `f` lives in (aliased) // reqires an immutable pointer, but `f` lives in (aliased)
// mutable memory. // mutable memory.
fn check_mutbl(req_mutbl: ast::mutability, fn check_mutbl(&self,
req_mutbl: ast::mutability,
cmt: cmt) -> bckres<preserve_condition> { cmt: cmt) -> bckres<preserve_condition> {
debug!("check_mutbl(req_mutbl=%?, cmt.mutbl=%?)", debug!("check_mutbl(req_mutbl=%?, cmt.mutbl=%?)",
req_mutbl, cmt.mutbl); req_mutbl, cmt.mutbl);
@ -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); debug!("adding %u loans to scope_id %?", loans.len(), scope_id);
match self.req_maps.req_loan_map.find(scope_id) { match self.req_maps.req_loan_map.find(scope_id) {
Some(l) => { Some(req_loans) => {
l.push(loans); req_loans.push_all(loans);
} }
None => { None => {
self.req_maps.req_loan_map.insert( let dvec = @dvec::from_vec(move loans);
scope_id, @dvec::from_vec(~[loans])); self.req_maps.req_loan_map.insert(scope_id, dvec);
} }
} }
} }
fn gather_pat(discr_cmt: cmt, root_pat: @ast::pat, fn gather_pat(&self,
arm_id: ast::node_id, alt_id: ast::node_id) { 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| { do self.bccx.cat_pattern(discr_cmt, root_pat) |cmt, pat| {
match pat.node { match pat.node {
ast::pat_ident(bm, _, _) if !self.pat_is_variant(pat) => { ast::pat_ident(bm, _, _) if !self.pat_is_variant(pat) => {
@ -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) pat_util::pat_is_variant(self.bccx.tcx.def_map, pat)
} }
} }

View file

@ -8,35 +8,37 @@ use result::{Result, Ok, Err};
impl borrowck_ctxt { impl borrowck_ctxt {
fn loan(cmt: cmt, fn loan(cmt: cmt,
scope_region: ty::region, scope_region: ty::region,
mutbl: ast::mutability) -> bckres<@DVec<loan>> { mutbl: ast::mutability) -> bckres<~[Loan]> {
let lc = loan_ctxt_(@{bccx: self, let lc = LoanContext {
scope_region: scope_region, bccx: self,
loans: @DVec()}); scope_region: scope_region,
loans: ~[]
};
match lc.loan(cmt, mutbl) { 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, bccx: borrowck_ctxt,
// the region scope for which we must preserve the memory // the region scope for which we must preserve the memory
scope_region: ty::region, scope_region: ty::region,
// accumulated list of loans that will be required // accumulated list of loans that will be required
loans: @DVec<loan> mut loans: ~[Loan]
};
enum loan_ctxt {
loan_ctxt_(@loan_ctxt_)
} }
impl loan_ctxt { impl LoanContext {
fn tcx() -> ty::ctxt { self.bccx.tcx } fn tcx(&self) -> ty::ctxt { self.bccx.tcx }
fn issue_loan(cmt: cmt, fn issue_loan(&self,
cmt: cmt,
scope_ub: ty::region, scope_ub: ty::region,
req_mutbl: ast::mutability) -> bckres<()> { req_mutbl: ast::mutability) -> bckres<()> {
if self.bccx.is_subregion_of(self.scope_region, scope_ub) { if self.bccx.is_subregion_of(self.scope_region, scope_ub) {
@ -57,12 +59,13 @@ impl loan_ctxt {
} }
} }
(*self.loans).push({ self.loans.push(Loan {
// Note: cmt.lp must be Some(_) because otherwise this // Note: cmt.lp must be Some(_) because otherwise this
// loan process does not apply at all. // loan process does not apply at all.
lp: cmt.lp.get(), lp: cmt.lp.get(),
cmt: cmt, cmt: cmt,
mutbl: req_mutbl}); mutbl: req_mutbl
});
return Ok(()); return Ok(());
} else { } else {
// The loan being requested lives longer than the data // The loan being requested lives longer than the data
@ -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)", debug!("loan(%s, %s)",
self.bccx.cmt_to_repr(cmt), self.bccx.cmt_to_repr(cmt),
self.bccx.mut_to_str(req_mutbl)); self.bccx.mut_to_str(req_mutbl));
@ -144,7 +147,8 @@ impl loan_ctxt {
// A "stable component" is one where assigning the base of the // A "stable component" is one where assigning the base of the
// component cannot cause the component itself to change types. // component cannot cause the component itself to change types.
// Example: record fields. // Example: record fields.
fn loan_stable_comp(cmt: cmt, fn loan_stable_comp(&self,
cmt: cmt,
cmt_base: cmt, cmt_base: cmt,
req_mutbl: ast::mutability) -> bckres<()> { req_mutbl: ast::mutability) -> bckres<()> {
let base_mutbl = match req_mutbl { let base_mutbl = match req_mutbl {
@ -162,7 +166,8 @@ impl loan_ctxt {
// An "unstable deref" means a deref of a ptr/comp where, if the // 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 // base of the deref is assigned to, pointers into the result of the
// deref would be invalidated. Examples: interior of variants, uniques. // 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, cmt_base: cmt,
req_mutbl: ast::mutability) -> bckres<()> { req_mutbl: ast::mutability) -> bckres<()> {
// Variant components: the base must be immutable, because // Variant components: the base must be immutable, because

View file

@ -0,0 +1,25 @@
use core::either::{Either, Left, Right};
fn f(x: &mut Either<int,float>, y: &Either<int,float>) -> int {
match *y {
Left(ref z) => {
*x = Right(1.0);
*z
}
_ => fail
}
}
fn g() {
let mut x: Either<int,float> = Left(3);
io::println(f(&mut x, &x).to_str()); //~ ERROR conflicts with prior loan
}
fn h() {
let mut x: Either<int,float> = Left(3);
let y: &Either<int, float> = &x;
let z: &mut Either<int, float> = &mut x; //~ ERROR conflicts with prior loan
*z = *y;
}
fn main() {}