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

Implement stronger guarantees for mutable borrows #14739

Merged
merged 9 commits into from
Jun 14, 2014
3 changes: 2 additions & 1 deletion src/libarena/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,8 @@ impl<T> TypedArenaChunk<T> {
None => {}
Some(mut next) => {
// We assume that the next chunk is completely filled.
next.destroy(next.capacity)
let capacity = next.capacity;
next.destroy(capacity)
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/libcollections/ringbuf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ impl<T> Deque<T> for RingBuf<T> {

/// Return a mutable reference to the last element in the RingBuf
fn back_mut<'a>(&'a mut self) -> Option<&'a mut T> {
if self.nelts > 0 { Some(self.get_mut(self.nelts - 1)) } else { None }
let nelts = self.nelts;
if nelts > 0 { Some(self.get_mut(nelts - 1)) } else { None }
}

/// Remove and return the first element in the RingBuf, or None if it is empty
Expand Down
11 changes: 7 additions & 4 deletions src/libcollections/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ impl<T> Vec<T> {
unsafe {
let mut xs = Vec::with_capacity(length);
while xs.len < length {
ptr::write(xs.as_mut_slice().unsafe_mut_ref(xs.len), op(xs.len));
let len = xs.len;
ptr::write(xs.as_mut_slice().unsafe_mut_ref(len), op(len));
xs.len += 1;
}
xs
Expand Down Expand Up @@ -210,7 +211,8 @@ impl<T: Clone> Vec<T> {
unsafe {
let mut xs = Vec::with_capacity(length);
while xs.len < length {
ptr::write(xs.as_mut_slice().unsafe_mut_ref(xs.len),
let len = xs.len;
ptr::write(xs.as_mut_slice().unsafe_mut_ref(len),
value.clone());
xs.len += 1;
}
Expand Down Expand Up @@ -321,9 +323,10 @@ impl<T:Clone> Clone for Vec<T> {
let this_slice = self.as_slice();
while vector.len < len {
unsafe {
let len = vector.len;
ptr::write(
vector.as_mut_slice().unsafe_mut_ref(vector.len),
this_slice.unsafe_ref(vector.len).clone());
vector.as_mut_slice().unsafe_mut_ref(len),
this_slice.unsafe_ref(len).clone());
}
vector.len += 1;
}
Expand Down
6 changes: 4 additions & 2 deletions src/libdebug/repr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,13 +127,15 @@ impl<'a> ReprVisitor<'a> {
#[inline]
pub fn get<T>(&mut self, f: |&mut ReprVisitor, &T| -> bool) -> bool {
unsafe {
f(self, mem::transmute::<*u8,&T>(self.ptr))
let ptr = self.ptr;
f(self, mem::transmute::<*u8,&T>(ptr))
}
}

#[inline]
pub fn visit_inner(&mut self, inner: *TyDesc) -> bool {
self.visit_ptr_inner(self.ptr, inner)
let ptr = self.ptr;
self.visit_ptr_inner(ptr, inner)
}

#[inline]
Expand Down
5 changes: 2 additions & 3 deletions src/libnative/io/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -637,16 +637,15 @@ impl rtio::RtioUdpSocket for UdpSocket {
mem::size_of::<libc::sockaddr_storage>() as libc::socklen_t;

let dolock = || self.lock_nonblocking();
let doread = |nb| unsafe {
let n = try!(read(fd, self.read_deadline, dolock, |nb| unsafe {
let flags = if nb {c::MSG_DONTWAIT} else {0};
libc::recvfrom(fd,
buf.as_mut_ptr() as *mut libc::c_void,
buf.len() as msglen_t,
flags,
storagep,
&mut addrlen) as libc::c_int
};
let n = try!(read(fd, self.read_deadline, dolock, doread));
}));
sockaddr_to_addr(&storage, addrlen as uint).and_then(|addr| {
Ok((n as uint, addr))
})
Expand Down
9 changes: 5 additions & 4 deletions src/libregex/parse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,18 +345,19 @@ impl<'a> Parser<'a> {
}

fn push_literal(&mut self, c: char) -> Result<(), Error> {
let flags = self.flags;
match c {
'.' => {
self.push(Dot(self.flags))
self.push(Dot(flags))
}
'^' => {
self.push(Begin(self.flags))
self.push(Begin(flags))
}
'$' => {
self.push(End(self.flags))
self.push(End(flags))
}
_ => {
self.push(Literal(c, self.flags))
self.push(Literal(c, flags))
}
}
Ok(())
Expand Down
194 changes: 128 additions & 66 deletions src/librustc/middle/borrowck/check_loans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,9 @@ pub fn check_loans(bccx: &BorrowckCtxt,
}

#[deriving(PartialEq)]
enum MoveError {
MoveOk,
MoveWhileBorrowed(/*loan*/Rc<LoanPath>, /*loan*/Span)
enum UseError {
UseOk,
UseWhileBorrowed(/*loan*/Rc<LoanPath>, /*loan*/Span)
}

impl<'a> CheckLoanCtxt<'a> {
Expand Down Expand Up @@ -438,8 +438,7 @@ impl<'a> CheckLoanCtxt<'a> {
Some(lp) => {
let moved_value_use_kind = match mode {
euv::Copy => {
// FIXME(#12624) -- If we are copying the value,
// we don't care if it's borrowed.
self.check_for_copy_of_frozen_path(id, span, &*lp);
MovedInUse
}
euv::Move(_) => {
Expand All @@ -454,7 +453,7 @@ impl<'a> CheckLoanCtxt<'a> {
}
Some(move_kind) => {
self.check_for_move_of_borrowed_path(id, span,
&lp, move_kind);
&*lp, move_kind);
if move_kind == move_data::Captured {
MovedInCapture
} else {
Expand All @@ -471,23 +470,47 @@ impl<'a> CheckLoanCtxt<'a> {
}
}

fn check_for_copy_of_frozen_path(&self,
id: ast::NodeId,
span: Span,
copy_path: &LoanPath) {
match self.analyze_restrictions_on_use(id, copy_path, ty::ImmBorrow) {
UseOk => { }
UseWhileBorrowed(loan_path, loan_span) => {
self.bccx.span_err(
span,
format!("cannot use `{}` because it was mutably borrowed",
self.bccx.loan_path_to_str(copy_path).as_slice())
.as_slice());
self.bccx.span_note(
loan_span,
format!("borrow of `{}` occurs here",
self.bccx.loan_path_to_str(&*loan_path).as_slice())
.as_slice());
}
}
}

fn check_for_move_of_borrowed_path(&self,
id: ast::NodeId,
span: Span,
move_path: &Rc<LoanPath>,
move_path: &LoanPath,
move_kind: move_data::MoveKind) {
match self.analyze_move_out_from(id, &**move_path) {
MoveOk => { }
MoveWhileBorrowed(loan_path, loan_span) => {
// We want to detect if there are any loans at all, so we search for
// any loans incompatible with MutBorrrow, since all other kinds of
// loans are incompatible with that.
match self.analyze_restrictions_on_use(id, move_path, ty::MutBorrow) {
UseOk => { }
UseWhileBorrowed(loan_path, loan_span) => {
let err_message = match move_kind {
move_data::Captured =>
format!("cannot move `{}` into closure because it is borrowed",
self.bccx.loan_path_to_str(&**move_path).as_slice()),
self.bccx.loan_path_to_str(move_path).as_slice()),
move_data::Declared |
move_data::MoveExpr |
move_data::MovePat =>
format!("cannot move out of `{}` because it is borrowed",
self.bccx.loan_path_to_str(&**move_path).as_slice())
self.bccx.loan_path_to_str(move_path).as_slice())
};

self.bccx.span_err(span, err_message.as_slice());
Expand All @@ -500,6 +523,99 @@ impl<'a> CheckLoanCtxt<'a> {
}
}

pub fn analyze_restrictions_on_use(&self,
expr_id: ast::NodeId,
use_path: &LoanPath,
borrow_kind: ty::BorrowKind)
-> UseError {
debug!("analyze_restrictions_on_use(expr_id={:?}, use_path={})",
self.tcx().map.node_to_str(expr_id),
use_path.repr(self.tcx()));

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) {
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,
id: ast::NodeId,
span: Span,
use_kind: MovedValueUseKind,
lp: &Rc<LoanPath>) {
/*!
* Reports an error if `expr` (which should be a path)
* is using a moved/uninitialized value
*/

debug!("check_if_path_is_moved(id={:?}, use_kind={:?}, lp={})",
id, use_kind, lp.repr(self.bccx.tcx));
self.move_data.each_move_of(id, lp, |move, moved_lp| {
self.bccx.report_use_of_moved_value(
span,
use_kind,
&**lp,
move,
moved_lp);
false
});
}

fn check_if_assigned_path_is_moved(&self,
id: ast::NodeId,
span: Span,
Expand Down Expand Up @@ -541,29 +657,6 @@ impl<'a> CheckLoanCtxt<'a> {
}
}

fn check_if_path_is_moved(&self,
id: ast::NodeId,
span: Span,
use_kind: MovedValueUseKind,
lp: &Rc<LoanPath>) {
/*!
* Reports an error if `expr` (which should be a path)
* is using a moved/uninitialized value
*/

debug!("check_if_path_is_moved(id={:?}, use_kind={:?}, lp={})",
id, use_kind, lp.repr(self.bccx.tcx));
self.move_data.each_move_of(id, lp, |move, moved_lp| {
self.bccx.report_use_of_moved_value(
span,
use_kind,
&**lp,
move,
moved_lp);
false
});
}

fn check_assignment(&self,
assignment_id: ast::NodeId,
assignment_span: Span,
Expand Down Expand Up @@ -862,35 +955,4 @@ impl<'a> CheckLoanCtxt<'a> {
format!("borrow of `{}` occurs here",
self.bccx.loan_path_to_str(loan_path)).as_slice());
}

pub fn analyze_move_out_from(&self,
expr_id: ast::NodeId,
move_path: &LoanPath)
-> MoveError {
debug!("analyze_move_out_from(expr_id={:?}, move_path={})",
self.tcx().map.node_to_str(expr_id),
move_path.repr(self.tcx()));

// We must check every element of a move path. See
// `borrowck-move-subcomponent.rs` for a test case.

// check for a conflicting loan:
let mut ret = MoveOk;
self.each_in_scope_restriction(expr_id, move_path, |loan, _| {
// Any restriction prevents moves.
ret = MoveWhileBorrowed(loan.loan_path.clone(), loan.span);
false
});

if ret != MoveOk {
return ret
}

match *move_path {
LpVar(_) => MoveOk,
LpExtend(ref subpath, _, _) => {
self.analyze_move_out_from(expr_id, &**subpath)
}
}
}
}
3 changes: 2 additions & 1 deletion src/librustc/middle/dataflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,12 +300,13 @@ impl<'a, O:DataFlowOperator+Clone+'static> DataFlowContext<'a, O> {
}

{
let words_per_id = self.words_per_id;
let mut propcx = PropagationContext {
dfcx: &mut *self,
changed: true
};

let mut temp = Vec::from_elem(self.words_per_id, 0u);
let mut temp = Vec::from_elem(words_per_id, 0u);
let mut loop_scopes = Vec::new();

while propcx.changed {
Expand Down
Loading