Auto merge of #108293 - Jarcho:mut_analyses, r=eholk

Take MIR dataflow analyses by mutable reference

The main motivation here is any analysis requiring dynamically sized scratch memory to work. One concrete example would be pointer target tracking, where tracking the results of a dereference can result in multiple possible targets. This leads to processing multi-level dereferences requiring the ability to handle a changing number of potential targets per step. A (simplified) function for this would be `fn apply_deref(potential_targets: &mut Vec<Target>)` which would use the scratch space contained in the analysis to send arguments and receive the results.

The alternative to this would be to wrap everything in a `RefCell`, which is what `MaybeRequiresStorage` currently does. This comes with a small perf cost and loses the compiler's guarantee that we don't try to take multiple borrows at the same time.

For the implementation:
* `AnalysisResults` is an unfortunate requirement to avoid an unconstrained type parameter error.
* `CloneAnalysis` could just be `Clone` instead, but that would result in more work than is required to have multiple cursors over the same result set.
* `ResultsVisitor` now takes the results type on in each function as there's no other way to have access to the analysis without cloning it. This could use an associated type rather than a type parameter, but the current approach makes it easier to not care about the type when it's not necessary.
* `MaybeRequiresStorage` now no longer uses a `RefCell`, but the graphviz formatter now does. It could be removed, but that would require even more changes and doesn't really seem necessary.

Author: bors

compiler/rustc_borrowck/src/dataflow.rs

