Finish some refactoring

Author: zgrannan

src/borrow_pcg/action/mod.rs

@@ -1,13 +1,11 @@
 use tracing::instrument;
 
-use super::borrow_pcg_edge::{BorrowPCGEdge, LocalNode, ToBorrowsEdge};
+use super::borrow_pcg_edge::{BorrowPCGEdge, LocalNode};
 use super::borrow_pcg_expansion::BorrowPCGExpansion;
 use super::edge::block::BlockEdge;
 use super::edge::kind::BorrowPCGEdgeKind;
-use super::path_condition::PathConditions;
 use super::state::BorrowsState;
-use crate::borrow_pcg::edge::abstraction::AbstractionType;
-use crate::combined_pcs::{PCGError, PCGNode, PCGNodeLike};
+use crate::combined_pcs::{PCGError, PCGNode};
 use crate::free_pcs::CapabilityKind;
 use crate::rustc_interface::{ast::Mutability, middle::mir::Location};
 use crate::utils::display::{DebugLines, DisplayWithRepacker};
@@ -287,13 +285,7 @@ impl<'tcx> BorrowsState<'tcx> {
             }
             BorrowPCGEdgeKind::Abstraction(_) => changed,
             BorrowPCGEdgeKind::Block(block_edge) => {
-                changed
-                    || self.set_capabilities_for_block_edge(
-                        block_edge,
-                        edge.conditions,
-                        for_exclusive,
-                        repacker,
-                    )
+                changed || self.set_capabilities_for_block_edge(block_edge, for_exclusive, repacker)
             }
         })
     }
