Borrowck: Simplify SCC annotation computation, placeholder rewriting

This simplifies the `PlaceholderReachability` `enum` by
replacing the case when no placeholders were reached with
a standard `Option::None`.

It also rewrites the API for `scc::Annotations` to be update-mut
rather than a more Functional programming style. This showed some slight
performance impact in early tests of the PR and definitely makes
the implementation simpler.

Author: amandasystems

compiler/rustc_borrowck/src/handle_placeholders.rs

@@ -62,65 +62,31 @@ impl scc::Annotations<RegionVid> for SccAnnotations<'_, '_, RegionTracker> {
 }
 
 #[derive(Copy, Debug, Clone, PartialEq, Eq)]
-enum PlaceholderReachability {
-    /// This SCC reaches no placeholders.
-    NoPlaceholders,
-    /// This SCC reaches at least one placeholder.
-    Placeholders {
-        /// The largest-universed placeholder we can reach
-        max_universe: (UniverseIndex, RegionVid),
-
-        /// The placeholder with the smallest ID
-        min_placeholder: RegionVid,
-
-        /// The placeholder with the largest ID
-        max_placeholder: RegionVid,
-    },
+struct PlaceholderReachability {
+    /// The largest-universed placeholder we can reach
+    max_universe: (UniverseIndex, RegionVid),
+
+    /// The placeholder with the smallest ID
+    min_placeholder: RegionVid,
+
+    /// The placeholder with the largest ID
+    max_placeholder: RegionVid,
 }
 
 impl PlaceholderReachability {
     /// Merge the reachable placeholders of two graph components.
-    fn merge(self, other: PlaceholderReachability) -> PlaceholderReachability {
-        use PlaceholderReachability::*;
-        match (self, other) {
-            (NoPlaceholders, NoPlaceholders) => NoPlaceholders,
-            (NoPlaceholders, p @ Placeholders { .. })
-            | (p @ Placeholders { .. }, NoPlaceholders) => p,
-            (
-                Placeholders {
-                    min_placeholder: min_pl,
-                    max_placeholder: max_pl,
-                    max_universe: max_u,
-                },
-                Placeholders { min_placeholder, max_placeholder, max_universe },
-            ) => Placeholders {
-                min_placeholder: min_pl.min(min_placeholder),
-                max_placeholder: max_pl.max(max_placeholder),
-                max_universe: max_u.max(max_universe),
-            },
-        }
-    }
-
-    fn max_universe(&self) -> Option<(UniverseIndex, RegionVid)> {
-        match self {
-            Self::NoPlaceholders => None,
-            Self::Placeholders { max_universe, .. } => Some(*max_universe),
-        }
-    }
-
-    /// If we have reached placeholders, determine if they can
-    /// be named from this universe.
-    fn can_be_named_by(&self, from: UniverseIndex) -> bool {
-        self.max_universe()
-            .is_none_or(|(max_placeholder_universe, _)| from.can_name(max_placeholder_universe))
+    fn merge(&mut self, other: &Self) {
+        self.max_universe = self.max_universe.max(other.max_universe);
+        self.min_placeholder = self.min_placeholder.min(other.min_placeholder);
+        self.max_placeholder = self.max_placeholder.max(other.max_placeholder);
     }
 }
 
 /// An annotation for region graph SCCs that tracks
 /// the values of its elements. This annotates a single SCC.
 #[derive(Copy, Debug, Clone)]
 pub(crate) struct RegionTracker {
-    reachable_placeholders: PlaceholderReachability,
+    reachable_placeholders: Option<PlaceholderReachability>,
 
     /// The largest universe nameable from this SCC.
     /// It is the smallest nameable universes of all
@@ -135,13 +101,13 @@ impl RegionTracker {
     pub(crate) fn new(rvid: RegionVid, definition: &RegionDefinition<'_>) -> Self {
         let reachable_placeholders =
             if matches!(definition.origin, NllRegionVariableOrigin::Placeholder(_)) {
-                PlaceholderReachability::Placeholders {
+                Some(PlaceholderReachability {
                     max_universe: (definition.universe, rvid),
                     min_placeholder: rvid,
                     max_placeholder: rvid,
-                }
+                })
             } else {
-                PlaceholderReachability::NoPlaceholders
+                None
             };
 
         Self {
@@ -159,43 +125,46 @@ impl RegionTracker {
     }
 
     pub(crate) fn max_placeholder_universe_reached(self) -> UniverseIndex {
-        if let Some((universe, _)) = self.reachable_placeholders.max_universe() {
-            universe
-        } else {
-            UniverseIndex::ROOT
-        }
+        self.reachable_placeholders.map(|pls| pls.max_universe.0).unwrap_or(UniverseIndex::ROOT)
+    }
+
+    /// Can all reachable placeholders be named from `from`?
+    /// True vacuously in case no placeholders were reached.
+    fn placeholders_can_be_named_by(&self, from: UniverseIndex) -> bool {
+        self.reachable_placeholders.is_none_or(|pls| from.can_name(pls.max_universe.0))
     }
 
     /// Determine if we can name all the placeholders in `other`.
     pub(crate) fn can_name_all_placeholders(&self, other: Self) -> bool {
-        other.reachable_placeholders.can_be_named_by(self.max_nameable_universe.0)
+        // HACK: We first check whether we can name the highest existential universe
+        // of `other`. This only exists to avoid errors in case that scc already
+        // depends on a placeholder it cannot name itself.
+        self.max_nameable_universe().can_name(other.max_nameable_universe())
+            || other.placeholders_can_be_named_by(self.max_nameable_universe.0)
     }
 
     /// If this SCC reaches a placeholder it can't name, return it.
     fn unnameable_placeholder(&self) -> Option<(UniverseIndex, RegionVid)> {
-        self.reachable_placeholders.max_universe().filter(|&(placeholder_universe, _)| {
-            !self.max_nameable_universe().can_name(placeholder_universe)
-        })
+        self.reachable_placeholders
+            .filter(|pls| !self.max_nameable_universe().can_name(pls.max_universe.0))
+            .map(|pls| pls.max_universe)
     }
 }
 
 impl scc::Annotation for RegionTracker {
-    fn merge_scc(self, other: Self) -> Self {
+    fn update_scc(&mut self, other: &Self) {
         trace!("{:?} << {:?}", self.representative, other.representative);
-
-        Self {
-            representative: self.representative.min(other.representative),
-            max_nameable_universe: self.max_nameable_universe.min(other.max_nameable_universe),
-            reachable_placeholders: self.reachable_placeholders.merge(other.reachable_placeholders),
-        }
+        self.representative = self.representative.min(other.representative);
+        self.update_reachable(other);
     }
 
-    fn merge_reached(self, other: Self) -> Self {
-        Self {
-            max_nameable_universe: self.max_nameable_universe.min(other.max_nameable_universe),
-            reachable_placeholders: self.reachable_placeholders.merge(other.reachable_placeholders),
-            representative: self.representative,
-        }
+    fn update_reachable(&mut self, other: &Self) {
+        self.max_nameable_universe = self.max_nameable_universe.min(other.max_nameable_universe);
+        match (self.reachable_placeholders.as_mut(), other.reachable_placeholders.as_ref()) {
+            (None, None) | (Some(_), None) => (),
+            (None, Some(theirs)) => self.reachable_placeholders = Some(*theirs),
+            (Some(ours), Some(theirs)) => ours.merge(theirs),
+        };
     }
 }
 

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

@@ -27,26 +27,18 @@ mod tests;
 /// the max/min element of the SCC, or all of the above.
 ///
 /// Concretely, the both merge operations must commute, e.g. where `merge`
-/// is `merge_scc` and `merge_reached`: `a.merge(b) == b.merge(a)`
+/// is `update_scc` and `update_reached`: `a.merge(b) == b.merge(a)`
 ///
 /// In general, what you want is probably always min/max according
 /// to some ordering, potentially with side constraints (min x such
 /// that P holds).
 pub trait Annotation: Debug + Copy {
     /// Merge two existing annotations into one during
-    /// path compression.o
-    fn merge_scc(self, other: Self) -> Self;
+    /// path compression.
+    fn update_scc(&mut self, other: &Self);
 
     /// Merge a successor into this annotation.
-    fn merge_reached(self, other: Self) -> Self;
-
-    fn update_scc(&mut self, other: Self) {
-        *self = self.merge_scc(other)
-    }
-
-    fn update_reachable(&mut self, other: Self) {
-        *self = self.merge_reached(other)
-    }
+    fn update_reachable(&mut self, other: &Self);
 }
 
 /// An accumulator for annotations.
@@ -70,12 +62,8 @@ impl<N: Idx, S: Idx + Ord> Annotations<N> for NoAnnotations<S> {
 
 /// The empty annotation, which does nothing.
 impl Annotation for () {
-    fn merge_reached(self, _other: Self) -> Self {
-        ()
-    }
-    fn merge_scc(self, _other: Self) -> Self {
-        ()
-    }
+    fn update_reachable(&mut self, _other: &Self) {}
+    fn update_scc(&mut self, _other: &Self) {}
 }
 
 /// Strongly connected components (SCC) of a graph. The type `N` is
@@ -614,7 +602,7 @@ where
                             *min_depth = successor_min_depth;
                             *min_cycle_root = successor_node;
                         }
-                        current_component_annotation.update_scc(successor_annotation);
+                        current_component_annotation.update_scc(&successor_annotation);
                     }
                     // The starting node `node` is succeeded by a fully identified SCC
                     // which is now added to the set under `scc_index`.
@@ -629,7 +617,7 @@ where
                         // the `successors_stack` for later.
                         trace!(?node, ?successor_scc_index);
                         successors_stack.push(successor_scc_index);
-                        current_component_annotation.update_reachable(successor_annotation);
+                        current_component_annotation.update_reachable(&successor_annotation);
                     }
                     // `node` has no more (direct) successors; search recursively.
                     None => {

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

@@ -32,12 +32,12 @@ impl Maxes {
 }
 
 impl Annotation for MaxReached {
-    fn merge_scc(self, other: Self) -> Self {
-        Self(std::cmp::max(other.0, self.0))
+    fn update_scc(&mut self, other: &Self) {
+        self.0 = self.0.max(other.0);
     }
 
-    fn merge_reached(self, other: Self) -> Self {
-        Self(std::cmp::max(other.0, self.0))
+    fn update_reachable(&mut self, other: &Self) {
+        self.0 = self.0.max(other.0);
     }
 }
 
@@ -75,13 +75,12 @@ impl Annotations<usize> for MinMaxes {
 }
 
 impl Annotation for MinMaxIn {
-    fn merge_scc(self, other: Self) -> Self {
-        Self { min: std::cmp::min(self.min, other.min), max: std::cmp::max(self.max, other.max) }
+    fn update_scc(&mut self, other: &Self) {
+        self.min = self.min.min(other.min);
+        self.max = self.max.max(other.max);
     }
 
-    fn merge_reached(self, _other: Self) -> Self {
-        self
-    }
+    fn update_reachable(&mut self, _other: &Self) {}
 }
 
 #[test]