WIP: implement borrowck

Author: frank-king

compiler/rustc_borrowck/src/borrow_set.rs

@@ -77,6 +77,12 @@ pub(crate) enum Pinnedness<'tcx> {
     Pinned { to: mir::Place<'tcx>, at: Location },
 }
 
+impl<'tcx> Pinnedness<'tcx> {
+    pub(crate) fn is_pinned(&self) -> bool {
+        matches!(self, Self::Pinned { .. })
+    }
+}
+
 #[derive(Debug, Clone)]
 pub struct BorrowData<'tcx> {
     /// Location where the borrow reservation starts.

compiler/rustc_borrowck/src/dataflow.rs

@@ -696,21 +696,6 @@ impl<'a, 'tcx> Pins<'a, 'tcx> {
 
         state.kill_all(definitely_conflicting_pins);
     }
-
-    /// Kill any pins whose Pin result local is `local`.
-    /// This is used when the Pin value goes out of scope (StorageDead).
-    fn kill_pins_by_pin_local(
-        &self,
-        state: &mut <Self as Analysis<'tcx>>::Domain,
-        local: mir::Local,
-    ) {
-        debug!("kill_pins_by_pin_local: local={:?}", local);
-
-        let pins_of_local =
-            self.pin_set.pin_local_map.get(&local).into_iter().flat_map(|bs| bs.iter()).copied();
-
-        state.kill_all(pins_of_local);
-    }
 }
 
 /// Forward dataflow computation of the set of pins that are in scope at a particular location.
@@ -739,13 +724,15 @@ impl<'tcx> rustc_mir_dataflow::Analysis<'tcx> for Pins<'_, 'tcx> {
     ) {
         // Kill pins early on reassignment/StorageDead so that the visitor
         // (which runs after the early phase) sees the updated pin state.
+        // Note: we do NOT kill pins when the Pin result local goes out of scope
+        // (kill_pins_by_pin_local), because a pinned place stays pinned until
+        // the place itself is reassigned.
         match &statement.kind {
             mir::StatementKind::Assign(box (lhs, _)) => {
                 self.kill_pins_on_place(state, *lhs);
             }
             mir::StatementKind::StorageDead(local) => {
                 self.kill_pins_on_place(state, Place::from(*local));
-                self.kill_pins_by_pin_local(state, *local);
             }
             _ => {}
         }
@@ -784,8 +771,6 @@ impl<'tcx> rustc_mir_dataflow::Analysis<'tcx> for Pins<'_, 'tcx> {
             mir::StatementKind::StorageDead(local) => {
                 // Kill all pins on locals that are going out of scope
                 self.kill_pins_on_place(state, Place::from(*local));
-                // Also kill pins whose Pin result local is going out of scope
-                self.kill_pins_by_pin_local(state, *local);
             }
 
             mir::StatementKind::FakeRead(..)

compiler/rustc_borrowck/src/lib.rs

@@ -1274,6 +1274,23 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
         }
     }
 
