Skip to content

Adjust borrow checker algorithm to address #4856 unsoundness, #4914

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

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
8 changes: 6 additions & 2 deletions src/libcore/hashmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,9 @@ pub mod linear {
},
};

self.value_for_bucket(idx)
unsafe { // FIXME(#4903)---requires flow-sensitive borrow checker
::cast::transmute_region(self.value_for_bucket(idx))
}
}

/// Return the value corresponding to the key in the map, or create,
Expand Down Expand Up @@ -412,7 +414,9 @@ pub mod linear {
},
};

self.value_for_bucket(idx)
unsafe { // FIXME(#4903)---requires flow-sensitive borrow checker
::cast::transmute_region(self.value_for_bucket(idx))
}
}

fn consume(&mut self, f: fn(K, V)) {
Expand Down
28 changes: 15 additions & 13 deletions src/libcore/private.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,15 +256,15 @@ pub unsafe fn shared_mutable_state<T: Owned>(data: T) ->
}

#[inline(always)]
pub unsafe fn get_shared_mutable_state<T: Owned>(rc: &a/SharedMutableState<T>)
-> &a/mut T {
pub unsafe fn get_shared_mutable_state<T: Owned>(
rc: *SharedMutableState<T>) -> *mut T
{
unsafe {
let ptr: ~ArcData<T> = cast::reinterpret_cast(&(*rc).data);
assert ptr.count > 0;
// Cast us back into the correct region
let r = cast::transmute_region(option::get_ref(&ptr.data));
let r = cast::transmute(option::get_ref(&ptr.data));
cast::forget(move ptr);
return cast::transmute_mut(r);
return r;
}
}
#[inline(always)]
Expand Down Expand Up @@ -376,15 +376,17 @@ impl<T: Owned> Exclusive<T> {
// the exclusive. Supporting that is a work in progress.
#[inline(always)]
unsafe fn with<U>(f: fn(x: &mut T) -> U) -> U {
let rec = unsafe { get_shared_mutable_state(&self.x) };
do rec.lock.lock {
if rec.failed {
die!(~"Poisoned exclusive - another task failed inside!");
unsafe {
let rec = get_shared_mutable_state(&self.x);
do (*rec).lock.lock {
if (*rec).failed {
die!(~"Poisoned exclusive - another task failed inside!");
}
(*rec).failed = true;
let result = f(&mut (*rec).data);
(*rec).failed = false;
move result
}
rec.failed = true;
let result = f(&mut rec.data);
rec.failed = false;
move result
}
}

Expand Down
8 changes: 5 additions & 3 deletions src/libcore/str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2071,17 +2071,19 @@ pub mod raw {

/// Appends a byte to a string. (Not UTF-8 safe).
pub unsafe fn push_byte(s: &mut ~str, b: u8) {
reserve_at_least(&mut *s, s.len() + 1);
let new_len = s.len() + 1;
reserve_at_least(&mut *s, new_len);
do as_buf(*s) |buf, len| {
let buf: *mut u8 = ::cast::reinterpret_cast(&buf);
*ptr::mut_offset(buf, len) = b;
}
set_len(&mut *s, s.len() + 1);
set_len(&mut *s, new_len);
}

/// Appends a vector of bytes to a string. (Not UTF-8 safe).
unsafe fn push_bytes(s: &mut ~str, bytes: &[u8]) {
reserve_at_least(&mut *s, s.len() + bytes.len());
let new_len = s.len() + bytes.len();
reserve_at_least(&mut *s, new_len);
for vec::each(bytes) |byte| { push_byte(&mut *s, *byte); }
}

Expand Down
17 changes: 11 additions & 6 deletions src/libcore/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -623,13 +623,15 @@ unsafe fn push_fast<T>(v: &mut ~[T], initval: T) {

#[inline(never)]
fn push_slow<T>(v: &mut ~[T], initval: T) {
reserve_at_least(&mut *v, v.len() + 1u);
let new_len = v.len() + 1;
reserve_at_least(&mut *v, new_len);
unsafe { push_fast(v, initval) }
}

#[inline(always)]
pub fn push_all<T: Copy>(v: &mut ~[T], rhs: &[const T]) {
reserve(&mut *v, v.len() + rhs.len());
let new_len = v.len() + rhs.len();
reserve(&mut *v, new_len);

for uint::range(0u, rhs.len()) |i| {
push(&mut *v, unsafe { raw::get(rhs, i) })
Expand All @@ -638,7 +640,8 @@ pub fn push_all<T: Copy>(v: &mut ~[T], rhs: &[const T]) {

#[inline(always)]
pub fn push_all_move<T>(v: &mut ~[T], mut rhs: ~[T]) {
reserve(&mut *v, v.len() + rhs.len());
let new_len = v.len() + rhs.len();
reserve(&mut *v, new_len);
unsafe {
do as_mut_buf(rhs) |p, len| {
for uint::range(0, len) |i| {
Expand All @@ -663,9 +666,9 @@ pub fn truncate<T>(v: &mut ~[T], newlen: uint) {
let mut dropped = rusti::init();
dropped <-> *ptr::mut_offset(p, i);
}
raw::set_len(&mut *v, newlen);
}
}
unsafe { raw::set_len(&mut *v, newlen); }
}

/**
Expand Down Expand Up @@ -740,7 +743,8 @@ pub pure fn append_mut<T: Copy>(lhs: ~[mut T], rhs: &[const T]) -> ~[mut T] {
* * initval - The value for the new elements
*/
pub fn grow<T: Copy>(v: &mut ~[T], n: uint, initval: &T) {
reserve_at_least(&mut *v, v.len() + n);
let new_len = v.len() + n;
reserve_at_least(&mut *v, new_len);
let mut i: uint = 0u;

while i < n {
Expand All @@ -763,7 +767,8 @@ pub fn grow<T: Copy>(v: &mut ~[T], n: uint, initval: &T) {
* value
*/
pub fn grow_fn<T>(v: &mut ~[T], n: uint, op: iter::InitOp<T>) {
reserve_at_least(&mut *v, v.len() + n);
let new_len = v.len() + n;
reserve_at_least(&mut *v, new_len);
let mut i: uint = 0u;
while i < n {
v.push(op(i));
Expand Down
62 changes: 36 additions & 26 deletions src/librustc/middle/borrowck/check_loans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -305,22 +305,31 @@ impl CheckLoanCtxt {
return;
}

match (old_loan.mutbl, new_loan.mutbl) {
(m_const, _) | (_, m_const) | (m_imm, m_imm) => {
/*ok*/
match (old_loan.kind, new_loan.kind) {
(PartialFreeze, PartialTake) | (PartialTake, PartialFreeze) |
(TotalFreeze, PartialFreeze) | (PartialFreeze, TotalFreeze) |
(Immobile, _) | (_, Immobile) |
(PartialFreeze, PartialFreeze) |
(PartialTake, PartialTake) |
(TotalFreeze, TotalFreeze) => {
/* ok */
}

(m_mutbl, m_mutbl) | (m_mutbl, m_imm) | (m_imm, m_mutbl) => {
(PartialTake, TotalFreeze) | (TotalFreeze, PartialTake) |
(TotalTake, TotalFreeze) | (TotalFreeze, TotalTake) |
(TotalTake, PartialFreeze) | (PartialFreeze, TotalTake) |
(TotalTake, PartialTake) | (PartialTake, TotalTake) |
(TotalTake, TotalTake) => {
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.loan_kind_to_str(new_loan.kind)));
self.bccx.span_note(
old_loan.cmt.span,
fmt!("prior loan as %s granted here",
self.bccx.mut_to_str(old_loan.mutbl)));
self.bccx.loan_kind_to_str(old_loan.kind)));
}
}
}
Expand Down Expand Up @@ -348,13 +357,13 @@ impl CheckLoanCtxt {
// are only assigned once
} else {
match cmt.mutbl {
m_mutbl => { /*ok*/ }
m_const | m_imm => {
self.bccx.span_err(
ex.span,
at.ing_form(self.bccx.cmt_to_str(cmt)));
return;
}
McDeclared | McInherited => { /*ok*/ }
McReadOnly | McImmutable => {
self.bccx.span_err(
ex.span,
at.ing_form(self.bccx.cmt_to_str(cmt)));
return;
}
}
}

Expand Down Expand Up @@ -428,19 +437,20 @@ impl CheckLoanCtxt {
cmt: cmt,
lp: @loan_path) {
for self.walk_loans_of(ex.id, lp) |loan| {
match loan.mutbl {
m_const => { /*ok*/ }
m_mutbl | m_imm => {
self.bccx.span_err(
ex.span,
fmt!("%s prohibited due to outstanding loan",
at.ing_form(self.bccx.cmt_to_str(cmt))));
self.bccx.span_note(
loan.cmt.span,
fmt!("loan of %s granted here",
self.bccx.cmt_to_str(loan.cmt)));
return;
}
match loan.kind {
Immobile => { /* ok */ }
TotalFreeze | PartialFreeze |
TotalTake | PartialTake => {
self.bccx.span_err(
ex.span,
fmt!("%s prohibited due to outstanding loan",
at.ing_form(self.bccx.cmt_to_str(cmt))));
self.bccx.span_note(
loan.cmt.span,
fmt!("loan of %s granted here",
self.bccx.cmt_to_str(loan.cmt)));
return;
}
}
}

Expand Down
68 changes: 43 additions & 25 deletions src/librustc/middle/borrowck/gather_loans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ use core::prelude::*;

use middle::borrowck::preserve::{PreserveCondition, PcOk, PcIfPure};
use middle::borrowck::{Loan, bckerr, bckres, BorrowckCtxt, err_mutbl};
use middle::borrowck::{LoanKind, TotalFreeze, PartialFreeze,
TotalTake, PartialTake, Immobile};
use middle::borrowck::{req_maps};
use middle::borrowck::loan;
use middle::mem_categorization::{cat_binding, cat_discr, cmt, comp_variant};
use middle::mem_categorization::{mem_categorization_ctxt};
use middle::mem_categorization::{opt_deref_kind};
Expand Down Expand Up @@ -340,13 +343,22 @@ impl GatherLoanCtxt {
fn guarantee_valid(@mut self,
cmt: cmt,
req_mutbl: ast::mutability,
scope_r: ty::Region) {
scope_r: ty::Region)
{

let loan_kind = match req_mutbl {
m_mutbl => TotalTake,
m_imm => TotalFreeze,
m_const => Immobile
};

self.bccx.stats.guaranteed_paths += 1;

debug!("guarantee_valid(cmt=%s, req_mutbl=%s, scope_r=%s)",
debug!("guarantee_valid(cmt=%s, req_mutbl=%?, \
loan_kind=%?, scope_r=%s)",
self.bccx.cmt_to_repr(cmt),
self.bccx.mut_to_str(req_mutbl),
req_mutbl,
loan_kind,
region_to_str(self.tcx(), scope_r));
let _i = indenter();

Expand All @@ -362,10 +374,10 @@ impl GatherLoanCtxt {
// 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) {
match loan::loan(self.bccx, cmt, scope_r, loan_kind) {
Err(ref e) => { self.bccx.report((*e)); }
Ok(move loans) => {
self.add_loans(cmt, req_mutbl, scope_r, move loans);
self.add_loans(cmt, loan_kind, scope_r, move loans);
}
}
}
Expand All @@ -378,7 +390,7 @@ impl GatherLoanCtxt {
// pointer is desired, that is ok as long as we are pure)
None => {
let result: bckres<PreserveCondition> = {
do self.check_mutbl(req_mutbl, cmt).chain |pc1| {
do self.check_mutbl(loan_kind, cmt).chain |pc1| {
do self.bccx.preserve(cmt, scope_r,
self.item_ub,
self.root_ub).chain |pc2| {
Expand Down Expand Up @@ -446,37 +458,41 @@ impl GatherLoanCtxt {
// reqires an immutable pointer, but `f` lives in (aliased)
// mutable memory.
fn check_mutbl(@mut self,
req_mutbl: ast::mutability,
loan_kind: LoanKind,
cmt: cmt)
-> bckres<PreserveCondition> {
debug!("check_mutbl(req_mutbl=%?, cmt.mutbl=%?)",
req_mutbl, cmt.mutbl);
debug!("check_mutbl(loan_kind=%?, cmt.mutbl=%?)",
loan_kind, cmt.mutbl);

if req_mutbl == m_const || req_mutbl == cmt.mutbl {
debug!("required is const or they are the same");
Ok(PcOk)
} else {
let e = bckerr { cmt: cmt, code: err_mutbl(req_mutbl) };
if req_mutbl == m_imm {
// if this is an @mut box, then it's generally OK to borrow as
// &imm; this will result in a write guard
if cmt.cat.is_mutable_box() {
match loan_kind {
Immobile => Ok(PcOk),

TotalTake | PartialTake => {
if cmt.mutbl.is_mutable() {
Ok(PcOk)
} else {
// you can treat mutable things as imm if you are pure
debug!("imm required, must be pure");
Err(bckerr { cmt: cmt, code: err_mutbl(loan_kind) })
}
}

TotalFreeze | PartialFreeze => {
if cmt.mutbl.is_immutable() {
Ok(PcOk)
} else if cmt.cat.is_mutable_box() {
Ok(PcOk)
} else {
// Eventually:
let e = bckerr {cmt: cmt,
code: err_mutbl(loan_kind)};
Ok(PcIfPure(e))
}
} else {
Err(e)
}
}
}

fn add_loans(@mut self,
cmt: cmt,
req_mutbl: ast::mutability,
loan_kind: LoanKind,
scope_r: ty::Region,
+loans: ~[Loan]) {
if loans.len() == 0 {
Expand Down Expand Up @@ -526,7 +542,7 @@ impl GatherLoanCtxt {

self.add_loans_to_scope_id(scope_id, move loans);

if req_mutbl == m_imm && cmt.mutbl != m_imm {
if loan_kind.is_freeze() && !cmt.mutbl.is_immutable() {
self.bccx.stats.loaned_paths_imm += 1;

if self.tcx().sess.borrowck_note_loan() {
Expand All @@ -542,7 +558,9 @@ impl GatherLoanCtxt {
fn add_loans_to_scope_id(@mut 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 %?: %s",
loans.len(), scope_id,
str::connect(loans.map(|l| self.bccx.loan_to_repr(l)), ", "));
match self.req_maps.req_loan_map.find(&scope_id) {
Some(req_loans) => {
req_loans.push_all(loans);
Expand Down
Loading