From 169dadcbbec366e30b444182a115ed3600053324 Mon Sep 17 00:00:00 2001 From: Boxy Uwu Date: Fri, 19 Dec 2025 22:08:58 +0000 Subject: [PATCH] two phase borrow handling cleanups --- compiler/rustc_borrowck/src/borrow_set.rs | 103 +++++++----------- compiler/rustc_borrowck/src/lib.rs | 4 +- .../src/polonius/legacy/loan_invalidations.rs | 4 +- .../rustc_borrowck/src/universal_regions.rs | 8 +- .../rustc_const_eval/src/interpret/step.rs | 2 +- compiler/rustc_middle/src/mir/statement.rs | 2 +- 6 files changed, 48 insertions(+), 75 deletions(-) diff --git a/compiler/rustc_borrowck/src/borrow_set.rs b/compiler/rustc_borrowck/src/borrow_set.rs index 303fa469332e..11f6a7d55e12 100644 --- a/compiler/rustc_borrowck/src/borrow_set.rs +++ b/compiler/rustc_borrowck/src/borrow_set.rs @@ -253,19 +253,51 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'tcx> { } let region = region.as_var(); - - let borrow = BorrowData { + let borrow = |activation_location| BorrowData { kind, region, reserve_location: location, - activation_location: TwoPhaseActivation::NotTwoPhase, + activation_location, borrowed_place, assigned_place: *assigned_place, }; - let (idx, _) = self.location_map.insert_full(location, borrow); - let idx = BorrowIndex::from(idx); - self.insert_as_pending_if_two_phase(location, assigned_place, kind, idx); + let idx = if !kind.is_two_phase_borrow() { + debug!(" -> {:?}", location); + let (idx, _) = self + .location_map + .insert_full(location, borrow(TwoPhaseActivation::NotTwoPhase)); + BorrowIndex::from(idx) + } else { + // When we encounter a 2-phase borrow statement, it will always + // be assigning into a temporary TEMP: + // + // TEMP = &foo + // + // so extract `temp`. + let Some(temp) = assigned_place.as_local() else { + span_bug!( + self.body.source_info(location).span, + "expected 2-phase borrow to assign to a local, not `{:?}`", + assigned_place, + ); + }; + + // Consider the borrow not activated to start. When we find an activation, we'll update + // this field. + let (idx, _) = + self.location_map.insert_full(location, borrow(TwoPhaseActivation::NotActivated)); + let idx = BorrowIndex::from(idx); + + // Insert `temp` into the list of pending activations. From + // now on, we'll be on the lookout for a use of it. Note that + // we are guaranteed that this use will come after the + // assignment. + let prev = self.pending_activations.insert(temp, idx); + assert_eq!(prev, None, "temporary associated with multiple two phase borrows"); + + idx + }; self.local_map.entry(borrowed_place.local).or_default().insert(idx); } @@ -334,62 +366,3 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'tcx> { self.super_rvalue(rvalue, location) } } - -impl<'a, 'tcx> GatherBorrows<'a, 'tcx> { - /// If this is a two-phase borrow, then we will record it - /// as "pending" until we find the activating use. - fn insert_as_pending_if_two_phase( - &mut self, - start_location: Location, - assigned_place: &mir::Place<'tcx>, - kind: mir::BorrowKind, - borrow_index: BorrowIndex, - ) { - debug!( - "Borrows::insert_as_pending_if_two_phase({:?}, {:?}, {:?})", - start_location, assigned_place, borrow_index, - ); - - if !kind.allows_two_phase_borrow() { - debug!(" -> {:?}", start_location); - return; - } - - // When we encounter a 2-phase borrow statement, it will always - // be assigning into a temporary TEMP: - // - // TEMP = &foo - // - // so extract `temp`. - let Some(temp) = assigned_place.as_local() else { - span_bug!( - self.body.source_info(start_location).span, - "expected 2-phase borrow to assign to a local, not `{:?}`", - assigned_place, - ); - }; - - // Consider the borrow not activated to start. When we find an activation, we'll update - // this field. - { - let borrow_data = &mut self.location_map[borrow_index.as_usize()]; - borrow_data.activation_location = TwoPhaseActivation::NotActivated; - } - - // Insert `temp` into the list of pending activations. From - // now on, we'll be on the lookout for a use of it. Note that - // we are guaranteed that this use will come after the - // assignment. - let old_value = self.pending_activations.insert(temp, borrow_index); - if let Some(old_index) = old_value { - span_bug!( - self.body.source_info(start_location).span, - "found already pending activation for temp: {:?} \ - at borrow_index: {:?} with associated data {:?}", - temp, - old_index, - self.location_map[old_index.as_usize()] - ); - } - } -} diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 91defbad0a0e..4a059481c326 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -1301,7 +1301,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { (Read(kind), BorrowKind::Mut { .. }) => { // Reading from mere reservations of mutable-borrows is OK. if !is_active(this.dominators(), borrow, location) { - assert!(borrow.kind.allows_two_phase_borrow()); + assert!(borrow.kind.is_two_phase_borrow()); return ControlFlow::Continue(()); } @@ -1464,7 +1464,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> { } BorrowKind::Mut { .. } => { let wk = WriteKind::MutableBorrow(bk); - if bk.allows_two_phase_borrow() { + if bk.is_two_phase_borrow() { (Deep, Reservation(wk)) } else { (Deep, Write(wk)) diff --git a/compiler/rustc_borrowck/src/polonius/legacy/loan_invalidations.rs b/compiler/rustc_borrowck/src/polonius/legacy/loan_invalidations.rs index d2eae2c7e65a..99567da92ffe 100644 --- a/compiler/rustc_borrowck/src/polonius/legacy/loan_invalidations.rs +++ b/compiler/rustc_borrowck/src/polonius/legacy/loan_invalidations.rs @@ -264,7 +264,7 @@ impl<'a, 'tcx> LoanInvalidationsGenerator<'a, 'tcx> { } BorrowKind::Mut { .. } => { let wk = WriteKind::MutableBorrow(bk); - if bk.allows_two_phase_borrow() { + if bk.is_two_phase_borrow() { (Deep, Reservation(wk)) } else { (Deep, Write(wk)) @@ -384,7 +384,7 @@ impl<'a, 'tcx> LoanInvalidationsGenerator<'a, 'tcx> { // Reading from mere reservations of mutable-borrows is OK. if !is_active(this.dominators, borrow, location) { // If the borrow isn't active yet, reads don't invalidate it - assert!(borrow.kind.allows_two_phase_borrow()); + assert!(borrow.kind.is_two_phase_borrow()); return ControlFlow::Continue(()); } diff --git a/compiler/rustc_borrowck/src/universal_regions.rs b/compiler/rustc_borrowck/src/universal_regions.rs index fda4719fc3f5..ea29f5a47ccc 100644 --- a/compiler/rustc_borrowck/src/universal_regions.rs +++ b/compiler/rustc_borrowck/src/universal_regions.rs @@ -207,10 +207,10 @@ struct UniversalRegionIndices<'tcx> { /// `ty::Region` to the internal `RegionVid` we are using. This is /// used because trait matching and type-checking will feed us /// region constraints that reference those regions and we need to - /// be able to map them to our internal `RegionVid`. This is - /// basically equivalent to an `GenericArgs`, except that it also - /// contains an entry for `ReStatic` -- it might be nice to just - /// use an args, and then handle `ReStatic` another way. + /// be able to map them to our internal `RegionVid`. + /// + /// This is similar to just using `GenericArgs`, except that it contains + /// an entry for `'static`, and also late bound parameters in scope. indices: FxIndexMap, RegionVid>, /// The vid assigned to `'static`. Used only for diagnostics. diff --git a/compiler/rustc_const_eval/src/interpret/step.rs b/compiler/rustc_const_eval/src/interpret/step.rs index 051c6c344506..4aa9030cfe61 100644 --- a/compiler/rustc_const_eval/src/interpret/step.rs +++ b/compiler/rustc_const_eval/src/interpret/step.rs @@ -218,7 +218,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { // A fresh reference was created, make sure it gets retagged. let val = M::retag_ptr_value( self, - if borrow_kind.allows_two_phase_borrow() { + if borrow_kind.is_two_phase_borrow() { mir::RetagKind::TwoPhase } else { mir::RetagKind::Default diff --git a/compiler/rustc_middle/src/mir/statement.rs b/compiler/rustc_middle/src/mir/statement.rs index adaac3d7ffc2..393e9c59c355 100644 --- a/compiler/rustc_middle/src/mir/statement.rs +++ b/compiler/rustc_middle/src/mir/statement.rs @@ -842,7 +842,7 @@ impl BorrowKind { /// Returns whether borrows represented by this kind are allowed to be split into separate /// Reservation and Activation phases. - pub fn allows_two_phase_borrow(&self) -> bool { + pub fn is_two_phase_borrow(&self) -> bool { match *self { BorrowKind::Shared | BorrowKind::Fake(_)