mirror of https://github.com/rust-lang/rust.git
Auto merge of #62010 - ecstatic-morse:kill-borrows-of-proj, r=pnkfelix
Kill conflicting borrows of places with projections. Resolves #62007. Due to a bug, the previous version of this check did not actually kill all conflicting borrows unless the borrowed place had no projections. Specifically, `sets.on_entry` will always be empty when `statement_effect` is called. It does not contain the set of borrows which are live at this point in the program. @pnkfelix describes why this was not caught before in #62007, and created an example where the current borrow checker failed unnecessarily. This PR adds their example as a test, but they will likely want to add some additional ones. r? @pnkfelix
This commit is contained in:
commit
305930cffe
|
@ -193,43 +193,38 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> {
|
|||
place: &Place<'tcx>
|
||||
) {
|
||||
debug!("kill_borrows_on_place: place={:?}", place);
|
||||
// Handle the `Place::Local(..)` case first and exit early.
|
||||
if let Place::Base(PlaceBase::Local(local)) = place {
|
||||
if let Some(borrow_indices) = self.borrow_set.local_map.get(&local) {
|
||||
debug!("kill_borrows_on_place: borrow_indices={:?}", borrow_indices);
|
||||
sets.kill_all(borrow_indices);
|
||||
|
||||
if let Some(local) = place.base_local() {
|
||||
let other_borrows_of_local = self
|
||||
.borrow_set
|
||||
.local_map
|
||||
.get(&local)
|
||||
.into_iter()
|
||||
.flat_map(|bs| bs.into_iter());
|
||||
|
||||
// If the borrowed place is a local with no projections, all other borrows of this
|
||||
// local must conflict. This is purely an optimization so we don't have to call
|
||||
// `places_conflict` for every borrow.
|
||||
if let Place::Base(PlaceBase::Local(_)) = place {
|
||||
sets.kill_all(other_borrows_of_local);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// Otherwise, look at all borrows that are live and if they conflict with the assignment
|
||||
// into our place then we can kill them.
|
||||
let mut borrows = sets.on_entry.clone();
|
||||
let _ = borrows.union(sets.gen_set);
|
||||
for borrow_index in borrows.iter() {
|
||||
let borrow_data = &self.borrows()[borrow_index];
|
||||
debug!(
|
||||
"kill_borrows_on_place: borrow_index={:?} borrow_data={:?}",
|
||||
borrow_index, borrow_data,
|
||||
);
|
||||
|
||||
// By passing `PlaceConflictBias::NoOverlap`, we conservatively assume that any given
|
||||
// pair of array indices are unequal, so that when `places_conflict` returns true, we
|
||||
// will be assured that two places being compared definitely denotes the same sets of
|
||||
// locations.
|
||||
if places_conflict::places_conflict(
|
||||
self.tcx,
|
||||
self.body,
|
||||
&borrow_data.borrowed_place,
|
||||
place,
|
||||
places_conflict::PlaceConflictBias::NoOverlap,
|
||||
) {
|
||||
debug!(
|
||||
"kill_borrows_on_place: (kill) borrow_index={:?} borrow_data={:?}",
|
||||
borrow_index, borrow_data,
|
||||
);
|
||||
sets.kill(borrow_index);
|
||||
}
|
||||
let definitely_conflicting_borrows = other_borrows_of_local
|
||||
.filter(|&&i| {
|
||||
places_conflict::places_conflict(
|
||||
self.tcx,
|
||||
self.body,
|
||||
&self.borrow_set.borrows[i].borrowed_place,
|
||||
place,
|
||||
places_conflict::PlaceConflictBias::NoOverlap)
|
||||
});
|
||||
|
||||
sets.kill_all(definitely_conflicting_borrows);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -0,0 +1,27 @@
|
|||
// run-pass
|
||||
|
||||
// Issue #62007: assigning over a deref projection of a box (in this
|
||||
// case, `*list = n;`) should be able to kill all borrows of `*list`,
|
||||
// so that `*list` can be borrowed on the next iteration through the
|
||||
// loop.
|
||||
|
||||
#![allow(dead_code)]
|
||||
|
||||
struct List<T> {
|
||||
value: T,
|
||||
next: Option<Box<List<T>>>,
|
||||
}
|
||||
|
||||
fn to_refs<T>(mut list: Box<&mut List<T>>) -> Vec<&mut T> {
|
||||
let mut result = vec![];
|
||||
loop {
|
||||
result.push(&mut list.value);
|
||||
if let Some(n) = list.next.as_mut() {
|
||||
*list = n;
|
||||
} else {
|
||||
return result;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
|
@ -0,0 +1,26 @@
|
|||
// run-pass
|
||||
|
||||
// Issue #62007: assigning over a field projection (`list.0 = n;` in
|
||||
// this case) should be able to kill all borrows of `list.0`, so that
|
||||
// `list.0` can be borrowed on the next iteration through the loop.
|
||||
|
||||
#![allow(dead_code)]
|
||||
|
||||
struct List<T> {
|
||||
value: T,
|
||||
next: Option<Box<List<T>>>,
|
||||
}
|
||||
|
||||
fn to_refs<T>(mut list: (&mut List<T>,)) -> Vec<&mut T> {
|
||||
let mut result = vec![];
|
||||
loop {
|
||||
result.push(&mut (list.0).value);
|
||||
if let Some(n) = (list.0).next.as_mut() {
|
||||
list.0 = n;
|
||||
} else {
|
||||
return result;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
|
@ -0,0 +1,32 @@
|
|||
// Issue #62007: assigning over a const-index projection of an array
|
||||
// (in this case, `list[I] = n;`) should in theory be able to kill all borrows
|
||||
// of `list[0]`, so that `list[0]` could be borrowed on the next
|
||||
// iteration through the loop.
|
||||
//
|
||||
// Currently the compiler does not allow this. We may want to consider
|
||||
// loosening that restriction in the future. (However, doing so would
|
||||
// at *least* require T-lang team approval, and probably an RFC; e.g.
|
||||
// such loosening might make complicate the user's mental mode; it
|
||||
// also would make code more brittle in the face of refactorings that
|
||||
// replace constants with variables.
|
||||
|
||||
#![allow(dead_code)]
|
||||
|
||||
struct List<T> {
|
||||
value: T,
|
||||
next: Option<Box<List<T>>>,
|
||||
}
|
||||
|
||||
fn to_refs<T>(mut list: [&mut List<T>; 2]) -> Vec<&mut T> {
|
||||
let mut result = vec![];
|
||||
loop {
|
||||
result.push(&mut list[0].value); //~ ERROR cannot borrow `list[_].value` as mutable
|
||||
if let Some(n) = list[0].next.as_mut() { //~ ERROR cannot borrow `list[_].next` as mutable
|
||||
list[0] = n;
|
||||
} else {
|
||||
return result;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
|
@ -0,0 +1,27 @@
|
|||
error[E0499]: cannot borrow `list[_].value` as mutable more than once at a time
|
||||
--> $DIR/issue-62007-assign-const-index.rs:23:21
|
||||
|
|
||||
LL | fn to_refs<T>(mut list: [&mut List<T>; 2]) -> Vec<&mut T> {
|
||||
| - let's call the lifetime of this reference `'1`
|
||||
...
|
||||
LL | result.push(&mut list[0].value);
|
||||
| ^^^^^^^^^^^^^^^^^^ mutable borrow starts here in previous iteration of loop
|
||||
...
|
||||
LL | return result;
|
||||
| ------ returning this value requires that `list[_].value` is borrowed for `'1`
|
||||
|
||||
error[E0499]: cannot borrow `list[_].next` as mutable more than once at a time
|
||||
--> $DIR/issue-62007-assign-const-index.rs:24:26
|
||||
|
|
||||
LL | fn to_refs<T>(mut list: [&mut List<T>; 2]) -> Vec<&mut T> {
|
||||
| - let's call the lifetime of this reference `'1`
|
||||
...
|
||||
LL | if let Some(n) = list[0].next.as_mut() {
|
||||
| ^^^^^^^^^^^^---------
|
||||
| |
|
||||
| mutable borrow starts here in previous iteration of loop
|
||||
| argument requires that `list[_].next` is borrowed for `'1`
|
||||
|
||||
error: aborting due to 2 previous errors
|
||||
|
||||
For more information about this error, try `rustc --explain E0499`.
|
|
@ -0,0 +1,25 @@
|
|||
// Double-check we didn't go too far with our resolution to issue
|
||||
// #62007: assigning over a field projection (`list.1 = n;` in this
|
||||
// case) should kill only borrows of `list.1`; `list.0` can *not*
|
||||
// necessarily be borrowed on the next iteration through the loop.
|
||||
|
||||
#![allow(dead_code)]
|
||||
|
||||
struct List<T> {
|
||||
value: T,
|
||||
next: Option<Box<List<T>>>,
|
||||
}
|
||||
|
||||
fn to_refs<'a, T>(mut list: (&'a mut List<T>, &'a mut List<T>)) -> Vec<&'a mut T> {
|
||||
let mut result = vec![];
|
||||
loop {
|
||||
result.push(&mut (list.0).value); //~ ERROR cannot borrow `list.0.value` as mutable
|
||||
if let Some(n) = (list.0).next.as_mut() { //~ ERROR cannot borrow `list.0.next` as mutable
|
||||
list.1 = n;
|
||||
} else {
|
||||
return result;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
|
@ -0,0 +1,27 @@
|
|||
error[E0499]: cannot borrow `list.0.value` as mutable more than once at a time
|
||||
--> $DIR/issue-62007-assign-differing-fields.rs:16:21
|
||||
|
|
||||
LL | fn to_refs<'a, T>(mut list: (&'a mut List<T>, &'a mut List<T>)) -> Vec<&'a mut T> {
|
||||
| -- lifetime `'a` defined here
|
||||
...
|
||||
LL | result.push(&mut (list.0).value);
|
||||
| ^^^^^^^^^^^^^^^^^^^ mutable borrow starts here in previous iteration of loop
|
||||
...
|
||||
LL | return result;
|
||||
| ------ returning this value requires that `list.0.value` is borrowed for `'a`
|
||||
|
||||
error[E0499]: cannot borrow `list.0.next` as mutable more than once at a time
|
||||
--> $DIR/issue-62007-assign-differing-fields.rs:17:26
|
||||
|
|
||||
LL | fn to_refs<'a, T>(mut list: (&'a mut List<T>, &'a mut List<T>)) -> Vec<&'a mut T> {
|
||||
| -- lifetime `'a` defined here
|
||||
...
|
||||
LL | if let Some(n) = (list.0).next.as_mut() {
|
||||
| ^^^^^^^^^^^^^---------
|
||||
| |
|
||||
| mutable borrow starts here in previous iteration of loop
|
||||
| argument requires that `list.0.next` is borrowed for `'a`
|
||||
|
||||
error: aborting due to 2 previous errors
|
||||
|
||||
For more information about this error, try `rustc --explain E0499`.
|
Loading…
Reference in New Issue