coverage: Treat the "merged node flow graph" as a plain data struct

By removing all methods from this struct and treating it as a collection of
data fields, we make it easier for a future PR to store that data in a query
result, without having to move all of its methods into `rustc_middle`.

Author: Zalathar

compiler/rustc_mir_transform/src/coverage/counters.rs

@@ -11,7 +11,9 @@ use rustc_middle::mir::coverage::{CounterId, CovTerm, Expression, ExpressionId,
 
 use crate::coverage::counters::balanced_flow::BalancedFlowGraph;
 use crate::coverage::counters::iter_nodes::IterNodes;
-use crate::coverage::counters::node_flow::{CounterTerm, MergedNodeFlowGraph, NodeCounters};
+use crate::coverage::counters::node_flow::{
+    CounterTerm, NodeCounters, make_node_counters, node_flow_data_for_balanced_graph,
+};
 use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph};
 
 mod balanced_flow;
@@ -27,20 +29,20 @@ pub(super) fn make_bcb_counters(
 ) -> CoverageCounters {
     // Create the derived graphs that are necessary for subsequent steps.
     let balanced_graph = BalancedFlowGraph::for_graph(graph, |n| !graph[n].is_out_summable);
-    let merged_graph = MergedNodeFlowGraph::for_balanced_graph(&balanced_graph);
+    let node_flow_data = node_flow_data_for_balanced_graph(&balanced_graph);
 
     // Use those graphs to determine which nodes get physical counters, and how
     // to compute the execution counts of other nodes from those counters.
-    let nodes = make_node_counter_priority_list(graph, balanced_graph);
-    let node_counters = merged_graph.make_node_counters(&nodes);
+    let priority_list = make_node_flow_priority_list(graph, balanced_graph);
+    let node_counters = make_node_counters(&node_flow_data, &priority_list);
 
     // Convert the counters into a form suitable for embedding into MIR.
     transcribe_counters(&node_counters, bcb_needs_counter)
 }
 
 /// Arranges the nodes in `balanced_graph` into a list, such that earlier nodes
 /// take priority in being given a counter expression instead of a physical counter.
