Skip to content

Commit ff70d6c

Browse files
bobzhangclaude
andcommitted
fix(quickcheck): produce shrink candidates without recursing per block
Replace the recursion in `removes_array` and `removes_list` with a cursor over the block offsets. Both now walk `0, k, 2k, ..` directly — ascending for the array, descending for the list — so building the iterator is O(1) and yielding a candidate uses no stack beyond the call itself. The candidate sequences and `size_hint`s are the ones the previous commit pinned, which is why that test file is unchanged here. This fixes the three stack-overflow tests from the previous commit on js, wasm and wasm-gc, so a property test whose counterexample is a large collection now shrinks instead of dying with `RangeError: Maximum call stack size exceeded`. It is also much faster on the `List` side, because the old form rebuilt the candidate through one `concat` per level on the way back up, where the new one does a single `take`/`drop`/`concat`. First candidate from a 10,000-element collection, native release: | Collection | Before | After | | ---------- | --------: | --------: | | Array | 240.69 ns | 288.55 ns | | List | 1.32 ms | 213.65 µs | The array case is a hair slower — it now allocates the candidate through an array spread instead of returning a shared empty literal — and in exchange the `k == n` special case added in #4208 is gone, since taking a zero-length prefix already costs nothing. The benchmark goes back to 10,000 elements; #4208 had to lower it to 1,000 precisely because of the bug this fixes. Reviewed by Codex CLI together with the preceding test commit: "Verified ordering, exact size hints, non-divisible lengths, `k == n` without element copying, and cursor compatibility with single-use iterators. Independent old/new models agree for 5,050 input pairs." Signed-off-by: Codex CLI <codex@openai.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016jtBJHK749cpStav5ZMNCr
1 parent 6d03c2a commit ff70d6c

2 files changed

Lines changed: 33 additions & 20 deletions

File tree

quickcheck/shrink/utils.mbt

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,29 +25,42 @@ fn[T] deferred(f : () -> Iter[T]) -> Iter[T] {
2525
}
2626

2727
///|
28-
/// `n` must be `xs.length()`; the recursive call maintains that, and both
29-
/// guards below rely on it.
28+
/// Yields `xs` with one block of `k` consecutive elements removed: one
29+
/// candidate per block offset `0, k, 2k, ..`, up to the last offset that still
30+
/// leaves a whole block, in ascending order.
31+
///
32+
/// `n` must be `xs.length()`.
3033
fn[T] removes_array(k : Int, n : Int, xs : Array[T]) -> Iter[Array[T]] {
3134
guard k <= n else { [||] }
32-
// Dropping every element leaves the empty array; taking the `k`-element
33-
// prefix first would copy the whole array only to discard it. Past this
34-
// guard `xs[k:]` is non-empty.
35-
guard k < n else { [|[]|] }
36-
let xs2 = xs[:k].to_owned()
37-
let xs1 = xs[k:].to_owned()
38-
[|xs1|].add(removes_array(k, n - k, xs1).map(x => xs2 + x))
35+
let mut start = 0
36+
Iter::new(
37+
fn() {
38+
guard start + k <= n else { None }
39+
let candidate = [..xs[:start], ..xs[start + k:]]
40+
start += k
41+
Some(candidate)
42+
},
43+
size_hint=(n - k) / k + 1,
44+
)
3945
}
4046

4147
///|
48+
/// The `@list.List` counterpart of `removes_array`, except that the offsets
49+
/// descend: the block nearest the end is removed first.
50+
///
51+
/// `n` must be `xs.length()`.
4252
fn[T] removes_list(k : Int, n : Int, xs : @list.List[T]) -> Iter[@list.List[T]] {
4353
guard k <= n else { [||] }
44-
let xs_drop = xs.drop(k)
45-
if xs_drop.is_empty() {
46-
[|List([])|]
47-
} else {
48-
let xs_take = xs.take(k)
49-
removes_list(k, n - k, xs_drop).map(x => xs_take.concat(x)).add([|xs_drop|])
50-
}
54+
let mut start = (n - k) / k * k
55+
Iter::new(
56+
fn() {
57+
guard start >= 0 else { None }
58+
let candidate = xs.take(start).concat(xs.drop(start + k))
59+
start -= k
60+
Some(candidate)
61+
},
62+
size_hint=(n - k) / k + 1,
63+
)
5164
}
5265

5366
///|

quickcheck/shrink_collection_bench_test.mbt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,13 @@
1313
// limitations under the License.
1414

1515
///|
16-
test "bench shrink Array first candidate size=1000" (it : @bench.T) {
17-
let input = Array::make(1000, ())
16+
test "bench shrink Array first candidate size=10000" (it : @bench.T) {
17+
let input = Array::make(10000, ())
1818
it.bench(fn() { it.keep(@shrink.Shrink::shrink(input).next()) })
1919
}
2020

2121
///|
22-
test "bench shrink List first candidate size=1000" (it : @bench.T) {
23-
let input : @list.List[Unit] = List(Array::make(1000, ()))
22+
test "bench shrink List first candidate size=10000" (it : @bench.T) {
23+
let input : @list.List[Unit] = List(Array::make(10000, ()))
2424
it.bench(fn() { it.keep(@shrink.Shrink::shrink(input).next()) })
2525
}

0 commit comments

Comments
 (0)