Auto merge of #139965 - amandasystems:marginally-improved-scc-annotations, r=lcnr

Decouple SCC annotations from SCCs

This rewires SCC annotations to have them be a separate, visitor-type data structure. It was broken out of #130227, which needed them to be able to remove unused annotations after computation without recomputing the SCCs themselves.

As a drive-by it also removes some redundant code from the hot loop in SCC construction for a performance improvement.

r? lcnr

Author: bors

compiler/rustc_borrowck/src/constraints/mod.rs

@@ -7,7 +7,7 @@ use rustc_middle::ty::{RegionVid, TyCtxt, VarianceDiagInfo};
 use rustc_span::Span;
 use tracing::{debug, instrument};
 
-use crate::region_infer::{ConstraintSccs, RegionDefinition, RegionTracker};
+use crate::region_infer::{AnnotatedSccs, ConstraintSccs, RegionDefinition, SccAnnotations};
 use crate::type_check::Locations;
 use crate::universal_regions::UniversalRegions;
 
@@ -61,12 +61,14 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {
         &self,
         static_region: RegionVid,
         definitions: &IndexVec<RegionVid, RegionDefinition<'tcx>>,
-    ) -> ConstraintSccs {
+    ) -> AnnotatedSccs {
         let constraint_graph = self.graph(definitions.len());
         let region_graph = &constraint_graph.region_graph(self, static_region);
-        ConstraintSccs::new_with_annotation(&region_graph, |r| {
-            RegionTracker::new(r, &definitions[r])
-        })
+        let mut annotation_visitor = SccAnnotations::new(definitions);
+        (
+            ConstraintSccs::new_with_annotation(&region_graph, &mut annotation_visitor),
+            annotation_visitor.scc_to_annotation,
+        )
     }
 
     /// This method handles Universe errors by rewriting the constraint
@@ -79,12 +81,12 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {
     /// eventually go away.
     ///
     /// For a more precise definition, see the documentation for
-    /// [`RegionTracker::has_incompatible_universes()`].
+    /// [`crate::region_infer::RegionTracker`].
     ///
     /// This edge case used to be handled during constraint propagation
     /// by iterating over the strongly connected components in the constraint
     /// graph while maintaining a set of bookkeeping mappings similar
-    /// to what is stored in `RegionTracker` and manually adding 'sttaic as
+    /// to what is stored in `RegionTracker` and manually adding 'static as
     /// needed.
     ///
     /// It was rewritten as part of the Polonius project with the goal of moving
@@ -108,9 +110,9 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {
         &mut self,
         universal_regions: &UniversalRegions<'tcx>,
         definitions: &IndexVec<RegionVid, RegionDefinition<'tcx>>,
-    ) -> ConstraintSccs {
+    ) -> AnnotatedSccs {
         let fr_static = universal_regions.fr_static;
-        let sccs = self.compute_sccs(fr_static, definitions);
+        let (sccs, annotations) = self.compute_sccs(fr_static, definitions);
 
         // Changed to `true` if we added any constraints to `self` and need to
         // recompute SCCs.
@@ -124,7 +126,7 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {
                 continue;
             }
 
-            let annotation = sccs.annotation(scc);
+            let annotation = annotations[scc];
 
             // If this SCC participates in a universe violation,
             // e.g. if it reaches a region with a universe smaller than
@@ -154,7 +156,7 @@ impl<'tcx> OutlivesConstraintSet<'tcx> {
             self.compute_sccs(fr_static, definitions)
         } else {
             // If we didn't add any back-edges; no more work needs doing
-            sccs
+            (sccs, annotations)
         }
     }
 }

compiler/rustc_borrowck/src/region_infer/mod.rs

@@ -47,12 +47,13 @@ mod reverse_sccs;
 
 pub(crate) mod values;
 
-pub(crate) type ConstraintSccs = Sccs<RegionVid, ConstraintSccIndex, RegionTracker>;
+pub(crate) type ConstraintSccs = Sccs<RegionVid, ConstraintSccIndex>;
+pub(crate) type AnnotatedSccs = (ConstraintSccs, IndexVec<ConstraintSccIndex, RegionTracker>);
 
 /// An annotation for region graph SCCs that tracks
-/// the values of its elements.
+/// the values of its elements. This annotates a single SCC.
 #[derive(Copy, Debug, Clone)]