-fn make_node_counter_priority_list(
+fn make_node_flow_priority_list(
     graph: &CoverageGraph,
     balanced_graph: BalancedFlowGraph<&CoverageGraph>,
 ) -> Vec<BasicCoverageBlock> {

compiler/rustc_mir_transform/src/coverage/counters/node_flow.rs

@@ -8,7 +8,7 @@
 
 use rustc_data_structures::graph;
 use rustc_index::bit_set::DenseBitSet;
-use rustc_index::{Idx, IndexVec};
+use rustc_index::{Idx, IndexSlice, IndexVec};
 use rustc_middle::mir::coverage::Op;
 
 use crate::coverage::counters::iter_nodes::IterNodes;
@@ -17,8 +17,8 @@ use crate::coverage::counters::union_find::UnionFind;
 #[cfg(test)]
 mod tests;
 
-/// View of some underlying graph, in which each node's successors have been
-/// merged into a single "supernode".
+/// Data representing a view of some underlying graph, in which each node's
+/// successors have been merged into a single "supernode".
 ///
 /// The resulting supernodes have no obvious meaning on their own.
 /// However, merging successor nodes means that a node's out-edges can all
@@ -29,7 +29,7 @@ mod tests;
 /// in the merged graph, it becomes possible to analyze the original node flows
 /// using techniques for analyzing edge flows.
 #[derive(Debug)]
-pub(crate) struct MergedNodeFlowGraph<Node: Idx> {
+pub(crate) struct NodeFlowData<Node: Idx> {
     /// Maps each node to the supernode that contains it, indicated by some
     /// arbitrary "root" node that is part of that supernode.
     supernodes: IndexVec<Node, Node>,
@@ -41,66 +41,59 @@ pub(crate) struct MergedNodeFlowGraph<Node: Idx> {
     succ_supernodes: IndexVec<Node, Node>,
 }
 
-impl<Node: Idx> MergedNodeFlowGraph<Node> {
-    /// Creates a "merged" view of an underlying graph.
-    ///
-    /// The given graph is assumed to have [“balanced flow”](balanced-flow),
-    /// though it does not necessarily have to be a `BalancedFlowGraph`.
-    ///
-    /// [balanced-flow]: `crate::coverage::counters::balanced_flow::BalancedFlowGraph`.
-    pub(crate) fn for_balanced_graph<G>(graph: G) -> Self
-    where
-        G: graph::DirectedGraph<Node = Node> + graph::Successors,
-    {
-        let mut supernodes = UnionFind::<G::Node>::new(graph.num_nodes());
-
-        // For each node, merge its successors into a single supernode, and
-        // arbitrarily choose one of those successors to represent all of them.
-        let successors = graph
-            .iter_nodes()
-            .map(|node| {
-                graph
-                    .successors(node)
-                    .reduce(|a, b| supernodes.unify(a, b))
-                    .expect("each node in a balanced graph must have at least one out-edge")
-            })
-            .collect::<IndexVec<G::Node, G::Node>>();
-
-        // Now that unification is complete, take a snapshot of the supernode forest,
-        // and resolve each arbitrarily-chosen successor to its canonical root.
-        // (This avoids having to explicitly resolve them later.)
-        let supernodes = supernodes.snapshot();
-        let succ_supernodes = successors.into_iter().map(|succ| supernodes[succ]).collect();
-
-        Self { supernodes, succ_supernodes }
-    }
-
-    fn num_nodes(&self) -> usize {
-        self.succ_supernodes.len()
-    }
+/// Creates a "merged" view of an underlying graph.
+///
+/// The given graph is assumed to have [“balanced flow”](balanced-flow),
+/// though it does not necessarily have to be a `BalancedFlowGraph`.
+///
+/// [balanced-flow]: `crate::coverage::counters::balanced_flow::BalancedFlowGraph`.
+pub(crate) fn node_flow_data_for_balanced_graph<G>(graph: G) -> NodeFlowData<G::Node>
+where
+    G: graph::Successors,
+{
+    let mut supernodes = UnionFind::<G::Node>::new(graph.num_nodes());
+
+    // For each node, merge its successors into a single supernode, and
+    // arbitrarily choose one of those successors to represent all of them.
+    let successors = graph
+        .iter_nodes()
+        .map(|node| {
+            graph
+                .successors(node)
+                .reduce(|a, b| supernodes.unify(a, b))
+                .expect("each node in a balanced graph must have at least one out-edge")
+        })
+        .collect::<IndexVec<G::Node, G::Node>>();
+
+    // Now that unification is complete, take a snapshot of the supernode forest,
+    // and resolve each arbitrarily-chosen successor to its canonical root.
+    // (This avoids having to explicitly resolve them later.)
+    let supernodes = supernodes.snapshot();
+    let succ_supernodes = successors.into_iter().map(|succ| supernodes[succ]).collect();
+
+    NodeFlowData { supernodes, succ_supernodes }
+}
 
-    fn is_supernode(&self, node: Node) -> bool {
-        self.supernodes[node] == node
+/// Uses the graph information in `node_flow_data`, together with a given
+/// permutation of all nodes in the graph, to create physical counters and
+/// counter expressions for each node in the underlying graph.
+///
+/// The given list must contain exactly one copy of each node in the
+/// underlying balanced-flow graph. The order of nodes is used as a hint to
+/// influence counter allocation:
+/// - Earlier nodes are more likely to receive counter expressions.
+/// - Later nodes are more likely to receive physical counters.
+pub(crate) fn make_node_counters<Node: Idx>(
+    node_flow_data: &NodeFlowData<Node>,
+    priority_list: &[Node],
+) -> NodeCounters<Node> {
+    let mut builder = SpantreeBuilder::new(node_flow_data);
+
+    for &node in priority_list {
+        builder.visit_node(node);
     }
 
-    /// Using the information in this merged graph, together with a given
-    /// permutation of all nodes in the graph, to create physical counters and
-    /// counter expressions for each node in the underlying graph.
-    ///
-    /// The given list must contain exactly one copy of each node in the
-    /// underlying balanced-flow graph. The order of nodes is used as a hint to
-    /// influence counter allocation:
-    /// - Earlier nodes are more likely to receive counter expressions.
-    /// - Later nodes are more likely to receive physical counters.
-    pub(crate) fn make_node_counters(&self, all_nodes_permutation: &[Node]) -> NodeCounters<Node> {
-        let mut builder = SpantreeBuilder::new(self);
-
-        for &node in all_nodes_permutation {
-            builder.visit_node(node);
-        }
-
-        NodeCounters { counter_terms: builder.finish() }
-    }
+    NodeCounters { counter_terms: builder.finish() }
 }
 
 /// End result of allocating physical counters and counter expressions for the
@@ -141,7 +134,9 @@ pub(crate) struct CounterTerm<Node> {
 
 #[derive(Debug)]
 struct SpantreeBuilder<'a, Node: Idx> {
-    graph: &'a MergedNodeFlowGraph<Node>,
+    supernodes: &'a IndexSlice<Node, Node>,
+    succ_supernodes: &'a IndexSlice<Node, Node>,
+
     is_unvisited: DenseBitSet<Node>,
     /// Links supernodes to each other, gradually forming a spanning tree of
     /// the merged-flow graph.
@@ -157,22 +152,28 @@ struct SpantreeBuilder<'a, Node: Idx> {
 }
 
 impl<'a, Node: Idx> SpantreeBuilder<'a, Node> {
-    fn new(graph: &'a MergedNodeFlowGraph<Node>) -> Self {
-        let num_nodes = graph.num_nodes();
+    fn new(node_flow_data: &'a NodeFlowData<Node>) -> Self {
+        let NodeFlowData { supernodes, succ_supernodes } = node_flow_data;
+        let num_nodes = supernodes.len();
         Self {
-            graph,
+            supernodes,
+            succ_supernodes,
             is_unvisited: DenseBitSet::new_filled(num_nodes),
             span_edges: IndexVec::from_fn_n(|_| None, num_nodes),
             yank_buffer: vec![],
             counter_terms: IndexVec::from_fn_n(|_| vec![], num_nodes),
         }
     }
 
+    fn is_supernode(&self, node: Node) -> bool {
+        self.supernodes[node] == node
+    }
+
     /// Given a supernode, finds the supernode that is the "root" of its
     /// spantree component. Two nodes that have the same spantree root are
     /// connected in the spantree.
     fn spantree_root(&self, this: Node) -> Node {
-        debug_assert!(self.graph.is_supernode(this));
+        debug_assert!(self.is_supernode(this));
 
         match self.span_edges[this] {
             None => this,
@@ -183,7 +184,7 @@ impl<'a, Node: Idx> SpantreeBuilder<'a, Node> {
     /// Rotates edges in the spantree so that `this` is the root of its
     /// spantree component.
     fn yank_to_spantree_root(&mut self, this: Node) {
-        debug_assert!(self.graph.is_supernode(this));
+        debug_assert!(self.is_supernode(this));
 
         // The rotation is done iteratively, by first traversing from `this` to
         // its root and storing the path in a buffer, and then traversing the
@@ -225,12 +226,12 @@ impl<'a, Node: Idx> SpantreeBuilder<'a, Node> {
 
         // Get the supernode containing `this`, and make it the root of its
         // component of the spantree.
-        let this_supernode = self.graph.supernodes[this];
+        let this_supernode = self.supernodes[this];
         self.yank_to_spantree_root(this_supernode);
 
         // Get the supernode containing all of this's successors.
-        let succ_supernode = self.graph.succ_supernodes[this];
-        debug_assert!(self.graph.is_supernode(succ_supernode));
+        let succ_supernode = self.succ_supernodes[this];
+        debug_assert!(self.is_supernode(succ_supernode));
 
         // If two supernodes are already connected in the spantree, they will
         // have the same spantree root. (Each supernode is connected to itself.)
@@ -279,18 +280,19 @@ impl<'a, Node: Idx> SpantreeBuilder<'a, Node> {
     /// Asserts that all nodes have been visited, and returns the computed
     /// counter expressions (made up of physical counters) for each node.
     fn finish(self) -> IndexVec<Node, Vec<CounterTerm<Node>>> {
-        let Self { graph, is_unvisited, span_edges, yank_buffer: _, counter_terms } = self;
+        let Self { ref span_edges, ref is_unvisited, ref counter_terms, .. } = self;
         assert!(is_unvisited.is_empty(), "some nodes were never visited: {is_unvisited:?}");
         debug_assert!(
             span_edges
                 .iter_enumerated()
-                .all(|(node, span_edge)| { span_edge.is_some() <= graph.is_supernode(node) }),
+                .all(|(node, span_edge)| { span_edge.is_some() <= self.is_supernode(node) }),
             "only supernodes can have a span edge",
         );
         debug_assert!(
             counter_terms.iter().all(|terms| !terms.is_empty()),
             "after visiting all nodes, every node should have at least one term",
         );
-        counter_terms
+
+        self.counter_terms
     }
 }

compiler/rustc_mir_transform/src/coverage/counters/node_flow/tests.rs

@@ -4,10 +4,12 @@ use rustc_data_structures::graph::vec_graph::VecGraph;
 use rustc_index::Idx;
 use rustc_middle::mir::coverage::Op;
 
-use super::{CounterTerm, MergedNodeFlowGraph, NodeCounters};
+use crate::coverage::counters::node_flow::{
+    CounterTerm, NodeCounters, NodeFlowData, make_node_counters, node_flow_data_for_balanced_graph,
+};
 
-fn merged_node_flow_graph<G: graph::Successors>(graph: G) -> MergedNodeFlowGraph<G::Node> {
-    MergedNodeFlowGraph::for_balanced_graph(graph)
+fn node_flow_data<G: graph::Successors>(graph: G) -> NodeFlowData<G::Node> {
+    node_flow_data_for_balanced_graph(graph)
 }
 
 fn make_graph<Node: Idx + Ord>(num_nodes: usize, edge_pairs: Vec<(Node, Node)>) -> VecGraph<Node> {
@@ -30,8 +32,8 @@ fn example_driver() {
         (4, 0),
     ]);
 
-    let merged = merged_node_flow_graph(&graph);
-    let counters = merged.make_node_counters(&[3, 1, 2, 0, 4]);
+    let node_flow_data = node_flow_data(&graph);
+    let counters = make_node_counters(&node_flow_data, &[3, 1, 2, 0, 4]);
 
     assert_eq!(format_counter_expressions(&counters), &[
         // (comment to force vertical formatting for clarity)