Skip to content

Commit 2a85f9a

Browse files
fix: correct GridIntoIterator order, yen k=0 panic, and DfsReachable stack overflow
Co-authored-by: samueltardieu <44656+samueltardieu@users.noreply.github.com>
1 parent 3761d48 commit 2a85f9a

File tree

6 files changed

+80
-12
lines changed

6 files changed

+80
-12
lines changed

src/directed/dfs.rs

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -163,19 +163,21 @@ where
163163
type Item = N;
164164

165165
fn next(&mut self) -> Option<Self::Item> {
166-
let n = self.to_see.pop()?;
167-
if self.visited.contains(&n) {
168-
return self.next();
169-
}
170-
self.visited.insert(n.clone());
171-
let mut to_insert = Vec::new();
172-
for s in (self.successors)(&n) {
173-
if !self.visited.contains(&s) {
174-
to_insert.push(s.clone());
166+
loop {
167+
let n = self.to_see.pop()?;
168+
if self.visited.contains(&n) {
169+
continue;
170+
}
171+
self.visited.insert(n.clone());
172+
let mut to_insert = Vec::new();
173+
for s in (self.successors)(&n) {
174+
if !self.visited.contains(&s) {
175+
to_insert.push(s.clone());
176+
}
175177
}
178+
self.to_see.extend(to_insert.into_iter().rev());
179+
return Some(n);
176180
}
177-
self.to_see.extend(to_insert.into_iter().rev());
178-
Some(n)
179181
}
180182
}
181183

src/directed/yen.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ where
108108
IN: IntoIterator<Item = (N, C)>,
109109
FS: FnMut(&N) -> bool,
110110
{
111+
if k == 0 {
112+
return vec![];
113+
}
111114
let Some((n, c)) = dijkstra_internal(start, &mut successors, &mut success) else {
112115
return vec![];
113116
};

src/grid.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,13 @@ impl Iterator for GridIntoIterator {
579579
}
580580
}
581581
} else {
582-
self.grid.exclusions.pop()
582+
self.grid
583+
.exclusions
584+
.get_index(self.x)
585+
.inspect(|_| {
586+
self.x += 1;
587+
})
588+
.copied()
583589
}
584590
}
585591
}

tests/dfs-reach.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,20 @@ fn issue_511_branches() {
1414
let it = dfs_reach(0, |&n| [n + 2, n + 5].into_iter().filter(|&x| x <= 10));
1515
assert_eq!(vec![0, 2, 4, 6, 8, 10, 9, 7, 5], it.collect::<Vec<_>>());
1616
}
17+
18+
/// Test that `dfs_reach` does not stack overflow when many duplicate nodes
19+
/// pile up in the `to_see` stack (would previously recurse for each duplicate).
20+
#[test]
21+
fn no_stack_overflow_with_duplicates() {
22+
// Each node has N successors all pointing to the same next node, creating
23+
// many duplicates in to_see. With the old recursive implementation, the
24+
// recursion depth could equal the number of duplicates, causing stack overflow.
25+
let n = 200_usize;
26+
// Node 0 -> [1, 1, 1, ...] (n copies of 1)
27+
// Node k -> [k+1, k+1, k+1, ...] (n copies of k+1) for k < n
28+
// Node n -> []
29+
let result: Vec<usize> =
30+
dfs_reach(0usize, |&k| if k < n { vec![k + 1; n] } else { vec![] }).collect();
31+
let expected: Vec<usize> = (0..=n).collect();
32+
assert_eq!(result, expected);
33+
}

tests/grid.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,3 +614,27 @@ fn equality() {
614614
g2.add_vertex((0, 0));
615615
assert_eq!(g, g2);
616616
}
617+
618+
/// Test that iter() and into_iter() produce the same elements in the same order
619+
/// for both dense and sparse grids.
620+
#[test]
621+
fn iter_and_into_iter_same_order() {
622+
// Sparse grid (few vertices)
623+
let mut g = Grid::new(4, 4);
624+
g.add_vertex((0, 0));
625+
g.add_vertex((2, 1));
626+
g.add_vertex((1, 3));
627+
let via_iter: Vec<_> = g.iter().collect();
628+
let via_into_iter: Vec<_> = g.into_iter().collect();
629+
assert_eq!(via_iter, via_into_iter);
630+
631+
// Dense grid (many vertices, few exclusions)
632+
let mut g = Grid::new(4, 4);
633+
g.fill();
634+
g.remove_vertex((0, 0));
635+
g.remove_vertex((2, 1));
636+
g.remove_vertex((1, 3));
637+
let via_iter: Vec<_> = g.iter().collect();
638+
let via_into_iter: Vec<_> = g.into_iter().collect();
639+
assert_eq!(via_iter, via_into_iter);
640+
}

tests/yen.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,3 +177,19 @@ fn multiple_equal_cost_paths() {
177177
assert_eq!(paths[0], (vec!['A', 'B', 'D'], 2));
178178
assert_eq!(paths[1], (vec!['A', 'C', 'D'], 2));
179179
}
180+
181+
/// Test that k=0 returns an empty result without panicking.
182+
#[test]
183+
fn k_zero() {
184+
let result = yen(
185+
&'c',
186+
|c| match c {
187+
'c' => vec![('d', 3)],
188+
'd' => vec![],
189+
_ => panic!(""),
190+
},
191+
|c| *c == 'd',
192+
0,
193+
);
194+
assert!(result.is_empty());
195+
}

0 commit comments

Comments
 (0)