Do not unwrap in cycle detector, since it cannot guarantee itself that this is safe

Author: Hendrik van Antwerpen

stack-graphs/src/cycles.rs

@@ -178,18 +178,20 @@ impl<A: Appendable + Clone> AppendingCycleDetector<A> {
         self.appendages.push_front(appendables, appendage);
     }
 
+    /// Tests if the path is cyclic. Returns a vector indicating the kind of cycles that were found.
+    /// If appending or concatenating all fragments succeeds, this function will never raise and error.
     pub fn is_cyclic(
         &self,
         graph: &StackGraph,
         partials: &mut PartialPaths,
         ctx: &mut A::Ctx,
         appendables: &mut Appendables<A>,
-    ) -> Vec<Cyclicity> {
+    ) -> Result<Vec<Cyclicity>, PathResolutionError> {
         let mut cycles = Vec::new();
 
         let end_node = match self.appendages.clone().pop_front(appendables) {
             Some(appendage) => appendage.end_node(ctx),
-            None => return cycles,
+            None => return Ok(cycles),
         };
 
         let mut maybe_cyclic_path = None;
@@ -207,31 +209,23 @@ impl<A: Appendable + Clone> AppendingCycleDetector<A> {
                             break;
                         }
                     }
-                    None => return cycles,
+                    None => return Ok(cycles),
                 }
             }
 
             // 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(appendage) = prefix_appendages.pop_front(appendables) {
-                prefix_path
-                    .resolve_to(graph, partials, appendage.start_node(ctx))
-                    .expect("resolving cycle prefix path failed");
-                appendage
-                    .append_to(graph, partials, ctx, &mut prefix_path)
-                    .expect("appending cycle prefix path failed");
+                prefix_path.resolve_to(graph, partials, appendage.start_node(ctx))?;
+                appendage.append_to(graph, partials, ctx, &mut prefix_path)?;
             }
 
             // 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)
-                .expect("resolving cyclic path failed");
+            prefix_path.resolve_to(graph, partials, cyclic_path.start_node)?;
             prefix_path.ensure_no_overlapping_variables(partials, &cyclic_path);
-            prefix_path
-                .concatenate(graph, partials, &cyclic_path)
-                .expect("concatenating cyclic path failed ");
+            prefix_path.concatenate(graph, partials, &cyclic_path)?;
             if let Some(cyclicity) = prefix_path.is_cyclic(graph, partials) {
                 cycles.push(cyclicity);
             }

stack-graphs/src/partial.rs

@@ -2743,6 +2743,7 @@ impl PartialPaths {
                 }
             } else if !path_cycle_detector
                 .is_cyclic(graph, self, &mut (), &mut edges)