+    fn pinned_borrows<'s>(
+        &self,
+        state: &'s BorrowckDomain,
+    ) -> Option<&'s MixedBitSet<BorrowIndex>> {
+        // FIXME(pin_ergonomics): borrowck behaviors depend on a safe trait
+        // which should not contain any safety invariants.
+        // if place
+        //     .ty(self.body, self.infcx.tcx)
+        //     .ty
+        //     .is_unpin(self.infcx.tcx, self.body.typing_env(self.infcx.tcx))
+        // {
+        //     None
+        // } else {
+        Some(&state.pinned_borrows)
+        // }
+    }
+
     #[instrument(level = "debug", skip(self, state))]
     fn check_access_for_conflict(
         &mut self,
@@ -1286,7 +1303,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
         let mut error_reported = false;
 
         let borrows_in_scope = self.borrows_in_scope(location, state);
-        let pinned_borrows = &state.pinned_borrows;
+        let pinned_borrows = self.pinned_borrows(state);
 
         each_borrow_involving_path(
             self,
@@ -1295,7 +1312,8 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
             (sd, place_span.0),
             self.borrow_set,
             |borrow_index| {
-                borrows_in_scope.contains(borrow_index) || pinned_borrows.contains(borrow_index)
+                borrows_in_scope.contains(borrow_index)
+                    || pinned_borrows.is_some_and(|p| p.contains(borrow_index))
             },
             |this, borrow_index, borrow| match (rw, borrow.kind) {
                 // Obviously an activation is compatible with its own
@@ -1333,6 +1351,11 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
                     ControlFlow::Continue(())
                 }
 
+                // Ignore the expired borrow (pinnedness never conflicts with a read)
+                (Read(_), BorrowKind::Mut { .. }) if !borrows_in_scope.contains(borrow_index) => {
+                    ControlFlow::Continue(())
+                }
+
                 (Read(kind), BorrowKind::Mut { .. }) => {
                     // Reading from mere reservations of mutable-borrows is OK.
                     if !is_active(this.dominators(), borrow, location) {
@@ -1356,19 +1379,44 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
                     ControlFlow::Break(())
                 }
 
-                (Reservation(kind) | Activation(kind, _) | Write(kind), _) => {
-                    if pinned_borrows.contains(borrow_index)
-                        && !borrows_in_scope.contains(borrow_index)
-                    {
-                        // drop or write to the place is allowed after pinning
-                        if matches!(
-                            kind,
-                            WriteKind::StorageDeadOrDrop | WriteKind::Mutate | WriteKind::Replace
-                        ) {
-                            return ControlFlow::Continue(());
+                // Handle the expired borrows (only pinned borrows)
+                (Reservation(kind) | Activation(kind, _) | Write(kind), _)
+                    if !borrows_in_scope.contains(borrow_index) =>
+                {
+                    debug_assert!(
+                        pinned_borrows.is_none_or(|p| p.contains(borrow_index)),
+                        "unexpected expired but non-pinned borrow {borrow_index:?}: {borrow:?}",
+                    );
+                    match kind {
+                        // Pinnedness doesn't conflict with a drop or write
+                        WriteKind::StorageDeadOrDrop | WriteKind::Mutate | WriteKind::Replace => {
+                            ControlFlow::Continue(())
+                        }
+                        // Mutable (pinned) borrow doesn't conflict with an expired borrow
+                        WriteKind::MutableBorrow(_)
+                            if self
+                                .borrow_set
+                                .location_map
+                                .get(&location)
+                                .is_none_or(|b| b.pinnedness.is_pinned()) =>
+                        {
+                            ControlFlow::Continue(())
+                        }
+                        // Mutable (but non-pinned) borrow conflicts with an earlier pinned borrow
+                        WriteKind::MutableBorrow(_) => {
+                            this.report_mutably_borrow_after_pinned(location, place_span, borrow);
+                            error_reported = true;
+                            ControlFlow::Break(())
+                        }
+                        WriteKind::Move => {
+                            this.report_move_after_pinned(location, place_span, borrow);
+                            error_reported = true;
+                            ControlFlow::Break(())
                         }
                     }
+                }
 
+                (Reservation(kind) | Activation(kind, _) | Write(kind), _) => {
                     match rw {
                         Reservation(..) => {
                             debug!(
@@ -1391,15 +1439,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
                     error_reported = true;
                     match kind {
                         WriteKind::MutableBorrow(bk) => {
-                            if pinned_borrows.contains(borrow_index) {
-                                this.report_mutably_borrow_after_pinned(
-                                    location, place_span, borrow,
-                                );
-                            } else {
-                                let err = this
-                                    .report_conflicting_borrow(location, place_span, bk, borrow);
-                                this.buffer_error(err);
-                            }
+                            let err =
+                                this.report_conflicting_borrow(location, place_span, bk, borrow);
+                            this.buffer_error(err);
                         }
                         WriteKind::StorageDeadOrDrop => this
                             .report_borrowed_value_does_not_live_long_enough(
@@ -1412,11 +1454,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
                             this.report_illegal_mutation_of_borrowed(location, place_span, borrow)
                         }
                         WriteKind::Move => {
-                            if pinned_borrows.contains(borrow_index) {
-                                this.report_move_after_pinned(location, place_span, borrow);
-                            } else {
-                                this.report_move_out_while_borrowed(location, place_span, borrow)
-                            }
+                            this.report_move_out_while_borrowed(location, place_span, borrow)
                         }
                         WriteKind::Replace => {
                             this.report_illegal_mutation_of_borrowed(location, place_span, borrow)

compiler/rustc_borrowck/src/pin_set.rs

@@ -8,10 +8,8 @@ use crate::consumers::BorrowSet;
 use crate::dataflow::BorrowIndex;
 
 pub(crate) struct PinSet {
-    /// Map from Pin result location to the borrows it pinned.
+    /// Map from Pin aggregate location to the borrows it pinned.
     pub(crate) pin_location_map: FxIndexMap<Location, FxIndexSet<BorrowIndex>>,
-    /// Map from Pin result local to all the borrows it pined.
-    pub(crate) pin_local_map: FxIndexMap<mir::Local, FxIndexSet<BorrowIndex>>,
 }
 
 impl PinSet {
@@ -20,19 +18,14 @@ impl PinSet {
         body: &Body<'tcx>,
         borrow_set: &BorrowSet<'tcx>,
     ) -> Self {
-        let mut visitor = GatherPins {
-            tcx,
-            body,
-            borrow_set,
-            pin_location_map: Default::default(),
-            pin_local_map: Default::default(),
-        };
+        let mut visitor =
+            GatherPins { tcx, body, borrow_set, pin_location_map: Default::default() };
 
         for (block, block_data) in traversal::preorder(body) {
             visitor.visit_basic_block_data(block, block_data);
         }
 
-        PinSet { pin_location_map: visitor.pin_location_map, pin_local_map: visitor.pin_local_map }
+        PinSet { pin_location_map: visitor.pin_location_map }
     }
 }
 
@@ -41,7 +34,6 @@ struct GatherPins<'a, 'tcx> {
     body: &'a Body<'tcx>,
     borrow_set: &'a BorrowSet<'tcx>,
     pin_location_map: FxIndexMap<Location, FxIndexSet<BorrowIndex>>,
-    pin_local_map: FxIndexMap<mir::Local, FxIndexSet<BorrowIndex>>,
 }
 
 impl<'tcx> Visitor<'tcx> for GatherPins<'_, 'tcx> {
@@ -66,8 +58,7 @@ impl<'tcx> Visitor<'tcx> for GatherPins<'_, 'tcx> {
             && let Some((borrow_location, _borrowed_place)) = self.find_original_borrowed_place(location, ref_place)
             && let Some(idx) = self.borrow_set.get_index_of(&borrow_location)
         {
-            self.pin_location_map.entry(borrow_location).or_default().insert(idx);
-            self.pin_local_map.entry(pin_result_place.local).or_default().insert(idx);
+            self.pin_location_map.entry(location).or_default().insert(idx);
         }
 
         self.super_assign(pin_result_place, rvalue, location)