Make it possible for ResultsCursor to borrow a Results.

`ResultsCursor` currently owns its `Results`. But sometimes the
`Results` is needed again afterwards. So there is
`ResultsCursor::into_results` for extracting the `Results`, which leads
to some awkwardness.

This commit adds `ResultsHandle`, a `Cow`-like type that can either
borrow or own a a `Results`. `ResultsCursor` now uses it. This is good
because some `ResultsCursor`s really want to own their `Results`, while
others just want to borrow it.

We end with with a few more lines of code, but get some nice cleanups.
- `ResultsCursor::into_results` is removed.
- `Formatter::into_results` is removed.
- `write_graphviz_results` now just borrows a `Results`, instead of the
  awkward "take ownership of a `Results` and then return it unchanged"
  pattern.
- `MaybeRequiresStorage` can borrow the `MaybeBorrowedLocals` results,
  avoiding two `clone` calls: one in `check_for_move` and one in
  `locals_live_across_suspend_points`.
- `Results` no longer needs to derive `Clone`.

This reinstates the cursor flexibility that was lost in #118230 -- which
removed the old `ResultsRefCursor` and `ResultsCloneCursor` types -- but
in a *much* simpler way. Hooray!

Author: nnethercote

compiler/rustc_mir_dataflow/src/framework/cursor.rs

@@ -1,13 +1,39 @@
 //! Random access inspection of the results of a dataflow analysis.
 
 use std::cmp::Ordering;
+use std::ops::Deref;
 
 #[cfg(debug_assertions)]
 use rustc_index::bit_set::BitSet;
 use rustc_middle::mir::{self, BasicBlock, Location};
 
 use super::{Analysis, Direction, Effect, EffectIndex, Results};
 
+/// Some `ResultsCursor`s want to own a `Results`, and some want to borrow a `Results`. This type
+/// allows either. We could use `Cow` but that would require `Results` and `A` to impl `Clone`. So
+/// this is a greatly cut-down alternative to `Cow`.
+pub enum ResultsHandle<'a, 'tcx, A>
+where
+    A: Analysis<'tcx>,
+{
+    Borrowed(&'a Results<'tcx, A>),
+    Owned(Results<'tcx, A>),
+}
+
+impl<'tcx, A> Deref for ResultsHandle<'_, 'tcx, A>
+where
+    A: Analysis<'tcx>,
+{
+    type Target = Results<'tcx, A>;
+
+    fn deref(&self) -> &Results<'tcx, A> {
+        match self {
+            ResultsHandle::Borrowed(borrowed) => borrowed,
+            ResultsHandle::Owned(owned) => owned,
+        }
+    }
+}
+
 /// Allows random access inspection of the results of a dataflow analysis.
 ///
 /// This cursor only has linear performance within a basic block when its statements are visited in
@@ -19,7 +45,7 @@ where
     A: Analysis<'tcx>,
 {
     body: &'mir mir::Body<'tcx>,
-    results: Results<'tcx, A>,
+    results: ResultsHandle<'mir, 'tcx, A>,
     state: A::Domain,
 
     pos: CursorPosition,
@@ -47,13 +73,8 @@ where
         self.body
     }
 
-    /// Unwraps this cursor, returning the underlying `Results`.
-    pub fn into_results(self) -> Results<'tcx, A> {
-        self.results
-    }
-
     /// Returns a new cursor that can inspect `results`.