@@ -59,7 +59,7 @@ macro_rules! impl_visitable {
             }
 
             fn reconstruct_before_statement_effect(
-                &self,
+                &mut self,
                 state: &mut Self::FlowState,
                 stmt: &mir::Statement<'tcx>,
                 loc: Location,
@@ -69,7 +69,7 @@ macro_rules! impl_visitable {
             }
 
             fn reconstruct_statement_effect(
-                &self,
+                &mut self,
                 state: &mut Self::FlowState,
                 stmt: &mir::Statement<'tcx>,
                 loc: Location,
@@ -79,7 +79,7 @@ macro_rules! impl_visitable {
             }
 
             fn reconstruct_before_terminator_effect(
-                &self,
+                &mut self,
                 state: &mut Self::FlowState,
                 term: &mir::Terminator<'tcx>,
                 loc: Location,
@@ -89,7 +89,7 @@ macro_rules! impl_visitable {
             }
 
             fn reconstruct_terminator_effect(
-                &self,
+                &mut self,
                 state: &mut Self::FlowState,
                 term: &mir::Terminator<'tcx>,
                 loc: Location,
@@ -343,7 +343,7 @@ impl<'tcx> rustc_mir_dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
     type Idx = BorrowIndex;
 
     fn before_statement_effect(
-        &self,
+        &mut self,
         trans: &mut impl GenKill<Self::Idx>,
         _statement: &mir::Statement<'tcx>,
         location: Location,
@@ -352,7 +352,7 @@ impl<'tcx> rustc_mir_dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
     }
 
     fn statement_effect(
-        &self,
+        &mut self,
         trans: &mut impl GenKill<Self::Idx>,
         stmt: &mir::Statement<'tcx>,
         location: Location,
@@ -400,7 +400,7 @@ impl<'tcx> rustc_mir_dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
     }
 
     fn before_terminator_effect(
-        &self,
+        &mut self,
         trans: &mut impl GenKill<Self::Idx>,
         _terminator: &mir::Terminator<'tcx>,
         location: Location,
@@ -409,7 +409,7 @@ impl<'tcx> rustc_mir_dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
     }
 
     fn terminator_effect(
-        &self,
+        &mut self,
         trans: &mut impl GenKill<Self::Idx>,
         terminator: &mir::Terminator<'tcx>,
         _location: Location,
@@ -426,7 +426,7 @@ impl<'tcx> rustc_mir_dataflow::GenKillAnalysis<'tcx> for Borrows<'_, 'tcx> {
     }
 
     fn call_return_effect(
-        &self,
+        &mut self,
         _trans: &mut impl GenKill<Self::Idx>,
         _block: mir::BasicBlock,
         _return_places: CallReturnPlaces<'_, 'tcx>,

compiler/rustc_borrowck/src/lib.rs

@@ -368,7 +368,7 @@ fn do_mir_borrowck<'tcx>(
     // Compute and report region errors, if any.
     mbcx.report_region_errors(nll_errors);
 
-    let results = BorrowckResults {
+    let mut results = BorrowckResults {
         ever_inits: flow_ever_inits,
         uninits: flow_uninits,
         borrows: flow_borrows,
@@ -379,7 +379,7 @@ fn do_mir_borrowck<'tcx>(
     rustc_mir_dataflow::visit_results(
         body,
         traversal::reverse_postorder(body).map(|(bb, _)| bb),
-        &results,
+        &mut results,
         &mut mbcx,
     );
 
@@ -598,11 +598,12 @@ struct MirBorrowckCtxt<'cx, 'tcx> {
 // 2. loans made in overlapping scopes do not conflict
 // 3. assignments do not affect things loaned out as immutable
 // 4. moves do not affect things loaned out in any way
-impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tcx> {
+impl<'cx, 'tcx, R> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx, R> for MirBorrowckCtxt<'cx, 'tcx> {
     type FlowState = Flows<'cx, 'tcx>;
 
     fn visit_statement_before_primary_effect(
         &mut self,
+        _results: &R,
         flow_state: &Flows<'cx, 'tcx>,
         stmt: &'cx Statement<'tcx>,
         location: Location,
@@ -672,6 +673,7 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx
 
     fn visit_terminator_before_primary_effect(
         &mut self,
+        _results: &R,
         flow_state: &Flows<'cx, 'tcx>,
         term: &'cx Terminator<'tcx>,
         loc: Location,
@@ -782,6 +784,7 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx
 
     fn visit_terminator_after_primary_effect(
         &mut self,
+        _results: &R,
         flow_state: &Flows<'cx, 'tcx>,
         term: &'cx Terminator<'tcx>,
         loc: Location,

compiler/rustc_const_eval/src/transform/check_consts/resolver.rs

@@ -337,7 +337,7 @@ where
     Q: Qualif,
 {
     fn apply_statement_effect(
-        &self,
+        &mut self,
         state: &mut Self::Domain,
         statement: &mir::Statement<'tcx>,
         location: Location,
@@ -346,7 +346,7 @@ where
     }
 
     fn apply_terminator_effect(
-        &self,
+        &mut self,
         state: &mut Self::Domain,
         terminator: &mir::Terminator<'tcx>,
         location: Location,
@@ -355,7 +355,7 @@ where
     }
 
     fn apply_call_return_effect(
-        &self,
+        &mut self,
         state: &mut Self::Domain,
         block: BasicBlock,
         return_places: CallReturnPlaces<'_, 'tcx>,

compiler/rustc_mir_dataflow/src/framework/cursor.rs

@@ -1,18 +1,60 @@
 //! Random access inspection of the results of a dataflow analysis.
 
-use crate::framework::BitSetExt;
+use crate::{framework::BitSetExt, CloneAnalysis};
 
-use std::borrow::Borrow;
+use std::borrow::{Borrow, BorrowMut};
 use std::cmp::Ordering;
 
 #[cfg(debug_assertions)]
 use rustc_index::bit_set::BitSet;
 use rustc_middle::mir::{self, BasicBlock, Location};
 
-use super::{Analysis, Direction, Effect, EffectIndex, Results};
+use super::{Analysis, Direction, Effect, EffectIndex, EntrySets, Results, ResultsCloned};
+
+// `AnalysisResults` is needed as an impl such as the following has an unconstrained type
+// parameter:
+// ```
+// impl<'tcx, A, E, R> ResultsCursor<'_, 'tcx, A, R>
+// where
+//     A: Analysis<'tcx>,
+//     E: Borrow<EntrySets<'tcx, A>>,
+//     R: Results<'tcx, A, E>,
+// {}
+// ```
+
+/// A type representing the analysis results consumed by a `ResultsCursor`.
+pub trait AnalysisResults<'tcx, A>: BorrowMut<Results<'tcx, A, Self::EntrySets>>
+where
+    A: Analysis<'tcx>,
+{
+    /// The type containing the entry sets for this `Results` type.
+    ///
+    /// Should be either `EntrySets<'tcx, A>` or `&EntrySets<'tcx, A>`.
+    type EntrySets: Borrow<EntrySets<'tcx, A>>;
+}
+impl<'tcx, A, E> AnalysisResults<'tcx, A> for Results<'tcx, A, E>
+where
+    A: Analysis<'tcx>,
+    E: Borrow<EntrySets<'tcx, A>>,
+{
+    type EntrySets = E;
+}
+impl<'a, 'tcx, A, E> AnalysisResults<'tcx, A> for &'a mut Results<'tcx, A, E>
+where
+    A: Analysis<'tcx>,
+    E: Borrow<EntrySets<'tcx, A>>,
+{
+    type EntrySets = E;
+}
 
 /// A `ResultsCursor` that borrows the underlying `Results`.
-pub type ResultsRefCursor<'a, 'mir, 'tcx, A> = ResultsCursor<'mir, 'tcx, A, &'a Results<'tcx, A>>;
+pub type ResultsRefCursor<'res, 'mir, 'tcx, A> =
+    ResultsCursor<'mir, 'tcx, A, &'res mut Results<'tcx, A>>;
+
+/// A `ResultsCursor` which uses a cloned `Analysis` while borrowing the underlying `Results`. This
+/// allows multiple cursors over the same `Results`.
+pub type ResultsClonedCursor<'res, 'mir, 'tcx, A> =
+    ResultsCursor<'mir, 'tcx, A, ResultsCloned<'res, 'tcx, A>>;
 
 /// Allows random access inspection of the results of a dataflow analysis.
 ///
@@ -45,7 +87,38 @@ where
 impl<'mir, 'tcx, A, R> ResultsCursor<'mir, 'tcx, A, R>
 where
     A: Analysis<'tcx>,
-    R: Borrow<Results<'tcx, A>>,
+{
+    /// Returns the dataflow state at the current location.
+    pub fn get(&self) -> &A::Domain {
+        &self.state
+    }
+
+    /// Returns the body this analysis was run on.
+    pub fn body(&self) -> &'mir mir::Body<'tcx> {
+        self.body
+    }
+
+    /// Unwraps this cursor, returning the underlying `Results`.
+    pub fn into_results(self) -> R {
+        self.results
+    }
+}
+
+impl<'res, 'mir, 'tcx, A> ResultsCursor<'mir, 'tcx, A, ResultsCloned<'res, 'tcx, A>>
+where
+    A: Analysis<'tcx> + CloneAnalysis,
+{
+    /// Creates a new cursor over the same `Results`. Note that the cursor's position is *not*
+    /// copied.
+    pub fn new_cursor(&self) -> Self {
+        Self::new(self.body, self.results.reclone_analysis())
+    }
+}
+
+impl<'mir, 'tcx, A, R> ResultsCursor<'mir, 'tcx, A, R>
+where
+    A: Analysis<'tcx>,
+    R: AnalysisResults<'tcx, A>,
 {
     /// Returns a new cursor that can inspect `results`.
     pub fn new(body: &'mir mir::Body<'tcx>, results: R) -> Self {
@@ -74,18 +147,28 @@ where
     }
 
     /// Returns the underlying `Results`.
-    pub fn results(&self) -> &Results<'tcx, A> {
-        &self.results.borrow()
+    pub fn results(&mut self) -> &Results<'tcx, A, R::EntrySets> {
+        self.results.borrow()
+    }
+
+    /// Returns the underlying `Results`.
+    pub fn mut_results(&mut self) -> &mut Results<'tcx, A, R::EntrySets> {
+        self.results.borrow_mut()
     }
 
     /// Returns the `Analysis` used to generate the underlying `Results`.
     pub fn analysis(&self) -> &A {
         &self.results.borrow().analysis
     }
 
-    /// Returns the dataflow state at the current location.
-    pub fn get(&self) -> &A::Domain {
-        &self.state
+    /// Returns the `Analysis` used to generate the underlying `Results`.
+    pub fn mut_analysis(&mut self) -> &mut A {
+        &mut self.results.borrow_mut().analysis
+    }
+
+    /// Returns both the dataflow state at the current location and the `Analysis`.
+    pub fn get_with_analysis(&mut self) -> (&A::Domain, &mut A) {
+        (&self.state, &mut self.results.borrow_mut().analysis)
     }
 
     /// Resets the cursor to hold the entry set for the given basic block.
@@ -97,7 +180,7 @@ where
         #[cfg(debug_assertions)]
         assert!(self.reachable_blocks.contains(block));
 
-        self.state.clone_from(&self.results.borrow().entry_set_for_block(block));
+        self.state.clone_from(self.results.borrow().entry_set_for_block(block));
         self.pos = CursorPosition::block_entry(block);
         self.state_needs_reset = false;
     }
@@ -186,7 +269,7 @@ where
             )
         };
 
-        let analysis = &self.results.borrow().analysis;
+        let analysis = &mut self.results.borrow_mut().analysis;
         let target_effect_index = effect.at_index(target.statement_index);
 
         A::Direction::apply_effects_in_range(
@@ -205,8 +288,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(&A, &mut A::Domain)) {
-        f(&self.results.borrow().analysis, &mut self.state);
+    pub fn apply_custom_effect(&mut self, f: impl FnOnce(&mut A, &mut A::Domain)) {
+        f(&mut self.results.borrow_mut().analysis, &mut self.state);
         self.state_needs_reset = true;
     }
 }
@@ -215,7 +298,6 @@ impl<'mir, 'tcx, A, R> ResultsCursor<'mir, 'tcx, A, R>
 where
     A: crate::GenKillAnalysis<'tcx>,
     A::Domain: BitSetExt<A::Idx>,
-    R: Borrow<Results<'tcx, A>>,
 {
     pub fn contains(&self, elem: A::Idx) -> bool {
         self.get().contains(elem)