Rollup merge of rust-lang#148165 - nnethercote:less-mut-Analysis, r=cjgillot

Use `mut` less in dataflow analysis

`&mut Analysis` is used a lot:
- In results visitors, even though only one visitor needs mutability.
- In all the `apply_*` methods, even though only one visitor needs mutability.

I've lost track of the number of times I've thought "why are these `mut` again?" and had to look through the code to remind myself. It's really unexpected, and most `Analysis` instances are immutable, because the `state` values are what get mutated.

This commit introduces `RefCell` in one analysis and one results visitor. This then lets another existing `RefCell` be removed, and a ton of `&mut Analysis` arguments become `&Analysis`. And then `Analysis` and `Results` can be recombined.

r? `@cjgillot`

Author: Zalathar

compiler/rustc_borrowck/src/dataflow.rs

@@ -44,7 +44,7 @@ impl<'a, 'tcx> Analysis<'tcx> for Borrowck<'a, 'tcx> {
     }
 
     fn apply_early_statement_effect(
-        &mut self,
+        &self,
         state: &mut Self::Domain,
         stmt: &mir::Statement<'tcx>,
         loc: Location,
@@ -55,7 +55,7 @@ impl<'a, 'tcx> Analysis<'tcx> for Borrowck<'a, 'tcx> {
     }
 
     fn apply_primary_statement_effect(
-        &mut self,
+        &self,
         state: &mut Self::Domain,
         stmt: &mir::Statement<'tcx>,
         loc: Location,
@@ -66,7 +66,7 @@ impl<'a, 'tcx> Analysis<'tcx> for Borrowck<'a, 'tcx> {
     }
 
     fn apply_early_terminator_effect(
-        &mut self,
+        &self,
         state: &mut Self::Domain,
         term: &mir::Terminator<'tcx>,
         loc: Location,
@@ -77,7 +77,7 @@ impl<'a, 'tcx> Analysis<'tcx> for Borrowck<'a, 'tcx> {
     }
 
     fn apply_primary_terminator_effect<'mir>(
-        &mut self,
+        &self,
         state: &mut Self::Domain,
         term: &'mir mir::Terminator<'tcx>,
         loc: Location,