-pub struct RegionTracker {
+pub(crate) struct RegionTracker {
     /// The largest universe of a placeholder reached from this SCC.
     /// This includes placeholders within this SCC.
     max_placeholder_universe_reached: UniverseIndex,
@@ -97,6 +98,32 @@ impl scc::Annotation for RegionTracker {
     }
 }
 
+/// A Visitor for SCC annotation construction.
+pub(crate) struct SccAnnotations<'d, 'tcx, A: scc::Annotation> {
+    pub(crate) scc_to_annotation: IndexVec<ConstraintSccIndex, A>,
+    definitions: &'d IndexVec<RegionVid, RegionDefinition<'tcx>>,
+}
+
+impl<'d, 'tcx, A: scc::Annotation> SccAnnotations<'d, 'tcx, A> {
+    pub(crate) fn new(definitions: &'d IndexVec<RegionVid, RegionDefinition<'tcx>>) -> Self {
+        Self { scc_to_annotation: IndexVec::new(), definitions }
+    }
+}
+
+impl scc::Annotations<RegionVid> for SccAnnotations<'_, '_, RegionTracker> {
+    fn new(&self, element: RegionVid) -> RegionTracker {
+        RegionTracker::new(element, &self.definitions[element])
+    }
+
+    fn annotate_scc(&mut self, scc: ConstraintSccIndex, annotation: RegionTracker) {
+        let idx = self.scc_to_annotation.push(annotation);
+        assert!(idx == scc);
+    }
+
+    type Ann = RegionTracker;
+    type SccIdx = ConstraintSccIndex;
+}
+
 impl RegionTracker {
     pub(crate) fn new(rvid: RegionVid, definition: &RegionDefinition<'_>) -> Self {
         let (representative_is_placeholder, representative_is_existential) = match definition.origin
@@ -166,6 +193,8 @@ pub struct RegionInferenceContext<'tcx> {
     /// compute the values of each region.
     constraint_sccs: ConstraintSccs,
 
+    scc_annotations: IndexVec<ConstraintSccIndex, RegionTracker>,
+
     /// Reverse of the SCC constraint graph --  i.e., an edge `A -> B` exists if
     /// `B: A`. This is used to compute the universal regions that are required
     /// to outlive a given SCC. Computed lazily.
@@ -446,7 +475,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
 
         let definitions = create_definitions(infcx, &universal_regions);
 
-        let constraint_sccs =
+        let (constraint_sccs, scc_annotations) =
             outlives_constraints.add_outlives_static(&universal_regions, &definitions);
         let constraints = Frozen::freeze(outlives_constraints);
         let constraint_graph = Frozen::freeze(constraints.graph(definitions.len()));
@@ -472,6 +501,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             constraints,
             constraint_graph,
             constraint_sccs,
+            scc_annotations,
             rev_scc_graph: None,
             member_constraints,
             member_constraints_applied: Vec::new(),
@@ -798,7 +828,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
 
         // If the member region lives in a higher universe, we currently choose
         // the most conservative option by leaving it unchanged.
-        if !self.constraint_sccs().annotation(scc).min_universe().is_root() {
+        if !self.scc_universe(scc).is_root() {
             return;
         }
 
@@ -874,8 +904,8 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     /// in `scc_a`. Used during constraint propagation, and only once
     /// the value of `scc_b` has been computed.
     fn universe_compatible(&self, scc_b: ConstraintSccIndex, scc_a: ConstraintSccIndex) -> bool {
-        let a_annotation = self.constraint_sccs().annotation(scc_a);
-        let b_annotation = self.constraint_sccs().annotation(scc_b);
+        let a_annotation = self.scc_annotations[scc_a];
+        let b_annotation = self.scc_annotations[scc_b];
         let a_universe = a_annotation.min_universe();
 
         // If scc_b's declared universe is a subset of
@@ -991,7 +1021,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             "lower_bound = {:?} r_scc={:?} universe={:?}",
             lower_bound,
             r_scc,
-            self.constraint_sccs.annotation(r_scc).min_universe()
+            self.scc_universe(r_scc)
         );
         // If the type test requires that `T: 'a` where `'a` is a
         // placeholder from another universe, that effectively requires
@@ -1472,7 +1502,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     /// The minimum universe of any variable reachable from this
     /// SCC, inside or outside of it.
     fn scc_universe(&self, scc: ConstraintSccIndex) -> UniverseIndex {
-        self.constraint_sccs().annotation(scc).min_universe()
+        self.scc_annotations[scc].min_universe()
     }
 
     /// Checks the final value for the free region `fr` to see if it
@@ -2216,7 +2246,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     /// they *must* be equal (though not having the same repr does not
     /// mean they are unequal).
     fn scc_representative(&self, scc: ConstraintSccIndex) -> RegionVid {
-        self.constraint_sccs.annotation(scc).representative
+        self.scc_annotations[scc].representative
     }
 
     pub(crate) fn liveness_constraints(&self) -> &LivenessValues {

compiler/rustc_data_structures/src/graph/scc/mod.rs

@@ -10,10 +10,11 @@
 
 use std::assert_matches::debug_assert_matches;
 use std::fmt::Debug;
+use std::marker::PhantomData;
 use std::ops::Range;
 
 use rustc_index::{Idx, IndexSlice, IndexVec};
-use tracing::{debug, instrument};
+use tracing::{debug, instrument, trace};
 
 use crate::fx::FxHashSet;
 use crate::graph::vec_graph::VecGraph;
@@ -48,6 +49,25 @@ pub trait Annotation: Debug + Copy {
     }
 }
 
+/// An accumulator for annotations.
+pub trait Annotations<N: Idx> {
+    type Ann: Annotation;
+    type SccIdx: Idx + Ord;
+
+    fn new(&self, element: N) -> Self::Ann;
+    fn annotate_scc(&mut self, scc: Self::SccIdx, annotation: Self::Ann);
+}
+
+/// The nil annotation accumulator, which does nothing.
+struct NoAnnotations<S: Idx + Ord>(PhantomData<S>);
+
+impl<N: Idx, S: Idx + Ord> Annotations<N> for NoAnnotations<S> {
+    type SccIdx = S;
+    type Ann = ();
+    fn new(&self, _element: N) {}
+    fn annotate_scc(&mut self, _scc: S, _annotation: ()) {}
+}
+
 /// The empty annotation, which does nothing.
 impl Annotation for () {
     fn merge_reached(self, _other: Self) -> Self {
@@ -62,23 +82,20 @@ impl Annotation for () {
 /// the index type for the graph nodes and `S` is the index type for
 /// the SCCs. We can map from each node to the SCC that it
 /// participates in, and we also have the successors of each SCC.
-pub struct Sccs<N: Idx, S: Idx, A: Annotation = ()> {
+pub struct Sccs<N: Idx, S: Idx> {
     /// For each node, what is the SCC index of the SCC to which it
     /// belongs.
     scc_indices: IndexVec<N, S>,
 
     /// Data about all the SCCs.
-    scc_data: SccData<S, A>,
+    scc_data: SccData<S>,
 }
 
 /// Information about an invidividual SCC node.
-struct SccDetails<A: Annotation> {
+struct SccDetails {
     /// For this SCC, the range of `all_successors` where its
     /// successors can be found.
     range: Range<usize>,
-
-    /// User-specified metadata about the SCC.
-    annotation: A,
 }
 
 // The name of this struct should discourage you from making it public and leaking
@@ -87,35 +104,29 @@ struct SccDetails<A: Annotation> {
 // is difficult when it's publicly inspectable.
 //
 // Obey the law of Demeter!
-struct SccData<S: Idx, A: Annotation> {
+struct SccData<S: Idx> {
     /// Maps SCC indices to their metadata, including
     /// offsets into `all_successors`.
-    scc_details: IndexVec<S, SccDetails<A>>,
+    scc_details: IndexVec<S, SccDetails>,
 
     /// Contains the successors for all the Sccs, concatenated. The
     /// range of indices corresponding to a given SCC is found in its
     /// `scc_details.range`.
     all_successors: Vec<S>,
 }
 
-impl<N: Idx, S: Idx + Ord> Sccs<N, S, ()> {
+impl<N: Idx, S: Idx + Ord> Sccs<N, S> {
     /// Compute SCCs without annotations.
     pub fn new(graph: &impl Successors<Node = N>) -> Self {
-        Self::new_with_annotation(graph, |_| ())
+        Self::new_with_annotation(graph, &mut NoAnnotations(PhantomData::<S>))
     }
-}
 
-impl<N: Idx, S: Idx + Ord, A: Annotation> Sccs<N, S, A> {
     /// Compute SCCs and annotate them with a user-supplied annotation
-    pub fn new_with_annotation<F: Fn(N) -> A>(
+    pub fn new_with_annotation<A: Annotations<N, SccIdx = S>>(
         graph: &impl Successors<Node = N>,
-        to_annotation: F,
+        annotations: &mut A,
     ) -> Self {
-        SccsConstruction::construct(graph, to_annotation)
-    }
-
-    pub fn annotation(&self, scc: S) -> A {
-        self.scc_data.annotation(scc)
+        SccsConstruction::construct(graph, annotations)
     }
 
     pub fn scc_indices(&self) -> &IndexSlice<N, S> {
@@ -160,27 +171,27 @@ impl<N: Idx, S: Idx + Ord, A: Annotation> Sccs<N, S, A> {
     }
 }
 
-impl<N: Idx, S: Idx + Ord, A: Annotation> DirectedGraph for Sccs<N, S, A> {
+impl<N: Idx, S: Idx + Ord> DirectedGraph for Sccs<N, S> {
     type Node = S;
 
     fn num_nodes(&self) -> usize {
         self.num_sccs()
     }
 }
 
-impl<N: Idx, S: Idx + Ord, A: Annotation> NumEdges for Sccs<N, S, A> {
+impl<N: Idx, S: Idx + Ord> NumEdges for Sccs<N, S> {
     fn num_edges(&self) -> usize {
         self.scc_data.all_successors.len()
     }
 }
 
-impl<N: Idx, S: Idx + Ord, A: Annotation> Successors for Sccs<N, S, A> {
+impl<N: Idx, S: Idx + Ord> Successors for Sccs<N, S> {
     fn successors(&self, node: S) -> impl Iterator<Item = Self::Node> {
         self.successors(node).iter().cloned()
     }
 }
 
-impl<S: Idx, A: Annotation> SccData<S, A> {
+impl<S: Idx> SccData<S> {
     /// Number of SCCs,
     fn len(&self) -> usize {
         self.scc_details.len()
@@ -192,45 +203,37 @@ impl<S: Idx, A: Annotation> SccData<S, A> {
     }
 
     /// Creates a new SCC with `successors` as its successors and
-    /// the maximum weight of its internal nodes `scc_max_weight` and
     /// returns the resulting index.
-    fn create_scc(&mut self, successors: impl IntoIterator<Item = S>, annotation: A) -> S {
+    fn create_scc(&mut self, successors: impl IntoIterator<Item = S>) -> S {
         // Store the successors on `scc_successors_vec`, remembering
         // the range of indices.
         let all_successors_start = self.all_successors.len();
         self.all_successors.extend(successors);
         let all_successors_end = self.all_successors.len();
 
         debug!(
-            "create_scc({:?}) successors={:?}, annotation={:?}",
+            "create_scc({:?}) successors={:?}",
             self.len(),
             &self.all_successors[all_successors_start..all_successors_end],
-            annotation
         );
 
         let range = all_successors_start..all_successors_end;
-        let metadata = SccDetails { range, annotation };
+        let metadata = SccDetails { range };
         self.scc_details.push(metadata)
     }
-
-    fn annotation(&self, scc: S) -> A {
-        self.scc_details[scc].annotation
-    }
 }
 
-struct SccsConstruction<'c, G, S, A, F>
+struct SccsConstruction<'c, 'a, G, A>
 where
     G: DirectedGraph + Successors,
-    S: Idx,
-    A: Annotation,
-    F: Fn(G::Node) -> A,
+    A: Annotations<G::Node>,
 {
     graph: &'c G,
 
     /// The state of each node; used during walk to record the stack
     /// and after walk to record what cycle each node ended up being
     /// in.
-    node_states: IndexVec<G::Node, NodeState<G::Node, S, A>>,
+    node_states: IndexVec<G::Node, NodeState<G::Node, A::SccIdx, A::Ann>>,
 
     /// The stack of nodes that we are visiting as part of the DFS.
     node_stack: Vec<G::Node>,
@@ -239,23 +242,21 @@ where
     /// position in this stack, and when we encounter a successor SCC,
     /// we push it on the stack. When we complete an SCC, we can pop
     /// everything off the stack that was found along the way.
-    successors_stack: Vec<S>,
+    successors_stack: Vec<A::SccIdx>,
 
     /// A set used to strip duplicates. As we accumulate successors
     /// into the successors_stack, we sometimes get duplicate entries.
     /// We use this set to remove those -- we also keep its storage
     /// around between successors to amortize memory allocation costs.
-    duplicate_set: FxHashSet<S>,
+    duplicate_set: FxHashSet<A::SccIdx>,
 
-    scc_data: SccData<S, A>,
+    scc_data: SccData<A::SccIdx>,
 
-    /// A function that constructs an initial SCC annotation
-    /// out of a single node.
-    to_annotation: F,
+    annotations: &'a mut A,
 }
 
 #[derive(Copy, Clone, Debug)]
-enum NodeState<N, S, A> {
+enum NodeState<N, S, A: Annotation> {
     /// This node has not yet been visited as part of the DFS.
     ///
     /// After SCC construction is complete, this state ought to be
@@ -286,7 +287,7 @@ enum NodeState<N, S, A> {
 
 /// The state of walking a given node.
 #[derive(Copy, Clone, Debug)]
-enum WalkReturn<S, A> {
+enum WalkReturn<S, A: Annotation> {
     /// The walk found a cycle, but the entire component is not known to have
     /// been fully walked yet. We only know the minimum depth of  this
     /// component in a minimum spanning tree of the graph. This component
@@ -299,12 +300,10 @@ enum WalkReturn<S, A> {
     Complete { scc_index: S, annotation: A },
 }
 
-impl<'c, G, S, A, F> SccsConstruction<'c, G, S, A, F>
+impl<'c, 'a, G, A> SccsConstruction<'c, 'a, G, A>
 where
     G: DirectedGraph + Successors,
-    S: Idx,
-    F: Fn(G::Node) -> A,
-    A: Annotation,
+    A: Annotations<G::Node>,
 {
     /// Identifies SCCs in the graph `G` and computes the resulting
     /// DAG. This uses a variant of [Tarjan's
@@ -320,7 +319,7 @@ where
     /// Additionally, we keep track of a current annotation of the SCC.
     ///
     /// [wikipedia]: https://bit.ly/2EZIx84
-    fn construct(graph: &'c G, to_annotation: F) -> Sccs<G::Node, S, A> {
+    fn construct(graph: &'c G, annotations: &'a mut A) -> Sccs<G::Node, A::SccIdx> {
         let num_nodes = graph.num_nodes();
 
         let mut this = Self {
@@ -330,7 +329,7 @@ where
             successors_stack: Vec::new(),
             scc_data: SccData { scc_details: IndexVec::new(), all_successors: Vec::new() },
             duplicate_set: FxHashSet::default(),
-            to_annotation,
+            annotations,
         };
 
         let scc_indices = graph
@@ -346,7 +345,7 @@ where
         Sccs { scc_indices, scc_data: this.scc_data }
     }
 
-    fn start_walk_from(&mut self, node: G::Node) -> WalkReturn<S, A> {
+    fn start_walk_from(&mut self, node: G::Node) -> WalkReturn<A::SccIdx, A::Ann> {
         self.inspect_node(node).unwrap_or_else(|| self.walk_unvisited_node(node))
     }
 
@@ -362,7 +361,7 @@ where
     /// Otherwise, we are looking at a node that has already been
     /// completely visited. We therefore return `WalkReturn::Complete`
     /// with its associated SCC index.
-    fn inspect_node(&mut self, node: G::Node) -> Option<WalkReturn<S, A>> {
+    fn inspect_node(&mut self, node: G::Node) -> Option<WalkReturn<A::SccIdx, A::Ann>> {
         Some(match self.find_state(node) {
             NodeState::InCycle { scc_index, annotation } => {
                 WalkReturn::Complete { scc_index, annotation }
@@ -385,7 +384,7 @@ where
     /// of `r2` (and updates `r` to reflect current result). This is
     /// basically the "find" part of a standard union-find algorithm
     /// (with path compression).
-    fn find_state(&mut self, mut node: G::Node) -> NodeState<G::Node, S, A> {
+    fn find_state(&mut self, mut node: G::Node) -> NodeState<G::Node, A::SccIdx, A::Ann> {
         // To avoid recursion we temporarily reuse the `parent` of each
         // InCycleWith link to encode a downwards link while compressing
         // the path. After we have found the root or deepest node being
@@ -408,7 +407,7 @@ where
         // a potentially derived version of the root state for non-root nodes in the chain.
         let (root_state, assigned_state) = {
             loop {
-                debug!("find_state(r = {node:?} in state {:?})", self.node_states[node]);
+                trace!("find_state(r = {node:?} in state {:?})", self.node_states[node]);
                 match self.node_states[node] {
                     // This must have been the first and only state since it is unexplored*;
                     // no update needed! * Unless there is a bug :')
@@ -482,7 +481,7 @@ where
             if previous_node == node {
                 return root_state;
             }
-            debug!("Compressing {node:?} down to {previous_node:?} with state {assigned_state:?}");
+            trace!("Compressing {node:?} down to {previous_node:?} with state {assigned_state:?}");
 
             // Update to previous node in the link.
             match self.node_states[previous_node] {
@@ -507,9 +506,9 @@ where
     /// Call this method when `inspect_node` has returned `None`. Having the
     /// caller decide avoids mutual recursion between the two methods and allows
     /// us to maintain an allocated stack for nodes on the path between calls.
-    #[instrument(skip(self, initial), level = "debug")]
-    fn walk_unvisited_node(&mut self, initial: G::Node) -> WalkReturn<S, A> {
-        debug!("Walk unvisited node: {initial:?}");
+    #[instrument(skip(self, initial), level = "trace")]
+    fn walk_unvisited_node(&mut self, initial: G::Node) -> WalkReturn<A::SccIdx, A::Ann> {
+        trace!("Walk unvisited node: {initial:?}");
         struct VisitingNodeFrame<G: DirectedGraph, Successors, A> {
             node: G::Node,
             successors: Option<Successors>,
@@ -537,7 +536,7 @@ where
             successors_len: 0,
             min_cycle_root: initial,
             successor_node: initial,
-            current_component_annotation: (self.to_annotation)(initial),
+            current_component_annotation: self.annotations.new(initial),
         }];
 
         let mut return_value = None;
@@ -556,19 +555,15 @@ where
             let node = *node;
             let depth = *depth;
 
-            // node is definitely in the current component, add it to the annotation.
-            if node != initial {
-                current_component_annotation.update_scc((self.to_annotation)(node));
-            }
-            debug!(
+            trace!(
                 "Visiting {node:?} at depth {depth:?}, annotation: {current_component_annotation:?}"
             );
 
             let successors = match successors {
                 Some(successors) => successors,
                 None => {
                     // This None marks that we still have the initialize this node's frame.
-                    debug!(?depth, ?node);
+                    trace!(?depth, ?node);
 
                     debug_assert_matches!(self.node_states[node], NodeState::NotVisited);
 
@@ -598,7 +593,7 @@ where
                 return_value.take().into_iter().map(|walk| (*successor_node, Some(walk)));
 
             let successor_walk = successors.map(|successor_node| {
-                debug!(?node, ?successor_node);
+                trace!(?node, ?successor_node);
                 (successor_node, self.inspect_node(successor_node))
             });
             for (successor_node, walk) in returned_walk.chain(successor_walk) {
@@ -609,13 +604,13 @@ where
                         min_depth: successor_min_depth,
                         annotation: successor_annotation,
                     }) => {
-                        debug!(
+                        trace!(
                             "Cycle found from {node:?}, minimum depth: {successor_min_depth:?}, annotation: {successor_annotation:?}"
                         );
                         // Track the minimum depth we can reach.
                         assert!(successor_min_depth <= depth);
                         if successor_min_depth < *min_depth {
-                            debug!(?node, ?successor_min_depth);
+                            trace!(?node, ?successor_min_depth);
                             *min_depth = successor_min_depth;
                             *min_cycle_root = successor_node;
                         }
@@ -627,20 +622,20 @@ where
                         scc_index: successor_scc_index,
                         annotation: successor_annotation,
                     }) => {
-                        debug!(
+                        trace!(
                             "Complete; {node:?} is root of complete-visited SCC idx {successor_scc_index:?} with annotation {successor_annotation:?}"
                         );
                         // Push the completed SCC indices onto
                         // the `successors_stack` for later.
-                        debug!(?node, ?successor_scc_index);
+                        trace!(?node, ?successor_scc_index);
                         successors_stack.push(successor_scc_index);
                         current_component_annotation.update_reachable(successor_annotation);
                     }
                     // `node` has no more (direct) successors; search recursively.
                     None => {
                         let depth = depth + 1;
-                        debug!("Recursing down into {successor_node:?} at depth {depth:?}");
-                        debug!(?depth, ?successor_node);
+                        trace!("Recursing down into {successor_node:?} at depth {depth:?}");
+                        trace!(?depth, ?successor_node);
                         // Remember which node the return value will come from.
                         frame.successor_node = successor_node;
                         // Start a new stack frame, then step into it.
@@ -652,14 +647,14 @@ where
                             min_depth: depth,
                             min_cycle_root: successor_node,
                             successor_node,
-                            current_component_annotation: (self.to_annotation)(successor_node),
+                            current_component_annotation: self.annotations.new(successor_node),
                         });
                         continue 'recurse;
                     }
                 }
             }
 
-            debug!("Finished walk from {node:?} with annotation: {current_component_annotation:?}");
+            trace!("Finished walk from {node:?} with annotation: {current_component_annotation:?}");
 
             // Completed walk, remove `node` from the stack.
             let r = self.node_stack.pop();
@@ -691,8 +686,9 @@ where
 
                 debug!("Creating SCC rooted in {node:?} with successor {:?}", frame.successor_node);
 
-                let scc_index =
-                    self.scc_data.create_scc(deduplicated_successors, current_component_annotation);
+                let scc_index = self.scc_data.create_scc(deduplicated_successors);
+
+                self.annotations.annotate_scc(scc_index, current_component_annotation);
 
                 self.node_states[node] =
                     NodeState::InCycle { scc_index, annotation: current_component_annotation };

compiler/rustc_data_structures/src/graph/scc/tests.rs

@@ -5,16 +5,39 @@ use crate::graph::tests::TestGraph;
 
 #[derive(Copy, Clone, Debug)]
 struct MaxReached(usize);
-type UsizeSccs = Sccs<usize, usize, ()>;
-type MaxReachedSccs = Sccs<usize, usize, MaxReached>;
+struct Maxes(IndexVec<usize, MaxReached>, fn(usize) -> usize);
+type UsizeSccs = Sccs<usize, usize>;
+
+impl Annotations<usize> for Maxes {
+    fn new(&self, element: usize) -> MaxReached {
+        MaxReached(self.1(element))
+    }
+
+    fn annotate_scc(&mut self, scc: usize, annotation: MaxReached) {
+        let i = self.0.push(annotation);
+        assert!(i == scc);
+    }
+
+    type Ann = MaxReached;
+    type SccIdx = usize;
+}
+
+impl Maxes {
+    fn annotation(&self, scc: usize) -> MaxReached {
+        self.0[scc]
+    }
+    fn new(mapping: fn(usize) -> usize) -> Self {
+        Self(IndexVec::new(), mapping)
+    }
+}
 
 impl Annotation for MaxReached {
     fn merge_scc(self, other: Self) -> Self {
         Self(std::cmp::max(other.0, self.0))
     }
 
     fn merge_reached(self, other: Self) -> Self {
-        self.merge_scc(other)
+        Self(std::cmp::max(other.0, self.0))
     }
 }
 
@@ -24,17 +47,32 @@ impl PartialEq<usize> for MaxReached {
     }
 }
 
-impl MaxReached {
-    fn from_usize(nr: usize) -> Self {
-        Self(nr)
-    }
-}
-
 #[derive(Copy, Clone, Debug)]
 struct MinMaxIn {
     min: usize,
     max: usize,
 }
+struct MinMaxes(IndexVec<usize, MinMaxIn>, fn(usize) -> MinMaxIn);
+
+impl MinMaxes {
+    fn annotation(&self, scc: usize) -> MinMaxIn {
+        self.0[scc]
+    }
+}
+
+impl Annotations<usize> for MinMaxes {
+    fn new(&self, element: usize) -> MinMaxIn {
+        self.1(element)
+    }
+
+    fn annotate_scc(&mut self, scc: usize, annotation: MinMaxIn) {
+        let i = self.0.push(annotation);
+        assert!(i == scc);
+    }
+
+    type Ann = MinMaxIn;
+    type SccIdx = usize;
+}
 
 impl Annotation for MinMaxIn {
     fn merge_scc(self, other: Self) -> Self {
@@ -261,67 +299,68 @@ fn bench_sccc(b: &mut test::Bencher) {
 #[test]
 fn test_max_self_loop() {
     let graph = TestGraph::new(0, &[(0, 0)]);
-    let sccs: MaxReachedSccs =
-        Sccs::new_with_annotation(&graph, |n| if n == 0 { MaxReached(17) } else { MaxReached(0) });
-    assert_eq!(sccs.annotation(0), 17);
+    let mut annotations = Maxes(IndexVec::new(), |n| if n == 0 { 17 } else { 0 });
+    Sccs::new_with_annotation(&graph, &mut annotations);
+    assert_eq!(annotations.0[0], 17);
 }
 
 #[test]
 fn test_max_branch() {
     let graph = TestGraph::new(0, &[(0, 1), (0, 2), (1, 3), (2, 4)]);
-    let sccs: MaxReachedSccs = Sccs::new_with_annotation(&graph, MaxReached::from_usize);
-    assert_eq!(sccs.annotation(sccs.scc(0)), 4);
-    assert_eq!(sccs.annotation(sccs.scc(1)), 3);
-    assert_eq!(sccs.annotation(sccs.scc(2)), 4);
+    let mut annotations = Maxes(IndexVec::new(), |n| n);
+    let sccs = Sccs::new_with_annotation(&graph, &mut annotations);
+    assert_eq!(annotations.0[sccs.scc(0)], 4);
+    assert_eq!(annotations.0[sccs.scc(1)], 3);
+    assert_eq!(annotations.0[sccs.scc(2)], 4);
 }
+
 #[test]
 fn test_single_cycle_max() {
     let graph = TestGraph::new(0, &[(0, 2), (2, 3), (2, 4), (4, 1), (1, 2)]);
-    let sccs: MaxReachedSccs = Sccs::new_with_annotation(&graph, MaxReached::from_usize);
-    assert_eq!(sccs.annotation(sccs.scc(2)), 4);
-    assert_eq!(sccs.annotation(sccs.scc(0)), 4);
-}
-
-#[test]
-fn test_simple_cycle_max() {
-    let graph = TestGraph::new(0, &[(0, 1), (1, 2), (2, 0)]);
-    let sccs: MaxReachedSccs = Sccs::new_with_annotation(&graph, MaxReached::from_usize);
-    assert_eq!(sccs.num_sccs(), 1);
+    let mut annotations = Maxes(IndexVec::new(), |n| n);
+    let sccs = Sccs::new_with_annotation(&graph, &mut annotations);
+    assert_eq!(annotations.0[sccs.scc(2)], 4);
+    assert_eq!(annotations.0[sccs.scc(0)], 4);
 }
 
 #[test]
 fn test_double_cycle_max() {
     let graph =
         TestGraph::new(0, &[(0, 1), (1, 2), (1, 4), (2, 3), (2, 4), (3, 5), (4, 1), (5, 4)]);
-    let sccs: MaxReachedSccs =
-        Sccs::new_with_annotation(&graph, |n| if n == 5 { MaxReached(2) } else { MaxReached(1) });
+    let mut annotations = Maxes(IndexVec::new(), |n| if n == 5 { 2 } else { 1 });
+
+    let sccs = Sccs::new_with_annotation(&graph, &mut annotations);
 
-    assert_eq!(sccs.annotation(sccs.scc(0)).0, 2);
+    assert_eq!(annotations.0[sccs.scc(0)].0, 2);
 }
 
 #[test]
 fn test_bug_minimised() {
     let graph = TestGraph::new(0, &[(0, 3), (0, 1), (3, 2), (2, 3), (1, 4), (4, 5), (5, 4)]);
-    let sccs: MaxReachedSccs = Sccs::new_with_annotation(&graph, |n| match n {
-        3 => MaxReached(1),
-        _ => MaxReached(0),
+    let mut annotations = Maxes(IndexVec::new(), |n| match n {
+        3 => 1,
+        _ => 0,
     });
-    assert_eq!(sccs.annotation(sccs.scc(2)), 1);
-    assert_eq!(sccs.annotation(sccs.scc(1)), 0);
-    assert_eq!(sccs.annotation(sccs.scc(4)), 0);
+
+    let sccs = Sccs::new_with_annotation(&graph, &mut annotations);
+    assert_eq!(annotations.annotation(sccs.scc(2)), 1);
+    assert_eq!(annotations.annotation(sccs.scc(1)), 0);
+    assert_eq!(annotations.annotation(sccs.scc(4)), 0);
 }
 
 #[test]
 fn test_bug_max_leak_minimised() {
     let graph = TestGraph::new(0, &[(0, 1), (0, 2), (1, 3), (3, 0), (3, 4), (4, 3)]);
-    let sccs: MaxReachedSccs = Sccs::new_with_annotation(&graph, |w| match w {
-        4 => MaxReached(1),
-        _ => MaxReached(0),
+    let mut annotations = Maxes(IndexVec::new(), |w| match w {
+        4 => 1,
+        _ => 0,
     });
 
-    assert_eq!(sccs.annotation(sccs.scc(2)), 0);
-    assert_eq!(sccs.annotation(sccs.scc(3)), 1);
-    assert_eq!(sccs.annotation(sccs.scc(0)), 1);
+    let sccs = Sccs::new_with_annotation(&graph, &mut annotations);
+
+    assert_eq!(annotations.annotation(sccs.scc(2)), 0);
+    assert_eq!(annotations.annotation(sccs.scc(3)), 1);
+    assert_eq!(annotations.annotation(sccs.scc(0)), 1);
 }
 
 #[test]
@@ -369,48 +408,49 @@ fn test_bug_max_leak() {
             (23, 24),
         ],
     );
-    let sccs: MaxReachedSccs = Sccs::new_with_annotation(&graph, |w| match w {
-        22 => MaxReached(1),
-        24 => MaxReached(2),
-        27 => MaxReached(2),
-        _ => MaxReached(0),
+    let mut annotations = Maxes::new(|w| match w {
+        22 => 1,
+        24 => 2,
+        27 => 2,
+        _ => 0,
     });
-
-    assert_eq!(sccs.annotation(sccs.scc(2)), 0);
-    assert_eq!(sccs.annotation(sccs.scc(7)), 0);
-    assert_eq!(sccs.annotation(sccs.scc(8)), 2);
-    assert_eq!(sccs.annotation(sccs.scc(23)), 2);
-    assert_eq!(sccs.annotation(sccs.scc(3)), 2);
-    assert_eq!(sccs.annotation(sccs.scc(0)), 2);
+    let sccs = Sccs::new_with_annotation(&graph, &mut annotations);
+
+    assert_eq!(annotations.annotation(sccs.scc(2)), 0);
+    assert_eq!(annotations.annotation(sccs.scc(7)), 0);
+    assert_eq!(annotations.annotation(sccs.scc(8)), 2);
+    assert_eq!(annotations.annotation(sccs.scc(23)), 2);
+    assert_eq!(annotations.annotation(sccs.scc(3)), 2);
+    assert_eq!(annotations.annotation(sccs.scc(0)), 2);
 }
 
 #[test]
 fn test_bug_max_zero_stick_shape() {
     let graph = TestGraph::new(0, &[(0, 1), (1, 2), (2, 3), (3, 2), (3, 4)]);
-
-    let sccs: MaxReachedSccs = Sccs::new_with_annotation(&graph, |w| match w {
-        4 => MaxReached(1),
-        _ => MaxReached(0),
+    let mut annotations = Maxes::new(|w| match w {
+        4 => 1,
+        _ => 0,
     });
+    let sccs = Sccs::new_with_annotation(&graph, &mut annotations);
 
-    assert_eq!(sccs.annotation(sccs.scc(0)), 1);
-    assert_eq!(sccs.annotation(sccs.scc(1)), 1);
-    assert_eq!(sccs.annotation(sccs.scc(2)), 1);
-    assert_eq!(sccs.annotation(sccs.scc(3)), 1);
-    assert_eq!(sccs.annotation(sccs.scc(4)), 1);
+    assert_eq!(annotations.annotation(sccs.scc(0)), 1);
+    assert_eq!(annotations.annotation(sccs.scc(1)), 1);
+    assert_eq!(annotations.annotation(sccs.scc(2)), 1);
+    assert_eq!(annotations.annotation(sccs.scc(3)), 1);
+    assert_eq!(annotations.annotation(sccs.scc(4)), 1);
 }
 
 #[test]
 fn test_min_max_in() {
     let graph = TestGraph::new(0, &[(0, 1), (0, 2), (1, 3), (3, 0), (3, 4), (4, 3), (3, 5)]);
-    let sccs: Sccs<usize, usize, MinMaxIn> =
-        Sccs::new_with_annotation(&graph, |w| MinMaxIn { min: w, max: w });
-
-    assert_eq!(sccs.annotation(sccs.scc(2)).min, 2);
-    assert_eq!(sccs.annotation(sccs.scc(2)).max, 2);
-    assert_eq!(sccs.annotation(sccs.scc(0)).min, 0);
-    assert_eq!(sccs.annotation(sccs.scc(0)).max, 4);
-    assert_eq!(sccs.annotation(sccs.scc(3)).min, 0);
-    assert_eq!(sccs.annotation(sccs.scc(3)).max, 4);
-    assert_eq!(sccs.annotation(sccs.scc(5)).min, 5);
+    let mut annotations = MinMaxes(IndexVec::new(), |w| MinMaxIn { min: w, max: w });
+    let sccs = Sccs::new_with_annotation(&graph, &mut annotations);
+
+    assert_eq!(annotations.annotation(sccs.scc(2)).min, 2);
+    assert_eq!(annotations.annotation(sccs.scc(2)).max, 2);
+    assert_eq!(annotations.annotation(sccs.scc(0)).min, 0);
+    assert_eq!(annotations.annotation(sccs.scc(0)).max, 4);
+    assert_eq!(annotations.annotation(sccs.scc(3)).min, 0);
+    assert_eq!(annotations.annotation(sccs.scc(3)).max, 4);
+    assert_eq!(annotations.annotation(sccs.scc(5)).min, 5);
 }