-    pub fn new(body: &'mir mir::Body<'tcx>, results: Results<'tcx, A>) -> Self {
+    pub fn new(body: &'mir mir::Body<'tcx>, results: ResultsHandle<'mir, 'tcx, A>) -> Self {
         let bottom_value = results.analysis.bottom_value(body);
         ResultsCursor {
             body,

compiler/rustc_mir_dataflow/src/framework/graphviz.rs

@@ -47,20 +47,16 @@ where
 {
     pub(crate) fn new(
         body: &'mir Body<'tcx>,
-        results: Results<'tcx, A>,
+        results: &'mir Results<'tcx, A>,
         style: OutputStyle,
     ) -> Self {
         let reachable = mir::traversal::reachable_as_bitset(body);
-        Formatter { cursor: results.into_results_cursor(body).into(), style, reachable }
+        Formatter { cursor: results.as_results_cursor(body).into(), style, reachable }
     }
 
     fn body(&self) -> &'mir Body<'tcx> {
         self.cursor.borrow().body()
     }
-
-    pub(crate) fn into_results(self) -> Results<'tcx, A> {
-        self.cursor.into_inner().into_results()
-    }
 }
 
 /// A pair of a basic block and an index into that basic blocks `successors`.

compiler/rustc_mir_dataflow/src/framework/mod.rs

@@ -306,14 +306,13 @@ pub trait Analysis<'tcx> {
         let results = Results { analysis: self, entry_sets };
 
         if tcx.sess.opts.unstable_opts.dump_mir_dataflow {
-            let (res, results) = write_graphviz_results(tcx, body, results, pass_name);
+            let res = write_graphviz_results(tcx, body, &results, pass_name);
             if let Err(e) = res {
                 error!("Failed to write graphviz dataflow results: {}", e);
             }
-            results
-        } else {
-            results
         }
+
+        results
     }
 }
 

compiler/rustc_mir_dataflow/src/framework/results.rs

@@ -17,11 +17,11 @@ use super::{Analysis, ResultsCursor, ResultsVisitor, graphviz, visit_results};
 use crate::errors::{
     DuplicateValuesFor, PathMustEndInFilename, RequiresAnArgument, UnknownFormatter,
 };
+use crate::framework::cursor::ResultsHandle;
 
 type EntrySets<'tcx, A> = IndexVec<BasicBlock, <A as Analysis<'tcx>>::Domain>;
 
 /// A dataflow analysis that has converged to fixpoint.
-#[derive(Clone)]
 pub struct Results<'tcx, A>
 where
     A: Analysis<'tcx>,
@@ -34,12 +34,22 @@ impl<'tcx, A> Results<'tcx, A>
 where
     A: Analysis<'tcx>,
 {
-    /// Creates a `ResultsCursor` that can inspect these `Results`.
+    /// Creates a `ResultsCursor` that can inspect these `Results`. Only borrows the `Results`,
+    /// which is appropriate when the `Results` is used outside the cursor.
+    pub fn as_results_cursor<'mir>(
+        &'mir self,
+        body: &'mir mir::Body<'tcx>,
+    ) -> ResultsCursor<'mir, 'tcx, A> {
+        ResultsCursor::new(body, ResultsHandle::Borrowed(self))
+    }
+
+    /// Creates a `ResultsCursor` that can inspect these `Results`. Takes ownership of the
+    /// `Results`, which is good when the `Results` is only used by the cursor.
     pub fn into_results_cursor<'mir>(
         self,
         body: &'mir mir::Body<'tcx>,
     ) -> ResultsCursor<'mir, 'tcx, A> {
-        ResultsCursor::new(body, self)
+        ResultsCursor::new(body, ResultsHandle::Owned(self))
     }
 
     /// Gets the dataflow state for the given block.
@@ -74,9 +84,9 @@ where
 pub(super) fn write_graphviz_results<'tcx, A>(
     tcx: TyCtxt<'tcx>,
     body: &mir::Body<'tcx>,
-    results: Results<'tcx, A>,
+    results: &Results<'tcx, A>,
     pass_name: Option<&'static str>,
-) -> (std::io::Result<()>, Results<'tcx, A>)
+) -> std::io::Result<()>
 where
     A: Analysis<'tcx>,
     A::Domain: DebugWithContext<A>,
@@ -87,7 +97,7 @@ where
     let def_id = body.source.def_id();
     let Ok(attrs) = RustcMirAttrs::parse(tcx, def_id) else {
         // Invalid `rustc_mir` attrs are reported in `RustcMirAttrs::parse`
-        return (Ok(()), results);
+        return Ok(());
     };
 
     let file = try {
@@ -104,12 +114,12 @@ where
                 create_dump_file(tcx, "dot", false, A::NAME, &pass_name.unwrap_or("-----"), body)?
             }
 