@@ -92,7 +92,7 @@ impl<'a, 'tcx> Analysis<'tcx> for Borrowck<'a, 'tcx> {
     }
 
     fn apply_call_return_effect(
-        &mut self,
+        &self,
         _state: &mut Self::Domain,
         _block: BasicBlock,
         _return_places: CallReturnPlaces<'_, 'tcx>,
@@ -533,7 +533,7 @@ impl<'tcx> rustc_mir_dataflow::Analysis<'tcx> for Borrows<'_, 'tcx> {
     }
 
     fn apply_early_statement_effect(
-        &mut self,
+        &self,
         state: &mut Self::Domain,
         _statement: &mir::Statement<'tcx>,
         location: Location,
@@ -542,7 +542,7 @@ impl<'tcx> rustc_mir_dataflow::Analysis<'tcx> for Borrows<'_, 'tcx> {
     }
 
     fn apply_primary_statement_effect(
-        &mut self,
+        &self,
         state: &mut Self::Domain,
         stmt: &mir::Statement<'tcx>,
         location: Location,
@@ -590,7 +590,7 @@ impl<'tcx> rustc_mir_dataflow::Analysis<'tcx> for Borrows<'_, 'tcx> {
     }
 
     fn apply_early_terminator_effect(
-        &mut self,
+        &self,
         state: &mut Self::Domain,
         _terminator: &mir::Terminator<'tcx>,
         location: Location,
@@ -599,7 +599,7 @@ impl<'tcx> rustc_mir_dataflow::Analysis<'tcx> for Borrows<'_, 'tcx> {
     }
 
     fn apply_primary_terminator_effect<'mir>(
-        &mut self,
+        &self,
         state: &mut Self::Domain,
         terminator: &'mir mir::Terminator<'tcx>,
         _location: Location,

compiler/rustc_borrowck/src/lib.rs

@@ -49,7 +49,7 @@ use rustc_mir_dataflow::move_paths::{
     InitIndex, InitLocation, LookupResult, MoveData, MovePathIndex,
 };
 use rustc_mir_dataflow::points::DenseLocationMap;
-use rustc_mir_dataflow::{Analysis, Results, ResultsVisitor, visit_results};
+use rustc_mir_dataflow::{Analysis, EntryStates, Results, ResultsVisitor, visit_results};
 use rustc_session::lint::builtin::{TAIL_EXPR_DROP_ORDER, UNUSED_MUT};
 use rustc_span::{ErrorGuaranteed, Span, Symbol};
 use smallvec::SmallVec;
@@ -537,13 +537,11 @@ fn borrowck_check_region_constraints<'tcx>(
         mbcx.report_region_errors(nll_errors);
     }
 
-    let (mut flow_analysis, flow_entry_states) =
-        get_flow_results(tcx, body, &move_data, &borrow_set, &regioncx);
+    let flow_results = get_flow_results(tcx, body, &move_data, &borrow_set, &regioncx);
     visit_results(
         body,
         traversal::reverse_postorder(body).map(|(bb, _)| bb),
-        &mut flow_analysis,
-        &flow_entry_states,
+        &flow_results,
         &mut mbcx,
     );
 
@@ -604,7 +602,7 @@ fn get_flow_results<'a, 'tcx>(
     move_data: &'a MoveData<'tcx>,
     borrow_set: &'a BorrowSet<'tcx>,
     regioncx: &RegionInferenceContext<'tcx>,
-) -> (Borrowck<'a, 'tcx>, Results<BorrowckDomain>) {
+) -> Results<'tcx, Borrowck<'a, 'tcx>> {
     // We compute these three analyses individually, but them combine them into
     // a single results so that `mbcx` can visit them all together.
     let borrows = Borrows::new(tcx, body, regioncx, borrow_set).iterate_to_fixpoint(
@@ -629,14 +627,14 @@ fn get_flow_results<'a, 'tcx>(
         ever_inits: ever_inits.analysis,
     };
 
-    assert_eq!(borrows.results.len(), uninits.results.len());
-    assert_eq!(borrows.results.len(), ever_inits.results.len());
-    let results: Results<_> =
-        itertools::izip!(borrows.results, uninits.results, ever_inits.results)
+    assert_eq!(borrows.entry_states.len(), uninits.entry_states.len());
+    assert_eq!(borrows.entry_states.len(), ever_inits.entry_states.len());
+    let entry_states: EntryStates<_> =
+        itertools::izip!(borrows.entry_states, uninits.entry_states, ever_inits.entry_states)
             .map(|(borrows, uninits, ever_inits)| BorrowckDomain { borrows, uninits, ever_inits })
             .collect();
 
-    (analysis, results)
+    Results { analysis, entry_states }
 }
 
 pub(crate) struct BorrowckInferCtxt<'tcx> {
@@ -790,7 +788,7 @@ struct MirBorrowckCtxt<'a, 'infcx, 'tcx> {
 impl<'a, 'tcx> ResultsVisitor<'tcx, Borrowck<'a, 'tcx>> for MirBorrowckCtxt<'a, '_, 'tcx> {
     fn visit_after_early_statement_effect(
         &mut self,
-        _analysis: &mut Borrowck<'a, 'tcx>,
+        _analysis: &Borrowck<'a, 'tcx>,
         state: &BorrowckDomain,
         stmt: &Statement<'tcx>,
         location: Location,
@@ -865,7 +863,7 @@ impl<'a, 'tcx> ResultsVisitor<'tcx, Borrowck<'a, 'tcx>> for MirBorrowckCtxt<'a,
 
     fn visit_after_early_terminator_effect(
         &mut self,
-        _analysis: &mut Borrowck<'a, 'tcx>,
+        _analysis: &Borrowck<'a, 'tcx>,
         state: &BorrowckDomain,
         term: &Terminator<'tcx>,
         loc: Location,
@@ -985,7 +983,7 @@ impl<'a, 'tcx> ResultsVisitor<'tcx, Borrowck<'a, 'tcx>> for MirBorrowckCtxt<'a,
 
     fn visit_after_primary_terminator_effect(
         &mut self,
-        _analysis: &mut Borrowck<'a, 'tcx>,
+        _analysis: &Borrowck<'a, 'tcx>,
         state: &BorrowckDomain,
         term: &Terminator<'tcx>,
         loc: Location,

compiler/rustc_const_eval/src/check_consts/resolver.rs

@@ -332,7 +332,7 @@ where
     }
 
     fn apply_primary_statement_effect(
-        &mut self,
+        &self,
         state: &mut Self::Domain,
         statement: &mir::Statement<'tcx>,
         location: Location,
@@ -341,7 +341,7 @@ where
     }
 
     fn apply_primary_terminator_effect<'mir>(
-        &mut self,
+        &self,
         state: &mut Self::Domain,
         terminator: &'mir mir::Terminator<'tcx>,
         location: Location,
@@ -351,7 +351,7 @@ where
     }
 
     fn apply_call_return_effect(
-        &mut self,
+        &self,
         state: &mut Self::Domain,
         block: BasicBlock,
         return_places: CallReturnPlaces<'_, 'tcx>,

compiler/rustc_mir_dataflow/src/framework/cursor.rs

@@ -1,39 +1,28 @@
 //! Random access inspection of the results of a dataflow analysis.
 
-use std::borrow::Cow;
 use std::cmp::Ordering;
-use std::ops::{Deref, DerefMut};
+use std::ops::Deref;
 
 #[cfg(debug_assertions)]
 use rustc_index::bit_set::DenseBitSet;
 use rustc_middle::mir::{self, BasicBlock, Location};
 
 use super::{Analysis, Direction, Effect, EffectIndex, Results};
 
-/// Some `ResultsCursor`s want to own an `Analysis`, and some want to borrow an `Analysis`, either
-/// mutable or immutably. This type allows all of the above. It's similar to `Cow`, but `Cow`
-/// doesn't allow mutable borrowing.
-enum CowMut<'a, T> {
-    BorrowedMut(&'a mut T),
+/// This is like `Cow`, but it lacks the `T: ToOwned` bound and doesn't support
+/// `to_owned`/`into_owned`.
+enum SimpleCow<'a, T> {
+    Borrowed(&'a T),
     Owned(T),
 }
 
-impl<T> Deref for CowMut<'_, T> {
+impl<T> Deref for SimpleCow<'_, T> {
     type Target = T;
 
     fn deref(&self) -> &T {
         match self {
-            CowMut::BorrowedMut(borrowed) => borrowed,
-            CowMut::Owned(owned) => owned,
-        }
-    }
-}
-
-impl<T> DerefMut for CowMut<'_, T> {
-    fn deref_mut(&mut self) -> &mut T {
-        match self {
-            CowMut::BorrowedMut(borrowed) => borrowed,
-            CowMut::Owned(owned) => owned,
+            SimpleCow::Borrowed(borrowed) => borrowed,
+            SimpleCow::Owned(owned) => owned,
         }
     }
 }
@@ -53,8 +42,7 @@ where
     A: Analysis<'tcx>,
 {
     body: &'mir mir::Body<'tcx>,
-    analysis: CowMut<'mir, A>,
-    results: Cow<'mir, Results<A::Domain>>,
+    results: SimpleCow<'mir, Results<'tcx, A>>,
     state: A::Domain,
 
     pos: CursorPosition,
@@ -82,15 +70,10 @@ where
         self.body
     }
 
-    fn new(
-        body: &'mir mir::Body<'tcx>,
-        analysis: CowMut<'mir, A>,
-        results: Cow<'mir, Results<A::Domain>>,
-    ) -> Self {
-        let bottom_value = analysis.bottom_value(body);
+    fn new(body: &'mir mir::Body<'tcx>, results: SimpleCow<'mir, Results<'tcx, A>>) -> Self {
+        let bottom_value = results.analysis.bottom_value(body);
         ResultsCursor {
             body,
-            analysis,
             results,
 
             // Initialize to the `bottom_value` and set `state_needs_reset` to tell the cursor that
@@ -106,21 +89,13 @@ where
     }
 
     /// Returns a new cursor that takes ownership of and inspects analysis results.
-    pub fn new_owning(
-        body: &'mir mir::Body<'tcx>,
-        analysis: A,
-        results: Results<A::Domain>,
-    ) -> Self {
-        Self::new(body, CowMut::Owned(analysis), Cow::Owned(results))
+    pub fn new_owning(body: &'mir mir::Body<'tcx>, results: Results<'tcx, A>) -> Self {
+        Self::new(body, SimpleCow::Owned(results))
     }
 
     /// Returns a new cursor that borrows and inspects analysis results.
-    pub fn new_borrowing(
-        body: &'mir mir::Body<'tcx>,
-        analysis: &'mir mut A,
-        results: &'mir Results<A::Domain>,
-    ) -> Self {
-        Self::new(body, CowMut::BorrowedMut(analysis), Cow::Borrowed(results))
+    pub fn new_borrowing(body: &'mir mir::Body<'tcx>, results: &'mir Results<'tcx, A>) -> Self {
+        Self::new(body, SimpleCow::Borrowed(results))
     }
 
     /// Allows inspection of unreachable basic blocks even with `debug_assertions` enabled.
@@ -132,7 +107,7 @@ where
 
     /// Returns the `Analysis` used to generate the underlying `Results`.
     pub fn analysis(&self) -> &A {
-        &self.analysis
+        &self.results.analysis
     }
 
     /// Resets the cursor to hold the entry set for the given basic block.
@@ -144,7 +119,7 @@ where
         #[cfg(debug_assertions)]
         assert!(self.reachable_blocks.contains(block));
 
-        self.state.clone_from(&self.results[block]);
+        self.state.clone_from(&self.results.entry_states[block]);
         self.pos = CursorPosition::block_entry(block);
         self.state_needs_reset = false;
     }
@@ -236,7 +211,7 @@ where
         let target_effect_index = effect.at_index(target.statement_index);
 
         A::Direction::apply_effects_in_range(
-            &mut *self.analysis,
+            &self.results.analysis,
             &mut self.state,
             target.block,
             block_data,
@@ -251,8 +226,8 @@ where
     ///
     /// This can be used, e.g., to apply the call return effect directly to the cursor without
     /// creating an extra copy of the dataflow state.
-    pub fn apply_custom_effect(&mut self, f: impl FnOnce(&mut A, &mut A::Domain)) {
-        f(&mut self.analysis, &mut self.state);
+    pub fn apply_custom_effect(&mut self, f: impl FnOnce(&A, &mut A::Domain)) {
+        f(&self.results.analysis, &mut self.state);
         self.state_needs_reset = true;
     }
 }