Unify cycle detector implementations

Author: Hendrik van Antwerpen

stack-graphs/src/cycles.rs

@@ -42,6 +42,7 @@ use crate::graph::StackGraph;
 use crate::partial::PartialPath;
 use crate::partial::PartialPaths;
 use crate::paths::Path;
+use crate::paths::PathResolutionError;
 use crate::stitching::Database;
 use crate::stitching::OwnedOrDatabasePath;
 
@@ -136,43 +137,64 @@ where
 }
 
 // ----------------------------------------------------------------------------
-// Cycle detector when appending edges
+// Cycle detector
+
+pub trait Appendable {
+    type Ctx;
+
+    fn append_to(
+        &self,
+        graph: &StackGraph,
+        partials: &mut PartialPaths,
+        ctx: &mut Self::Ctx,
+        path: &mut PartialPath,
+    ) -> Result<(), PathResolutionError>;
+    fn start_node(&self, ctx: &mut Self::Ctx) -> Handle<Node>;
+    fn end_node(&self, ctx: &mut Self::Ctx) -> Handle<Node>;
+}
 
 #[derive(Clone)]
-pub struct EdgeAppendingCycleDetector {
-    edges: List<Edge>,
+pub struct AppendingCycleDetector<A> {
+    appendages: List<A>,
 }
 
-pub type AppendedEdges = ListArena<Edge>;
+pub type Appendages<A> = ListArena<A>;
 
-impl EdgeAppendingCycleDetector {
+impl<A: Appendable + Clone> AppendingCycleDetector<A> {
     pub fn new() -> Self {
         Self {
-            edges: List::empty(),
+            appendages: List::empty(),
         }
     }
 
-    pub fn append_edge(
+    pub fn from(appendages: &mut Appendages<A>, appendage: A) -> Self {
+        let mut result = Self::new();
+        result.appendages.push_front(appendages, appendage);
+        result
+    }
+
+    pub fn append(
         &mut self,
         graph: &StackGraph,
         partials: &mut PartialPaths,
-        appended_edges: &mut AppendedEdges,
-        edge: Edge,
-    ) -> Result<(), ()> {
-        let end_node = edge.sink;
-        self.edges.push_front(appended_edges, edge);
+        ctx: &mut A::Ctx,
+        appendages: &mut Appendages<A>,
+        appendage: A,
+    ) -> Result<(), PathResolutionError> {
+        let end_node = appendage.end_node(ctx);
+        self.appendages.push_front(appendages, appendage);
 
         let mut maybe_cyclic_path = None;
-        let mut index = self.edges;
-        let mut edges = self.edges;
+        let mut index = self.appendages;
+        let mut values = self.appendages;
         loop {
             // find loop point
             let mut count = 0usize;
             loop {
-                match index.pop_front(appended_edges) {
-                    Some(edge) => {
+                match index.pop_front(appendages) {
+                    Some(appendage) => {
                         count += 1;
-                        if edge.source == end_node {
+                        if appendage.start_node(ctx) == end_node {
                             break;
                         }
                     }
@@ -181,18 +203,21 @@ impl EdgeAppendingCycleDetector {
             }
 
             // get prefix edges
-            let mut prefix_edges = List::empty();
+            let mut prefix_appendages = List::empty();
             for _ in 0..count {
-                prefix_edges.push_front(appended_edges, *edges.pop_front(appended_edges).unwrap());
+                let appendage = values.pop_front(appendages).unwrap();
+                prefix_appendages.push_front(appendages, appendage.clone());
             }
 
-            // build prefix path
+            // build prefix path -- prefix starts at end_node, because this is a cycle
             let mut prefix_path = PartialPath::from_node(graph, partials, end_node);
-            while let Some(edge) = prefix_edges.pop_front(appended_edges) {
+            while let Some(appendage) = prefix_appendages.pop_front(appendages) {
                 prefix_path
-                    .resolve_to(graph, partials, edge.source)
+                    .resolve_to(graph, partials, appendage.start_node(ctx))
+                    .unwrap();
+                appendage
+                    .append_to(graph, partials, ctx, &mut prefix_path)
                     .unwrap();
-                prefix_path.append(graph, partials, *edge).unwrap();
             }
 
             // build cyclic path
@@ -206,99 +231,54 @@ impl EdgeAppendingCycleDetector {
                 .concatenate(graph, partials, &cyclic_path)
                 .unwrap();
             if !prefix_path.is_productive(graph, partials) {
-                return Err(());
+                return Err(PathResolutionError::DisallowedCycle);
             }
             maybe_cyclic_path = Some(prefix_path);
         }
     }
 }
 
-// ----------------------------------------------------------------------------
-// Cycle detector when appending partial paths
+impl Appendable for Edge {
+    type Ctx = ();
 
-#[derive(Clone)]
-pub struct PartialPathAppendingCycleDetector {
-    paths: List<OwnedOrDatabasePath>,
-}
+    fn append_to(
+        &self,
+        graph: &StackGraph,
+        partials: &mut PartialPaths,
+        _: &mut (),
+        path: &mut PartialPath,
+    ) -> Result<(), PathResolutionError> {
+        path.append(graph, partials, *self)
+    }
 
-pub type AppendedPartialPaths = ListArena<OwnedOrDatabasePath>;
-
-impl PartialPathAppendingCycleDetector {
-    pub fn from_partial_path(
-        _graph: &StackGraph,
-        _partials: &mut PartialPaths,
-        _db: &mut Database,
-        appended_paths: &mut AppendedPartialPaths,
-        path: OwnedOrDatabasePath,
-    ) -> Self {
-        let mut paths = List::empty();
-        paths.push_front(appended_paths, path);
-        Self { paths }
+    fn start_node(&self, _: &mut ()) -> Handle<Node> {
+        self.source
     }
 
-    pub fn append_partial_path(
-        &mut self,
-        graph: &StackGraph,
-        partials: &mut PartialPaths,
-        db: &Database,
-        appended_paths: &mut AppendedPartialPaths,
-        path: OwnedOrDatabasePath,
-    ) -> Result<(), ()> {
-        let end_node = path.get(db).end_node;
-        self.paths.push_front(appended_paths, path);
+    fn end_node(&self, _: &mut ()) -> Handle<Node> {
+        self.sink
+    }
+}
 
-        let mut maybe_cyclic_path = None;
-        let mut index = self.paths;
-        let mut paths = self.paths;
-        loop {
-            // find loop point
-            let mut count = 0usize;
-            loop {
-                match index.pop_front(appended_paths) {
-                    Some(path) => {
-                        count += 1;
-                        if path.get(db).start_node == end_node {
-                            break;
-                        }
-                    }
-                    None => return Ok(()),
-                }
-            }
+impl Appendable for OwnedOrDatabasePath {
+    type Ctx = Database;
 
-            // get prefix paths
-            let mut prefix_paths = List::empty();
-            for _ in 0..count {
-                prefix_paths.push_front(
-                    appended_paths,
-                    paths.pop_front(appended_paths).unwrap().clone(),
-                );
-            }
+    fn append_to(
+        &self,
+        graph: &StackGraph,
+        partials: &mut PartialPaths,
+        db: &mut Database,
+        path: &mut PartialPath,
+    ) -> Result<(), PathResolutionError> {
+        path.ensure_no_overlapping_variables(partials, self.get(db));
+        path.concatenate(graph, partials, self.get(db))
+    }
 
-            // build prefix path
-            let mut prefix_path = PartialPath::from_node(graph, partials, end_node);
-            while let Some(path) = prefix_paths.pop_front(appended_paths) {
-                let path = path.get(db);
-                prefix_path
-                    .resolve_to(graph, partials, path.start_node)
-                    .unwrap();
-                prefix_path.ensure_no_overlapping_variables(partials, path);
-                prefix_path.concatenate(graph, partials, path).unwrap();
-            }
+    fn start_node(&self, db: &mut Database) -> Handle<Node> {
+        self.get(db).start_node
+    }
 
-            // build cyclic path
-            let cyclic_path = maybe_cyclic_path
-                .unwrap_or_else(|| PartialPath::from_node(graph, partials, end_node));
-            prefix_path
-                .resolve_to(graph, partials, cyclic_path.start_node)
-                .unwrap();
-            prefix_path.ensure_no_overlapping_variables(partials, &cyclic_path);
-            prefix_path
-                .concatenate(graph, partials, &cyclic_path)
-                .unwrap();
-            if !prefix_path.is_productive(graph, partials) {
-                return Err(());
-            }
-            maybe_cyclic_path = Some(prefix_path);
-        }
+    fn end_node(&self, db: &mut Database) -> Handle<Node> {
+        self.get(db).end_node
     }
 }

stack-graphs/src/partial.rs

@@ -45,8 +45,8 @@ use smallvec::SmallVec;
 use crate::arena::Deque;
 use crate::arena::DequeArena;
 use crate::arena::Handle;
-use crate::cycles::AppendedEdges;
-use crate::cycles::EdgeAppendingCycleDetector;
+use crate::cycles::Appendages;
+use crate::cycles::AppendingCycleDetector;
 use crate::cycles::SimilarPathDetector;
 use crate::graph::Edge;
 use crate::graph::File;
@@ -2378,13 +2378,13 @@ impl PartialPath {
     /// as a parameter, instead of building it up ourselves, so that you have control over which
     /// particular collection type to use, and so that you can reuse result collections across
     /// multiple calls.
-    pub fn extend_from_file<R: Extend<(PartialPath, EdgeAppendingCycleDetector)>>(
+    pub fn extend_from_file<R: Extend<(PartialPath, AppendingCycleDetector<Edge>)>>(
         &self,
         graph: &StackGraph,
         partials: &mut PartialPaths,
         file: Handle<File>,
-        edges: &mut AppendedEdges,
-        path_cycle_detector: EdgeAppendingCycleDetector,
+        edges: &mut Appendages<Edge>,
+        path_cycle_detector: AppendingCycleDetector<Edge>,
         result: &mut R,
     ) {
         let extensions = graph.outgoing_edges(self.end_node);
@@ -2408,7 +2408,7 @@ impl PartialPath {
                 continue;
             }
             if new_cycle_detector
-                .append_edge(graph, partials, edges, extension)
+                .append(graph, partials, &mut (), edges, extension)
                 .is_err()
             {
                 copious_debugging!("         * cycle");
@@ -2700,7 +2700,7 @@ impl PartialPaths {
         copious_debugging!("Find all partial paths in {}", graph[file]);
         let mut similar_path_detector = SimilarPathDetector::new();
         let mut queue = VecDeque::new();
-        let mut edges = AppendedEdges::new();
+        let mut edges = Appendages::new();
         queue.extend(
             graph
                 .nodes_for_file(file)
@@ -2709,7 +2709,7 @@ impl PartialPaths {
                 .map(|node| {
                     (
                         PartialPath::from_node(graph, self, node),
-                        EdgeAppendingCycleDetector::new(),
+                        AppendingCycleDetector::new(),
                     )
                 }),
         );

stack-graphs/src/paths.rs

@@ -716,6 +716,8 @@ impl<'a> DisplayWithPaths for &'a Path {
 /// Errors that can occur during the path resolution process.
 #[derive(Debug)]
 pub enum PathResolutionError {
+    /// The path is cyclic, and the cycle is disallowed.
+    DisallowedCycle,
     /// The path contains a _jump to scope_ node, but there are no scopes on the scope stack to
     /// jump to.
     EmptyScopeStack,