-            _ => return (Ok(()), results),
+            _ => return Ok(()),
         }
     };
     let mut file = match file {
         Ok(f) => f,
-        Err(e) => return (Err(e), results),
+        Err(e) => return Err(e),
     };
 
     let style = match attrs.formatter {
@@ -119,7 +129,7 @@ where
 
     let mut buf = Vec::new();
 
-    let graphviz = graphviz::Formatter::new(body, results, style);
+    let graphviz = graphviz::Formatter::new(body, &results, style);
     let mut render_opts =
         vec![dot::RenderOption::Fontname(tcx.sess.opts.unstable_opts.graphviz_font.clone())];
     if tcx.sess.opts.unstable_opts.graphviz_dark_mode {
@@ -132,7 +142,7 @@ where
         file.write_all(&buf)?;
     };
 
-    (lhs, graphviz.into_results())
+    lhs
 }
 
 #[derive(Default)]

compiler/rustc_mir_dataflow/src/impls/storage_liveness.rs

@@ -100,13 +100,13 @@ impl<'a, 'tcx> Analysis<'tcx> for MaybeStorageDead<'a> {
 /// given location; i.e. whether its storage can go away without being observed.
 pub struct MaybeRequiresStorage<'mir, 'tcx> {
     body: &'mir Body<'tcx>,
-    borrowed_locals: Results<'tcx, MaybeBorrowedLocals>,
+    borrowed_locals: &'mir Results<'tcx, MaybeBorrowedLocals>,
 }
 
 impl<'mir, 'tcx> MaybeRequiresStorage<'mir, 'tcx> {
     pub fn new(
         body: &'mir Body<'tcx>,
-        borrowed_locals: Results<'tcx, MaybeBorrowedLocals>,
+        borrowed_locals: &'mir Results<'tcx, MaybeBorrowedLocals>,
     ) -> Self {
         MaybeRequiresStorage { body, borrowed_locals }
     }
@@ -278,7 +278,7 @@ impl<'tcx> Analysis<'tcx> for MaybeRequiresStorage<'_, 'tcx> {
 impl<'tcx> MaybeRequiresStorage<'_, 'tcx> {
     /// Kill locals that are fully moved and have not been borrowed.
     fn check_for_move(&self, trans: &mut <Self as Analysis<'tcx>>::Domain, loc: Location) {
-        let borrowed_locals = self.borrowed_locals.clone().into_results_cursor(self.body);
+        let borrowed_locals = self.borrowed_locals.as_results_cursor(self.body);
         let mut visitor = MoveVisitor { trans, borrowed_locals };
         visitor.visit_location(self.body, loc);
     }

compiler/rustc_mir_transform/src/coroutine.rs

@@ -673,13 +673,12 @@ fn locals_live_across_suspend_points<'tcx>(
     // borrowed (even if they are still active).
     let borrowed_locals_results =
         MaybeBorrowedLocals.iterate_to_fixpoint(tcx, body, Some("coroutine"));
-    let mut borrowed_locals_cursor = borrowed_locals_results.clone().into_results_cursor(body);
+    let mut borrowed_locals_cursor = borrowed_locals_results.as_results_cursor(body);
 
-    // Calculate the MIR locals that we actually need to keep storage around
-    // for.
-    let mut requires_storage_cursor = MaybeRequiresStorage::new(body, borrowed_locals_results)
-        .iterate_to_fixpoint(tcx, body, None)
-        .into_results_cursor(body);
+    // Calculate the MIR locals for which we need to keep storage around.
+    let requires_storage_results = MaybeRequiresStorage::new(body, &borrowed_locals_results)
+        .iterate_to_fixpoint(tcx, body, None);
+    let mut requires_storage_cursor = requires_storage_results.as_results_cursor(body);
 
     // Calculate the liveness of MIR locals ignoring borrows.
     let mut liveness =
@@ -752,7 +751,7 @@ fn locals_live_across_suspend_points<'tcx>(
         body,
         &saved_locals,
         always_live_locals.clone(),
-        requires_storage_cursor.into_results(),
+        requires_storage_results,
     );
 
     LivenessInfo {