Skip to content

Commit

Permalink
Make it possible for ResultsCursor to borrow a Results.
Browse files Browse the repository at this point in the history
`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` and `Formatter::into_results` are
  removed.
- `write_graphviz_results` now just borrows a `Results`, instead of the
  awkward "take ownership of a `Results` and then return it unchanged"
  pattern.

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!
  • Loading branch information
nnethercote committed Nov 18, 2024
1 parent ec5d6e7 commit a47e638
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 31 deletions.
51 changes: 44 additions & 7 deletions compiler/rustc_mir_dataflow/src/framework/cursor.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,55 @@
//! Random access inspection of the results of a dataflow analysis.
use std::cmp::Ordering;
use std::ops::{Deref, DerefMut};

#[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`, either
/// mutable or immutably. This type allows all of the above. It's similar to `Cow`.
pub enum ResultsHandle<'a, 'tcx, A>
where
A: Analysis<'tcx>,
{
Borrowed(&'a Results<'tcx, A>),
BorrowedMut(&'a mut 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::BorrowedMut(borrowed) => borrowed,
ResultsHandle::Owned(owned) => owned,
}
}
}

impl<'tcx, A> DerefMut for ResultsHandle<'_, 'tcx, A>
where
A: Analysis<'tcx>,
{
fn deref_mut(&mut self) -> &mut Results<'tcx, A> {
match self {
ResultsHandle::Borrowed(_borrowed) => {
panic!("tried to deref_mut a `ResultsHandle::Borrowed")
}
ResultsHandle::BorrowedMut(borrowed) => borrowed,
ResultsHandle::Owned(owned) => owned,
}
}
}

/// Allows random access inspection of the results of a dataflow analysis. Use this when you want
/// to inspect domain values only in certain locations; use `ResultsVisitor` if you want to inspect
/// domain values in many or all locations.
Expand All @@ -23,7 +65,7 @@ where
A: Analysis<'tcx>,
{
body: &'mir mir::Body<'tcx>,
results: Results<'tcx, A>,
results: ResultsHandle<'mir, 'tcx, A>,
state: A::Domain,

pos: CursorPosition,
Expand Down Expand Up @@ -51,13 +93,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,
Expand Down
8 changes: 2 additions & 6 deletions compiler/rustc_mir_dataflow/src/framework/graphviz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_mir_dataflow/src/framework/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,14 +302,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
}
}

Expand Down
35 changes: 27 additions & 8 deletions compiler/rustc_mir_dataflow/src/framework/results.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use super::{Analysis, ResultsCursor, ResultsVisitor, graphviz, visit_results};
use crate::errors::{
DuplicateValuesFor, PathMustEndInFilename, RequiresAnArgument, UnknownFormatter,
};
use crate::framework::cursor::ResultsHandle;

pub type EntrySets<'tcx, A> = IndexVec<BasicBlock, <A as Analysis<'tcx>>::Domain>;

Expand All @@ -36,12 +37,30 @@ 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`. Immutably 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 mutate these `Results`. Mutably borrows the `Results`,
/// which is appropriate when the `Results` is used outside the cursor.
pub fn as_results_cursor_mut<'mir>(
&'mir mut self,
body: &'mir mir::Body<'tcx>,
) -> ResultsCursor<'mir, 'tcx, A> {
ResultsCursor::new(body, ResultsHandle::BorrowedMut(self))
}

/// Creates a `ResultsCursor` that takes ownership of the `Results`.
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.
Expand Down Expand Up @@ -76,9 +95,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>,
Expand All @@ -89,7 +108,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 {
Expand All @@ -106,12 +125,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 {
Expand All @@ -134,7 +153,7 @@ where
file.write_all(&buf)?;
};

(lhs, graphviz.into_results())
lhs
}

#[derive(Default)]
Expand Down
11 changes: 5 additions & 6 deletions compiler/rustc_mir_transform/src/coroutine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -676,12 +676,11 @@ fn locals_live_across_suspend_points<'tcx>(

let mut borrowed_locals_cursor = borrowed_locals_results.clone().into_results_cursor(body);

// Calculate the MIR locals that we actually need to keep storage around
// for.
let mut requires_storage_cursor =
// Calculate the MIR locals that we need to keep storage around for.
let mut requires_storage_results =
MaybeRequiresStorage::new(borrowed_locals_results.into_results_cursor(body))
.iterate_to_fixpoint(tcx, body, None)
.into_results_cursor(body);
.iterate_to_fixpoint(tcx, body, None);
let mut requires_storage_cursor = requires_storage_results.as_results_cursor_mut(body);

// Calculate the liveness of MIR locals ignoring borrows.
let mut liveness =
Expand Down Expand Up @@ -754,7 +753,7 @@ fn locals_live_across_suspend_points<'tcx>(
body,
&saved_locals,
always_live_locals.clone(),
requires_storage_cursor.into_results(),
requires_storage_results,
);

LivenessInfo {
Expand Down

0 comments on commit a47e638

Please sign in to comment.