Skip to content

Commit a8ce44f

Browse files
committed
Simplify graphviz::Formatter.
`Formatter` currently has a `RefCell<Option<Results>>` field. This is so the `Results` can be temporarily taken and put into a `ResultsCursor` that is used by `BlockFormatter`, and then put back, which is messy. This commit changes `Formatter` to have a `RefCell<ResultsCursor>` and `BlockFormatter` to have a `&mut ResultsCursor`, which greatly simplifies the code at the `Formatter`/`BlockFormatter` interaction point in `Formatter::node_label`. It also means we construct a `ResultsCursor` once per `Formatter`, instead of once per `node_label` call. The commit also: - documents the reason for the `RefCell`; - adds a `Formatter::body` method, replacing the `Formatter::body` field.
1 parent 744eb2f commit a8ce44f

File tree

1 file changed

+26
-27
lines changed

1 file changed

+26
-27
lines changed

compiler/rustc_mir_dataflow/src/framework/graphviz.rs

+26-27
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,11 @@ pub(crate) struct Formatter<'mir, 'tcx, A>
3232
where
3333
A: Analysis<'tcx>,
3434
{
35-
body: &'mir Body<'tcx>,
36-
results: RefCell<Option<Results<'tcx, A>>>,
35+
// The `RefCell` is used because `<Formatter as Labeller>::node_label`
36+
// takes `&self`, but it needs to modify the cursor. This is also the
37+
// reason for the `Formatter`/`BlockFormatter` split; `BlockFormatter` has
38+
// the operations that involve the mutation, i.e. within the `borrow_mut`.
39+
cursor: RefCell<ResultsCursor<'mir, 'tcx, A>>,
3740
style: OutputStyle,
3841
reachable: BitSet<BasicBlock>,
3942
}
@@ -48,11 +51,15 @@ where
4851
style: OutputStyle,
4952
) -> Self {
5053
let reachable = mir::traversal::reachable_as_bitset(body);
51-
Formatter { body, results: Some(results).into(), style, reachable }
54+
Formatter { cursor: results.into_results_cursor(body).into(), style, reachable }
55+
}
56+
57+
fn body(&self) -> &'mir Body<'tcx> {
58+
self.cursor.borrow().body()
5259
}
5360

5461
pub(crate) fn into_results(self) -> Results<'tcx, A> {
55-
self.results.into_inner().unwrap()
62+
self.cursor.into_inner().into_results()
5663
}
5764
}
5865

@@ -81,7 +88,7 @@ where
8188
type Edge = CfgEdge;
8289

8390
fn graph_id(&self) -> dot::Id<'_> {
84-
let name = graphviz_safe_def_name(self.body.source.def_id());
91+
let name = graphviz_safe_def_name(self.body().source.def_id());
8592
dot::Id::new(format!("graph_for_def_id_{name}")).unwrap()
8693
}
8794

@@ -91,19 +98,11 @@ where
9198

9299
fn node_label(&self, block: &Self::Node) -> dot::LabelText<'_> {
93100
let mut label = Vec::new();
94-
self.results.replace_with(|results| {
95-
// `Formatter::result` is a `RefCell<Option<_>>` so we can replace
96-
// the value with `None`, move it into the results cursor, move it
97-
// back out, and return it to the refcell wrapped in `Some`.
98-
let mut fmt = BlockFormatter {
99-
cursor: results.take().unwrap().into_results_cursor(self.body),
100-
style: self.style,
101-
bg: Background::Light,
102-
};
101+
let mut cursor = self.cursor.borrow_mut();
102+
let mut fmt =
103+
BlockFormatter { cursor: &mut cursor, style: self.style, bg: Background::Light };
104+
fmt.write_node_label(&mut label, *block).unwrap();
103105

104-
fmt.write_node_label(&mut label, *block).unwrap();
105-
Some(fmt.cursor.into_results())
106-
});
107106
dot::LabelText::html(String::from_utf8(label).unwrap())
108107
}
109108

@@ -112,20 +111,20 @@ where
112111
}
113112

114113
fn edge_label(&self, e: &Self::Edge) -> dot::LabelText<'_> {
115-
let label = &self.body[e.source].terminator().kind.fmt_successor_labels()[e.index];
114+
let label = &self.body()[e.source].terminator().kind.fmt_successor_labels()[e.index];
116115
dot::LabelText::label(label.clone())
117116
}
118117
}
119118

120-
impl<'mir, 'tcx, A> dot::GraphWalk<'mir> for Formatter<'mir, 'tcx, A>
119+
impl<'tcx, A> dot::GraphWalk<'_> for Formatter<'_, 'tcx, A>
121120
where
122121
A: Analysis<'tcx>,
123122
{
124123
type Node = BasicBlock;
125124
type Edge = CfgEdge;
126125

127126
fn nodes(&self) -> dot::Nodes<'_, Self::Node> {
128-
self.body
127+
self.body()
129128
.basic_blocks
130129
.indices()
131130
.filter(|&idx| self.reachable.contains(idx))
@@ -134,10 +133,10 @@ where
134133
}
135134

136135
fn edges(&self) -> dot::Edges<'_, Self::Edge> {
137-
self.body
138-
.basic_blocks
136+
let body = self.body();
137+
body.basic_blocks
139138
.indices()
140-
.flat_map(|bb| dataflow_successors(self.body, bb))
139+
.flat_map(|bb| dataflow_successors(body, bb))
141140
.collect::<Vec<_>>()
142141
.into()
143142
}
@@ -147,20 +146,20 @@ where
147146
}
148147

149148
fn target(&self, edge: &Self::Edge) -> Self::Node {
150-
self.body[edge.source].terminator().successors().nth(edge.index).unwrap()
149+
self.body()[edge.source].terminator().successors().nth(edge.index).unwrap()
151150
}
152151
}
153152

154-
struct BlockFormatter<'mir, 'tcx, A>
153+
struct BlockFormatter<'a, 'mir, 'tcx, A>
155154
where
156155
A: Analysis<'tcx>,
157156
{
158-
cursor: ResultsCursor<'mir, 'tcx, A>,
157+
cursor: &'a mut ResultsCursor<'mir, 'tcx, A>,
159158
bg: Background,
160159
style: OutputStyle,
161160
}
162161

163-
impl<'mir, 'tcx, A> BlockFormatter<'mir, 'tcx, A>
162+
impl<'tcx, A> BlockFormatter<'_, '_, 'tcx, A>
164163
where
165164
A: Analysis<'tcx>,
166165
A::Domain: DebugWithContext<A>,

0 commit comments

Comments
 (0)