- Improve robustness of cycle detection

process_vertex() now will panic (except for calls from is_cyclic()) if
the number of vertices seen don't match the graph's vertex count when
processing is complete.

Annoyingly, I can't figure out how to make the expected test for the
panic take a const value as an argument, so I've copy/pasted the string
into the test macro text.

Author: garyanaplan

src/iterators/topo.rs

@@ -10,6 +10,8 @@ extern crate alloc;
 #[cfg(feature = "no_std")]
 use alloc::vec::Vec;
 
+const PANIC_MSG: &str = "graph contains cycle(s)";
+
 #[derive(Debug)]
 /// Topological Iterator
 pub struct Topo<'a, T> {
@@ -43,7 +45,7 @@ impl<'a, T> Topo<'a, T> {
     /// Will return None if:
     ///
     /// * No vertices are left.
-    fn process_vertex(&mut self) -> Option<&'a VertexId> {
+    fn process_vertex(&mut self, check_cyclic: bool) -> Option<&'a VertexId> {
         match self.roots.pop() {
             Some(node) => {
                 self.vertices.push(node);
@@ -64,7 +66,12 @@ impl<'a, T> Topo<'a, T> {
                 }
                 Some(node)
             },
-            None => None,
+            None => {
+                if check_cyclic && self.vertices.len() != self.iterable.vertex_count() {
+                    panic!(PANIC_MSG);
+                }
+                None
+            },
         }
     }
 
@@ -76,7 +83,7 @@ impl<'a, T> Topo<'a, T> {
     /// It is a logic error to use this iterator after calling this function.
     pub fn is_cyclic(&mut self) -> bool {
         //Search until an answer is found.
-        while self.process_vertex().is_some() {}
+        while self.process_vertex(false).is_some() {}
 
         self.vertices.len() != self.iterable.vertex_count()
     }
@@ -92,7 +99,7 @@ impl<'a, T> Iterator for Topo<'a, T> {
     }
     fn next(&mut self) -> Option<Self::Item> {
         (0..self.size_hint().1.unwrap())
-            .filter_map(move |_| self.process_vertex())
+            .filter_map(move |_| self.process_vertex(true))
             .next()
     }
 }
@@ -153,4 +160,39 @@ mod tests {
 
         assert!(topo.is_cyclic());
     }
+
+    #[test]
+    #[should_panic(expected = "graph contains cycle(s)")]
+    fn is_cyclic_and_panic() {
+        let mut graph: Graph<usize> = Graph::new();
+
+        let v1 = graph.add_vertex(1);
+        let v2 = graph.add_vertex(2);
+        let v3 = graph.add_vertex(3);
+        let v4 = graph.add_vertex(4);
+        let v5 = graph.add_vertex(5);
+        let v6 = graph.add_vertex(6);
+        let v7 = graph.add_vertex(7);
+
+        graph.add_edge(&v1, &v4).unwrap();
+        graph.add_edge(&v2, &v4).unwrap();
+        graph.add_edge(&v2, &v5).unwrap();
+        graph.add_edge(&v3, &v5).unwrap();
+        graph.add_edge(&v4, &v6).unwrap();
+        graph.add_edge(&v4, &v7).unwrap();
+        graph.add_edge(&v5, &v6).unwrap();
+        graph.add_edge(&v6, &v7).unwrap();
+        graph.add_edge(&v7, &v2).unwrap();
+
+        let mut topo = graph.topo();
+
+        topo.next();
+        topo.next();
+        topo.next();
+        topo.next();
+        topo.next();
+        topo.next();
+        topo.next();
+    }
+
 }