mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

use places_conflict to handle reassignment

Browse Source 
This fixes the handling of reassignment of struct fields.
This commit is contained in:
Ariel Ben-Yehuda 2017-12-07 20:48:12 +02:00
parent 97c58ed66c
commit b64ddecae8
3 changed files with 55 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

91
src/librustc_mir/borrow_check/mod.rs
View File

@ -605,8 +605,10 @@ enum WriteKind {
/// - Take flow state into consideration in `is_assignable()` for local variables
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
enum LocalMutationIsAllowed {
Move,
Yes,
/// We want use of immutable upvars to cause a "write to immutable upvar"
/// error, not an "reassignment" error.
ExceptUpvars,
No
}
@ -802,7 +804,12 @@ fn mutate_place(
context,
place_span,
(kind, Write(WriteKind::Mutate)),
LocalMutationIsAllowed::Yes,
// We want immutable upvars to cause an "assignment to immutable var"
// error, not an "reassignment of immutable var" error, because the
// latter can't find a good previous assignment span.
//
// There's probably a better way to do this.
LocalMutationIsAllowed::ExceptUpvars,
flow_state,
);
@ -922,7 +929,7 @@ fn consume_operand(
context,
(place, span),
(Deep, Write(WriteKind::Move)),
LocalMutationIsAllowed::Move,
LocalMutationIsAllowed::Yes,
flow_state,
);
@ -1009,34 +1016,19 @@ fn check_if_reassignment_to_immutable_state(
(place, span): (&Place<'tcx>, Span),
flow_state: &Flows<'cx, 'gcx, 'tcx>,
) {
let move_data = self.move_data;
debug!("check_if_reassignment_to_immutable_state({:?})", place);
// determine if this path has a non-mut owner (and thus needs checking).
if let Ok(()) = self.is_mutable(place, LocalMutationIsAllowed::No) {
return;
}
debug!("check_if_reassignment_to_immutable_state({:?}) - is an imm local", place);
match self.move_path_closest_to(place) {
Ok(mpi) => for ii in &move_data.init_path_map[mpi] {
if flow_state.ever_inits.contains(ii) {
let first_assign_span = self.move_data.inits[*ii].span;
self.report_illegal_reassignment(context, (place, span), first_assign_span);
break;
}
},
Err(NoMovePathFound::ReachedStatic) => {
let item_msg = match self.describe_place(place) {
Some(name) => format!("immutable static item `{}`", name),
None => "immutable static item".to_owned(),
};
self.tcx.sess.delay_span_bug(
span,
&format!(
"cannot assign to {}, should have been caught by \
`check_access_permissions()`",
item_msg
),
);
for i in flow_state.ever_inits.elems_incoming() {
let init = self.move_data.inits[i];
let init_place = &self.move_data.move_paths[init.path].place;
if self.places_conflict(&init_place, place, Deep) {
self.report_illegal_reassignment(context, (place, span), init.span);
break;
}
}
}
@ -1341,7 +1333,7 @@ fn is_mutable<'d>(
match local.mutability {
Mutability::Not => match is_local_mutation_allowed {
LocalMutationIsAllowed::Yes |
LocalMutationIsAllowed::Move => Ok(()),
LocalMutationIsAllowed::ExceptUpvars => Ok(()),
LocalMutationIsAllowed::No => Err(place),
},
Mutability::Mut => Ok(()),
@ -1410,8 +1402,9 @@ fn is_mutable<'d>(
decl, is_local_mutation_allowed, place);
return match (decl.mutability, is_local_mutation_allowed) {
(Mutability::Not, LocalMutationIsAllowed::No) |
(Mutability::Not, LocalMutationIsAllowed::Yes) => Err(place),
(Mutability::Not, LocalMutationIsAllowed::Move) |
(Mutability::Not, LocalMutationIsAllowed::ExceptUpvars)
=> Err(place),
(Mutability::Not, LocalMutationIsAllowed::Yes) |
(Mutability::Mut, _) => self.is_unique(&proj.base),
};
}
@ -1666,13 +1659,17 @@ fn place_element_conflict(&self,
}
}
}
fn borrow_conflicts_with_place(&mut self,
borrow: &BorrowData<'tcx>,
place: &Place<'tcx>,
access: ShallowOrDeep)
-> bool
/// Returns whether an access of kind `access` to `access_place` conflicts with
/// a borrow/full access to `borrow_place` (for deep accesses to mutable
/// locations, this function is symmetric between `borrow_place` & `access_place`).
fn places_conflict(&mut self,
borrow_place: &Place<'tcx>,
access_place: &Place<'tcx>,
access: ShallowOrDeep)
-> bool
{
debug!("borrow_conflicts_with_place({:?},{:?},{:?})", borrow, place, access);
debug!("places_conflict({:?},{:?},{:?})", borrow_place, access_place, access);
// Return all the prefixes of `place` in reverse order, including
// downcasts.
@ -1694,9 +1691,9 @@ fn place_elements<'a, 'tcx>(place: &'a Place<'tcx>) -> Vec<&'a Place<'tcx>>
}
}
let borrow_components = place_elements(&borrow.place);
let access_components = place_elements(place);
debug!("borrow_conflicts_with_place: components {:?} / {:?}",
let borrow_components = place_elements(borrow_place);
let access_components = place_elements(access_place);
debug!("places_conflict: components {:?} / {:?}",
borrow_components, access_components);
let borrow_components = borrow_components.into_iter()
@ -1746,7 +1743,7 @@ fn place_elements<'a, 'tcx>(place: &'a Place<'tcx>) -> Vec<&'a Place<'tcx>>
// - If we did run out of accesss, the borrow can access a part of it.
for (borrow_c, access_c) in borrow_components.zip(access_components) {
// loop invariant: borrow_c is always either equal to access_c or disjoint from it.
debug!("borrow_conflicts_with_place: {:?} vs. {:?}", borrow_c, access_c);
debug!("places_conflict: {:?} vs. {:?}", borrow_c, access_c);
match (borrow_c, access_c) {
(None, _) => {
// If we didn't run out of access, the borrow can access all of our
@ -1759,7 +1756,7 @@ fn place_elements<'a, 'tcx>(place: &'a Place<'tcx>) -> Vec<&'a Place<'tcx>>
//
// FIXME: Differs from AST-borrowck; includes drive-by fix
// to #38899. Will probably need back-compat mode flag.
debug!("borrow_conflict_with_place: full borrow, CONFLICT");
debug!("places_conflict: full borrow, CONFLICT");
return true;
}
(Some(borrow_c), None) => {
@ -1784,7 +1781,7 @@ fn place_elements<'a, 'tcx>(place: &'a Place<'tcx>) -> Vec<&'a Place<'tcx>>
//
// e.g. a (mutable) borrow of `a[5]` while we read the
// array length of `a`.
debug!("borrow_conflicts_with_place: implicit field");
debug!("places_conflict: implicit field");
return false;
}
@ -1792,7 +1789,7 @@ fn place_elements<'a, 'tcx>(place: &'a Place<'tcx>) -> Vec<&'a Place<'tcx>>
// e.g. a borrow of `*x.y` while we shallowly access `x.y` or some
// prefix thereof - the shallow access can't touch anything behind
// the pointer.
debug!("borrow_conflicts_with_place: shallow access behind ptr");
debug!("places_conflict: shallow access behind ptr");
return false;
}
(ProjectionElem::Deref, ty::TyRef(_, ty::TypeAndMut {
@ -1803,7 +1800,7 @@ fn place_elements<'a, 'tcx>(place: &'a Place<'tcx>) -> Vec<&'a Place<'tcx>>
// I'm not sure why we are tracking these borrows - shared
// references can *always* be aliased, which means the
// permission check already account for this borrow.
debug!("borrow_conflicts_with_place: behind a shared ref");
debug!("places_conflict: behind a shared ref");
return false;
}
@ -1836,7 +1833,7 @@ fn place_elements<'a, 'tcx>(place: &'a Place<'tcx>) -> Vec<&'a Place<'tcx>>
// idea, at least for now, so just give up and
// report a conflict. This is unsafe code anyway so
// the user could always use raw pointers.
debug!("borrow_conflicts_with_place: arbitrary -> conflict");
debug!("places_conflict: arbitrary -> conflict");
return true;
}
Overlap::EqualOrDisjoint => {
@ -1845,7 +1842,7 @@ fn place_elements<'a, 'tcx>(place: &'a Place<'tcx>) -> Vec<&'a Place<'tcx>>
Overlap::Disjoint => {
// We have proven the borrow disjoint - further
// projections will remain disjoint.
debug!("borrow_conflicts_with_place: disjoint");
debug!("places_conflict: disjoint");
return false;
}
}
@ -1874,10 +1871,10 @@ fn each_borrow_involving_path<F>(
// check for loan restricting path P being used. Accounts for
// borrows of P, P.a.b, etc.
'next_borrow: for i in flow_state.borrows.elems_incoming() {
for i in flow_state.borrows.elems_incoming() {
let borrowed = &data[i];
if self.borrow_conflicts_with_place(borrowed, place, access) {
if self.places_conflict(&borrowed.place, place, access) {
let ctrl = op(self, i, borrowed);
if ctrl == Control::Break { return; }
}

9
src/test/compile-fail/immut-function-arguments.rs
View File

@ -8,14 +8,17 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// revisions: ast mir
//[mir]compile-flags: -Z borrowck=mir
fn f(y: Box<isize>) {
*y = 5; //~ ERROR cannot assign
*y = 5; //[ast]~ ERROR cannot assign
//[mir]~^ ERROR cannot assign twice
}
fn g() {
let _frob = |q: Box<isize>| { *q = 2; }; //~ ERROR cannot assign
let _frob = |q: Box<isize>| { *q = 2; }; //[ast]~ ERROR cannot assign
//[mir]~^ ERROR cannot assign twice
}
fn main() {}

6
src/test/compile-fail/mutable-class-fields.rs
View File

@ -8,6 +8,9 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// revisions: ast mir
//[mir]compile-flags: -Z borrowck=mir
struct cat {
meows : usize,
how_hungry : isize,
@ -22,5 +25,6 @@ fn cat(in_x : usize, in_y : isize) -> cat {
fn main() {
let nyan : cat = cat(52, 99);
nyan.how_hungry = 0; //~ ERROR cannot assign
nyan.how_hungry = 0; //[ast]~ ERROR cannot assign
//[mir]~^ ERROR cannot assign
}