+                .expect("cyclic test failed when finding partial paths")
                 .is_empty()
             {
                 copious_debugging!("    * cycle");

stack-graphs/src/paths.rs

@@ -26,7 +26,7 @@ use crate::arena::DequeArena;
 use crate::arena::Handle;
 use crate::arena::List;
 use crate::arena::ListArena;
-use crate::cycles::Appendages;
+use crate::cycles::Appendables;
 use crate::cycles::AppendingCycleDetector;
 use crate::cycles::SimilarPathDetector;
 use crate::graph::Edge;
@@ -875,7 +875,7 @@ impl Path {
         &self,
         graph: &StackGraph,
         paths: &mut Paths,
-        edges: &mut Appendages<Edge>,
+        edges: &mut Appendables<Edge>,
         path_cycle_detector: AppendingCycleDetector<Edge>,
         result: &mut R,
     ) {
@@ -924,7 +924,7 @@ impl Paths {
             })
             .collect::<VecDeque<_>>();
         let mut partials = PartialPaths::new();
-        let mut edges = Appendages::new();
+        let mut edges = Appendables::new();
         while let Some((path, path_cycle_detector)) = queue.pop_front() {
             cancellation_flag.check("finding paths")?;
             if !similar_path_detector_detector
@@ -936,6 +936,7 @@ impl Paths {
             }
             if !path_cycle_detector
                 .is_cyclic(graph, &mut partials, &mut (), &mut edges)
+                .expect("cyclic test failed when finding paths")
                 .into_iter()
                 .all(|c| c == Cyclicity::StrengthensPrecondition)
             {

stack-graphs/src/stitching.rs

@@ -482,7 +482,7 @@ pub struct PathStitcher {
         VecDeque<Path>,
         VecDeque<AppendingCycleDetector<OwnedOrDatabasePath>>,
     ),
-    appended_paths: Appendages<OwnedOrDatabasePath>,
+    appended_paths: Appendables<OwnedOrDatabasePath>,
     cycle_detector: SimilarPathDetector<Path>,
     max_work_per_phase: usize,
     #[cfg(feature = "copious-debugging")]
@@ -517,7 +517,7 @@ impl PathStitcher {
             copious_debugging!("    Initial node {}", node.display(graph));
             db.find_candidate_partial_paths_from_node(graph, partials, node, &mut candidate_paths);
         }
-        let mut appended_paths = Appendages::new();
+        let mut appended_paths = Appendables::new();
         let next_iteration = candidate_paths
             .iter()
             .filter_map(|partial_path| {
@@ -620,8 +620,9 @@ impl PathStitcher {
                 continue;
             }
             new_cycle_detector.append(&mut self.appended_paths, extension.into());
-            let cycles =
-                new_cycle_detector.is_cyclic(graph, partials, db, &mut self.appended_paths);
+            let cycles = new_cycle_detector
+                .is_cyclic(graph, partials, db, &mut self.appended_paths)
+                .expect("cyclic test failed when stitching paths");
             if !cycles
                 .into_iter()
                 .all(|c| c == Cyclicity::StrengthensPrecondition)
@@ -963,8 +964,9 @@ impl ForwardPartialPathStitcher {
                     continue;
                 }
                 new_cycle_detector.append(&mut self.appended_paths, extension.into());
-                let cycles =
-                    new_cycle_detector.is_cyclic(graph, partials, db, &mut self.appended_paths);
+                let cycles = new_cycle_detector
+                    .is_cyclic(graph, partials, db, &mut self.appended_paths)
+                    .expect("cyclic test failed when stitching partial paths");
                 if !cycles
                     .into_iter()
                     .all(|c| c == Cyclicity::StrengthensPrecondition)

stack-graphs/tests/it/cycles.rs

@@ -177,12 +177,14 @@ fn finding_simple_identity_cycle_is_detected() {
             cd.append(&mut edges, *edge);
             assert!(cd
                 .is_cyclic(&graph, &mut partials, ctx, &mut edges)
+                .unwrap()
                 .is_empty());
         }
         cd.append(&mut edges, edge(foo_def, r, 0));
         assert_eq!(
             vec![Cyclicity::StrengthensPostcondition],
             cd.is_cyclic(&graph, &mut partials, ctx, &mut edges)
+                .unwrap()
         );
     }
 
@@ -227,6 +229,7 @@ fn stitching_simple_identity_cycle_is_detected() {
         assert_eq!(
             vec![Cyclicity::StrengthensPostcondition],
             cd.is_cyclic(&graph, &mut partials, &mut db, &mut paths)
+                .unwrap()
         );
     }
 }
@@ -267,12 +270,14 @@ fn finding_composite_identity_cycle_is_detected() {
             cd.append(&mut edges, *edge);
             assert!(cd
                 .is_cyclic(&graph, &mut partials, ctx, &mut edges)
+                .unwrap()
                 .is_empty());
         }
         cd.append(&mut edges, edge(bar_ref, s, 0));
         assert_eq!(
             vec![Cyclicity::StrengthensPostcondition],
             cd.is_cyclic(&graph, &mut partials, ctx, &mut edges)
+                .unwrap()
         );
     }
 
@@ -320,6 +325,7 @@ fn stitching_composite_identity_cycle_is_detected() {
         assert_eq!(
             vec![Cyclicity::StrengthensPostcondition],
             cd.is_cyclic(&graph, &mut partials, &mut db, &mut paths)
+                .unwrap()
         );
     }
 }