@@ -304,7 +296,6 @@ impl<'tcx> BorrowsState<'tcx> {
     fn set_capabilities_for_block_edge(
         &mut self,
         member: BlockEdge<'tcx>,
-        pc: PathConditions,
         for_exclusive: bool,
         repacker: PlaceRepacker<'_, 'tcx>,
     ) -> bool {

src/borrow_pcg/borrow_pcg_capabilities.rs

@@ -5,11 +5,16 @@ use crate::{
     free_pcs::CapabilityKind,
     rustc_interface::data_structures::fx::FxHashMap,
     utils::{
-        display::{DebugLines, DisplayWithRepacker}, maybe_old::MaybeOldPlace, Place, PlaceRepacker
+        display::{DebugLines, DisplayWithRepacker},
+        maybe_old::MaybeOldPlace,
+        Place, PlaceRepacker,
     },
 };
 
-use super::{has_pcs_elem::{HasPcsElems, MakePlaceOld}, latest::Latest};
+use super::{
+    has_pcs_elem::{HasPcsElems, MakePlaceOld},
+    latest::Latest,
+};
 
 /// Tracks the capabilities of places in the borrow PCG. We don't store this
 /// information in the borrows graph directly to facilitate simpler logic for

src/borrow_pcg/borrow_pcg_edge.rs

@@ -1,5 +1,4 @@
 use rustc_interface::{
-    ast::Mutability,
     data_structures::fx::FxHashSet,
     middle::mir::{self, BasicBlock, PlaceElem},
 };
@@ -268,13 +267,6 @@ impl<'tcx> From<CGNode<'tcx>> for PCGNode<'tcx> {
 pub type BlockedNode<'tcx> = PCGNode<'tcx>;
 
 impl<'tcx> PCGNode<'tcx> {
-    pub(crate) fn mutability(&self, repacker: PlaceRepacker<'_, 'tcx>) -> Mutability {
-        match self {
-            PCGNode::Place(rp) => rp.mutability(repacker),
-            PCGNode::RegionProjection(rp) => rp.mutability(repacker),
-        }
-    }
-
     pub(crate) fn as_cg_node(self) -> Option<CGNode<'tcx>> {
         match self {
             PCGNode::Place(MaybeRemotePlace::Remote(remote_place)) => {

src/borrow_pcg/edge/block.rs

@@ -2,8 +2,7 @@ use serde_json::json;
 use smallvec::SmallVec;
 
 use crate::combined_pcs::PCGNode;
-use crate::pcg_validity_assert;
-use crate::rustc_interface::{ast::Mutability, data_structures::fx::FxHashSet};
+use crate::rustc_interface::data_structures::fx::FxHashSet;
 use crate::utils::display::DisplayWithRepacker;
 use crate::utils::place::maybe_old::MaybeOldPlace;
 use crate::utils::validity::HasValidityCheck;
@@ -164,19 +163,6 @@ impl<'tcx> HasPcsElems<MaybeOldPlace<'tcx>> for BlockEdge<'tcx> {
 }
 
 impl<'tcx> BlockEdge<'tcx> {
-    /// Returns `true` iff the lifetime projections are mutable
-    pub(crate) fn mutability(&self, repacker: PlaceRepacker<'_, 'tcx>) -> Mutability {
-        let mut_values = self
-            .inputs
-            .iter()
-            .map(|p| p.mutability(repacker))
-            .collect::<Vec<_>>();
-        pcg_validity_assert!(
-            mut_values.windows(2).all(|w| w[0] == w[1]),
-            "All mutability values must be the same"
-        );
-        mut_values[0]
-    }
 
     pub(crate) fn new(
         inputs: BlockEdgeInputs<'tcx>,

src/borrow_pcg/region_projection.rs

@@ -18,7 +18,6 @@ use crate::validity_checks_enabled;
 use crate::{
     combined_pcs::{LocalNodeLike, PCGNode, PCGNodeLike},
     rustc_interface::{
-        ast::Mutability,
         data_structures::fx::FxHashSet,
         index::{Idx, IndexVec},
         middle::{
@@ -477,28 +476,6 @@ impl<'tcx> RegionProjection<'tcx> {
         self.base.as_local_place().map(|p| p.local())
     }
 
-    /// Returns `true` iff the place is a mutable reference, or if the place is
-    /// a mutable struct. If the place is a remote place, it is mutable iff the
-    /// corresponding input argument is a mutable reference.
-    pub(crate) fn mutability(&self, repacker: PlaceRepacker<'_, 'tcx>) -> Mutability {
-        let place = match self.base {
-            MaybeRemoteRegionProjectionBase::Place(p) => p.related_local_place(),
-            MaybeRemoteRegionProjectionBase::Const(_) => {
-                return Mutability::Not;
-            }
-        };
-        place.ref_mutability(repacker).unwrap_or_else(|| {
-            if let Ok(root_place) =
-                place.is_mutable(crate::utils::LocalMutationIsAllowed::Yes, repacker)
-                && root_place.is_local_mutation_allowed == crate::utils::LocalMutationIsAllowed::Yes
-            {
-                Mutability::Mut
-            } else {
-                Mutability::Not
-            }
-        })
-    }
-
     fn as_local_region_projection(
         &self,
         repacker: PlaceRepacker<'_, 'tcx>,

src/borrow_pcg/state/mod.rs

@@ -5,15 +5,14 @@ use super::{
         BlockedNode, BorrowPCGEdge, BorrowPCGEdgeLike, BorrowPCGEdgeRef, LocalNode, ToBorrowsEdge,
     },
     coupling_graph_constructor::BorrowCheckerInterface,
-    edge::kind::BorrowPCGEdgeKind,
     graph::{BorrowsGraph, FrozenGraphRef},
     latest::Latest,
     path_condition::{PathCondition, PathConditions},
 };
-use crate::borrow_pcg::edge::borrow::BorrowEdge;
 use crate::utils::place::maybe_old::MaybeOldPlace;
 use crate::utils::place::maybe_remote::MaybeRemotePlace;
 use crate::{borrow_pcg::action::executed_actions::ExecutedActions, combined_pcs::PCGError};
+use crate::borrow_pcg::edge::borrow::BorrowEdge;
 use crate::{
     borrow_pcg::edge_data::EdgeData,
     combined_pcs::{PCGNode, PCGNodeLike},
@@ -236,21 +235,6 @@ impl<'tcx> BorrowsState<'tcx> {
         changed
     }
 
-    fn min_blocked_by_capability(
-        &self,
-        edge: &BorrowPCGEdgeKind<'tcx>,
-        repacker: PlaceRepacker<'_, 'tcx>,
-    ) -> Option<CapabilityKind> {
-        let mut iter = edge.blocked_by_nodes(repacker).into_iter();
-        let first_node = iter.next()?;
-        let mut cap = self.get_capability(first_node.into())?;
-        for node in iter {
-            let other_cap = self.get_capability(node.into())?;
-            cap = cap.minimum(other_cap)?;
-        }
-        Some(cap)
-    }
-
     pub(super) fn remove_edge_and_set_latest(
         &mut self,
         edge: impl BorrowPCGEdgeLike<'tcx>,
@@ -286,16 +270,18 @@ impl<'tcx> BorrowsState<'tcx> {
             if let Some(local_node) = node.as_local_node(repacker) {
                 let blocked_cap = self.get_capability(node);
 
-                let restore_cap = self.min_blocked_by_capability(edge.kind(), repacker);
+                let restore_cap = if local_node.place().projects_shared_ref(repacker) {
+                    CapabilityKind::Read
+                } else {
+                    CapabilityKind::Exclusive
+                };
 
-                if let Some(restore_cap) = restore_cap {
-                    if blocked_cap.is_none_or(|bc| bc < restore_cap) {
-                        self.record_and_apply_action(
-                            BorrowPCGAction::restore_capability(local_node, restore_cap),
-                            &mut actions,
-                            repacker,
-                        )?;
-                    }
+                if blocked_cap.is_none_or(|bc| bc < restore_cap) {
+                    self.record_and_apply_action(
+                        BorrowPCGAction::restore_capability(local_node, restore_cap),
+                        &mut actions,
+                        repacker,
+                    )?;
                 }
             }
         }
@@ -493,7 +479,7 @@ impl<'tcx> BorrowsState<'tcx> {
                 BorrowKind::Mut {
                     kind: MutBorrowKind::Default,
                 } => {
-                    self.remove_capability(blocked_place.into());
+                    let _ = self.remove_capability(blocked_place.into());
                 }
                 _ => {
                     match self.get_capability(blocked_place.into()) {

src/borrow_pcg/state/obtain.rs

@@ -79,10 +79,8 @@ impl<'tcx> BorrowsState<'tcx> {
         }
 
         if !self.contains(place, repacker) {
-            let extra_acts = self.expand_to(place, repacker, location, obtain_reason)?;
+            let extra_acts = self.expand_to(place, repacker, location)?;
             actions.extend(extra_acts);
-        } else {
-            eprintln!("Already contains {:?}", place);
         }
 
         Ok(actions)
@@ -189,7 +187,6 @@ impl<'tcx> BorrowsState<'tcx> {
         to_place: Place<'tcx>,
         repacker: PlaceRepacker<'_, 'tcx>,
         location: Location,
-        obtain_reason: ObtainReason,
     ) -> Result<ExecutedActions<'tcx>, PCGError> {
         let mut actions = ExecutedActions::new();
 
@@ -236,8 +233,7 @@ impl<'tcx> BorrowsState<'tcx> {
                     BorrowPCGEdge::new(
                         BorrowPCGEdgeKind::BorrowPCGExpansion(expansion),
                         PathConditions::new(location.block),
-                    )
-                    .into(),
+                    ),
                     true,
                 );
                 self.record_and_apply_action(action, &mut actions, repacker)?;
@@ -264,8 +260,7 @@ impl<'tcx> BorrowsState<'tcx> {
                             BorrowPCGEdge::new(
                                 BorrowPCGEdgeKind::BorrowPCGExpansion(expansion),
                                 PathConditions::new(location.block),
-                            )
-                            .into(),
+                            ),
                             true,
                         ),
                         &mut actions,

src/borrow_pcg/visitor/mod.rs

@@ -22,7 +22,7 @@ use crate::{
 };
 
 use super::{
-    action::{BorrowPCGAction, BorrowPCGActionKind},
+    action::BorrowPCGAction,
     borrow_pcg_edge::BorrowPCGEdge,
     coupling_graph_constructor::BorrowCheckerInterface,
     edge::block::BlockEdge,
@@ -166,8 +166,7 @@ impl<'tcx, 'mir, 'state> BorrowsVisitor<'tcx, 'mir, 'state> {
                         )
                         .into(),
                         PathConditions::AtBlock(location.block),
-                    )
-                    .into(),
+                    ),
                     true,
                 ));
             }
@@ -262,7 +261,6 @@ impl<'tcx, 'mir, 'state> BorrowsVisitor<'tcx, 'mir, 'state> {
                 BorrowPCGEdge::new(
                     AbstractionType::FunctionCall(
                         FunctionCallAbstraction::new(location, *func_def_id, substs, edges.clone())
-                            .into(),
                     )
                     .into(),
                     PathConditions::AtBlock(location.block),

src/utils/place/maybe_old.rs

@@ -7,7 +7,6 @@ use crate::borrow_pcg::region_projection::{
 };
 use crate::borrow_pcg::visitor::extract_regions;
 use crate::combined_pcs::{LocalNodeLike, PCGError, PCGNode, PCGNodeLike};
-use crate::rustc_interface::ast::Mutability;
 use crate::rustc_interface::index::IndexVec;
 use crate::rustc_interface::middle::mir;
 use crate::rustc_interface::middle::mir::tcx::PlaceTy;
@@ -224,19 +223,6 @@ impl<'tcx> MaybeOldPlace<'tcx> {
             .map(|rp| rp.set_base(rp.base.into(), repacker))
     }
 
-    pub(crate) fn mutability(&self, repacker: PlaceRepacker<'_, 'tcx>) -> Mutability {
-        let place: Place<'_> = self.place();
-        place.ref_mutability(repacker).unwrap_or_else(|| {
-            if let Ok(root_place) =
-                place.is_mutable(crate::utils::LocalMutationIsAllowed::Yes, repacker)
-                && root_place.is_local_mutation_allowed == crate::utils::LocalMutationIsAllowed::Yes
-            {
-                Mutability::Mut
-            } else {
-                Mutability::Not
-            }
-        })
-    }
     pub(crate) fn is_owned(&self, repacker: PlaceRepacker<'_, 'tcx>) -> bool {
         self.place().is_owned(repacker)
     }

src/utils/place/maybe_remote.rs

@@ -3,7 +3,6 @@ use crate::borrow_pcg::region_projection::{
     MaybeRemoteRegionProjectionBase, PCGRegion, RegionIdx, RegionProjectionBaseLike,
 };
 use crate::combined_pcs::{PCGNode, PCGNodeLike};
-use crate::rustc_interface::ast::Mutability;
 use crate::rustc_interface::index::IndexVec;
 use crate::rustc_interface::middle::{mir, ty};
 use crate::utils::display::DisplayWithRepacker;
@@ -86,12 +85,6 @@ impl std::fmt::Display for MaybeRemotePlace<'_> {
 }
 
 impl<'tcx> MaybeRemotePlace<'tcx> {
-    pub(crate) fn mutability(&self, repacker: PlaceRepacker<'_, 'tcx>) -> Mutability {
-        match self {
-            MaybeRemotePlace::Local(p) => p.mutability(repacker),
-            MaybeRemotePlace::Remote(rp) => rp.mutability(repacker),
-        }
-    }
 
     pub fn place_assigned_to_local(local: mir::Local) -> Self {
         MaybeRemotePlace::Remote(RemotePlace { local })
@@ -184,18 +177,4 @@ impl RemotePlace {
             _ => todo!(),
         }
     }
-
-    pub(crate) fn mutability(&self, repacker: PlaceRepacker<'_, '_>) -> Mutability {
-        let place: Place<'_> = self.local.into();
-        place.ref_mutability(repacker).unwrap_or_else(|| {
-            if let Ok(root_place) =
-                place.is_mutable(crate::utils::LocalMutationIsAllowed::Yes, repacker)
-                && root_place.is_local_mutation_allowed == crate::utils::LocalMutationIsAllowed::Yes
-            {
-                Mutability::Mut
-            } else {
-                Mutability::Not
-            }
-        })
-    }
 }