diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index 899765a1d2daa..7617d3b997d08 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -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); } } } diff --git a/src/test/run-pass/borrowck/issue-62007-assign-box.rs b/src/test/run-pass/borrowck/issue-62007-assign-box.rs new file mode 100644 index 0000000000000..f6fbea821b521 --- /dev/null +++ b/src/test/run-pass/borrowck/issue-62007-assign-box.rs @@ -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 { + value: T, + next: Option>>, +} + +fn to_refs(mut list: Box<&mut List>) -> 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() {} diff --git a/src/test/run-pass/borrowck/issue-62007-assign-field.rs b/src/test/run-pass/borrowck/issue-62007-assign-field.rs new file mode 100644 index 0000000000000..5b21c083816a4 --- /dev/null +++ b/src/test/run-pass/borrowck/issue-62007-assign-field.rs @@ -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 { + value: T, + next: Option>>, +} + +fn to_refs(mut list: (&mut List,)) -> 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() {} diff --git a/src/test/ui/nll/issue-62007-assign-const-index.rs b/src/test/ui/nll/issue-62007-assign-const-index.rs new file mode 100644 index 0000000000000..3ea5d3a7ad007 --- /dev/null +++ b/src/test/ui/nll/issue-62007-assign-const-index.rs @@ -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 { + value: T, + next: Option>>, +} + +fn to_refs(mut list: [&mut List; 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() {} diff --git a/src/test/ui/nll/issue-62007-assign-const-index.stderr b/src/test/ui/nll/issue-62007-assign-const-index.stderr new file mode 100644 index 0000000000000..758a14d017705 --- /dev/null +++ b/src/test/ui/nll/issue-62007-assign-const-index.stderr @@ -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(mut list: [&mut List; 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(mut list: [&mut List; 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`. diff --git a/src/test/ui/nll/issue-62007-assign-differing-fields.rs b/src/test/ui/nll/issue-62007-assign-differing-fields.rs new file mode 100644 index 0000000000000..29d92b7b85c12 --- /dev/null +++ b/src/test/ui/nll/issue-62007-assign-differing-fields.rs @@ -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 { + value: T, + next: Option>>, +} + +fn to_refs<'a, T>(mut list: (&'a mut List, &'a mut List)) -> 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() {} diff --git a/src/test/ui/nll/issue-62007-assign-differing-fields.stderr b/src/test/ui/nll/issue-62007-assign-differing-fields.stderr new file mode 100644 index 0000000000000..f942d7628b507 --- /dev/null +++ b/src/test/ui/nll/issue-62007-assign-differing-fields.stderr @@ -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, &'a mut List)) -> 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, &'a mut List)) -> 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`.