1
Fork 0

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` 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!
This commit is contained in:
Nicholas Nethercote 2024-11-18 16:04:03 +11:00
parent 1914dbe694
commit be7c6a3b43
5 changed files with 81 additions and 31 deletions

View file

@ -1,6 +1,7 @@
//! Random access inspection of the results of a dataflow analysis. //! Random access inspection of the results of a dataflow analysis.
use std::cmp::Ordering; use std::cmp::Ordering;
use std::ops::{Deref, DerefMut};
#[cfg(debug_assertions)] #[cfg(debug_assertions)]
use rustc_index::bit_set::BitSet; use rustc_index::bit_set::BitSet;
@ -8,6 +9,47 @@ use rustc_middle::mir::{self, BasicBlock, Location};
use super::{Analysis, Direction, Effect, EffectIndex, Results}; 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 /// 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 /// to inspect domain values only in certain locations; use `ResultsVisitor` if you want to inspect
/// domain values in many or all locations. /// domain values in many or all locations.
@ -23,7 +65,7 @@ where
A: Analysis<'tcx>, A: Analysis<'tcx>,
{ {
body: &'mir mir::Body<'tcx>, body: &'mir mir::Body<'tcx>,
results: Results<'tcx, A>, results: ResultsHandle<'mir, 'tcx, A>,
state: A::Domain, state: A::Domain,
pos: CursorPosition, pos: CursorPosition,
@ -51,13 +93,8 @@ where
self.body 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`. /// 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); let bottom_value = results.analysis.bottom_value(body);
ResultsCursor { ResultsCursor {
body, body,

View file

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

View file

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

View file

@ -17,6 +17,7 @@ use super::{Analysis, ResultsCursor, ResultsVisitor, graphviz, visit_results};
use crate::errors::{ use crate::errors::{
DuplicateValuesFor, PathMustEndInFilename, RequiresAnArgument, UnknownFormatter, DuplicateValuesFor, PathMustEndInFilename, RequiresAnArgument, UnknownFormatter,
}; };
use crate::framework::cursor::ResultsHandle;
pub type EntrySets<'tcx, A> = IndexVec<BasicBlock, <A as Analysis<'tcx>>::Domain>; pub type EntrySets<'tcx, A> = IndexVec<BasicBlock, <A as Analysis<'tcx>>::Domain>;
@ -36,12 +37,30 @@ impl<'tcx, A> Results<'tcx, A>
where where
A: Analysis<'tcx>, 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>( pub fn into_results_cursor<'mir>(
self, self,
body: &'mir mir::Body<'tcx>, body: &'mir mir::Body<'tcx>,
) -> ResultsCursor<'mir, 'tcx, A> { ) -> ResultsCursor<'mir, 'tcx, A> {
ResultsCursor::new(body, self) ResultsCursor::new(body, ResultsHandle::Owned(self))
} }
/// Gets the dataflow state for the given block. /// Gets the dataflow state for the given block.
@ -76,9 +95,9 @@ where
pub(super) fn write_graphviz_results<'tcx, A>( pub(super) fn write_graphviz_results<'tcx, A>(
tcx: TyCtxt<'tcx>, tcx: TyCtxt<'tcx>,
body: &mir::Body<'tcx>, body: &mir::Body<'tcx>,
results: Results<'tcx, A>, results: &Results<'tcx, A>,
pass_name: Option<&'static str>, pass_name: Option<&'static str>,
) -> (std::io::Result<()>, Results<'tcx, A>) ) -> std::io::Result<()>
where where
A: Analysis<'tcx>, A: Analysis<'tcx>,
A::Domain: DebugWithContext<A>, A::Domain: DebugWithContext<A>,
@ -89,7 +108,7 @@ where
let def_id = body.source.def_id(); let def_id = body.source.def_id();
let Ok(attrs) = RustcMirAttrs::parse(tcx, def_id) else { let Ok(attrs) = RustcMirAttrs::parse(tcx, def_id) else {
// Invalid `rustc_mir` attrs are reported in `RustcMirAttrs::parse` // Invalid `rustc_mir` attrs are reported in `RustcMirAttrs::parse`
return (Ok(()), results); return Ok(());
}; };
let file = try { let file = try {
@ -106,12 +125,12 @@ where
create_dump_file(tcx, "dot", false, A::NAME, &pass_name.unwrap_or("-----"), body)? create_dump_file(tcx, "dot", false, A::NAME, &pass_name.unwrap_or("-----"), body)?
} }
_ => return (Ok(()), results), _ => return Ok(()),
} }
}; };
let mut file = match file { let mut file = match file {
Ok(f) => f, Ok(f) => f,
Err(e) => return (Err(e), results), Err(e) => return Err(e),
}; };
let style = match attrs.formatter { let style = match attrs.formatter {
@ -134,7 +153,7 @@ where
file.write_all(&buf)?; file.write_all(&buf)?;
}; };
(lhs, graphviz.into_results()) lhs
} }
#[derive(Default)] #[derive(Default)]

View file

@ -676,12 +676,11 @@ fn locals_live_across_suspend_points<'tcx>(
let mut borrowed_locals_cursor = borrowed_locals_results.clone().into_results_cursor(body); 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 // Calculate the MIR locals that we need to keep storage around for.
// for. let mut requires_storage_results =
let mut requires_storage_cursor =
MaybeRequiresStorage::new(borrowed_locals_results.into_results_cursor(body)) MaybeRequiresStorage::new(borrowed_locals_results.into_results_cursor(body))
.iterate_to_fixpoint(tcx, body, None) .iterate_to_fixpoint(tcx, body, None);
.into_results_cursor(body); let mut requires_storage_cursor = requires_storage_results.as_results_cursor_mut(body);
// Calculate the liveness of MIR locals ignoring borrows. // Calculate the liveness of MIR locals ignoring borrows.
let mut liveness = let mut liveness =
@ -754,7 +753,7 @@ fn locals_live_across_suspend_points<'tcx>(
body, body,
&saved_locals, &saved_locals,
always_live_locals.clone(), always_live_locals.clone(),
requires_storage_cursor.into_results(), requires_storage_results,
); );
LivenessInfo { LivenessInfo {