From f957eb7ec96875e1d875d7e8a7b050e2457247fb Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 2 Jun 2026 08:19:50 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(stdlib):=20return=20error=20instead=20o?= =?UTF-8?q?f=20panicking=20on=20out-of-range=20combinations=20size=20?= =?UTF-8?q?=F0=9F=A7=AE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- ndc_stdlib/src/sequence.rs | 9 +++------ ndc_vm/src/iterator.rs | 15 +++++++++++++-- .../900_bugs/bug0027_combinations_huge_k.ndc | 13 +++++++++++++ .../900_bugs/bug0028_combinations_negative_k.ndc | 7 +++++++ .../900_bugs/bug0029_permutations_negative_k.ndc | 5 +++++ .../bug0030_subsequences_negative_length.ndc | 6 ++++++ 6 files changed, 47 insertions(+), 8 deletions(-) create mode 100644 tests/functional/programs/900_bugs/bug0027_combinations_huge_k.ndc create mode 100644 tests/functional/programs/900_bugs/bug0028_combinations_negative_k.ndc create mode 100644 tests/functional/programs/900_bugs/bug0029_permutations_negative_k.ndc create mode 100644 tests/functional/programs/900_bugs/bug0030_subsequences_negative_length.ndc diff --git a/ndc_stdlib/src/sequence.rs b/ndc_stdlib/src/sequence.rs index ea6c7f8d..21879c74 100644 --- a/ndc_stdlib/src/sequence.rs +++ b/ndc_stdlib/src/sequence.rs @@ -560,8 +560,7 @@ mod inner { /// Returns the `k` sized combinations of the given sequence `seq` as a lazy iterator of tuples. #[function(return_type = Iterator)] - pub fn combinations(seq: SeqValue, k: i64) -> anyhow::Result { - let k = k as usize; + pub fn combinations(seq: SeqValue, k: usize) -> anyhow::Result { let iter = CombinationsIter::new(seq, k) .ok_or_else(|| anyhow!("combinations requires a sequence"))?; Ok(Value::iterator(iter.into_shared())) @@ -576,8 +575,7 @@ mod inner { /// Returns the `k` sized permutations of the given sequence `seq` as a list of tuples. #[function(return_type = Vec<_>)] - pub fn permutations(seq: SeqValue, k: i64) -> anyhow::Result { - let k = k as usize; + pub fn permutations(seq: SeqValue, k: usize) -> anyhow::Result { Ok(Value::list( seq.try_into_iter() .ok_or_else(|| anyhow!("permutations requires a sequence"))? @@ -738,8 +736,7 @@ mod inner { /// Return a list that represents the powerset of the elements of `seq` that are exactly `length` long. #[function(name = "subsequences", return_type = Vec<_>)] - pub fn subsequences_len(seq: SeqValue, length: i64) -> anyhow::Result { - let length = length as usize; + pub fn subsequences_len(seq: SeqValue, length: usize) -> anyhow::Result { Ok(Value::list( seq.try_into_iter() .ok_or_else(|| anyhow!("subsequences requires a sequence"))? diff --git a/ndc_vm/src/iterator.rs b/ndc_vm/src/iterator.rs index f4d2c25d..6a37667a 100644 --- a/ndc_vm/src/iterator.rs +++ b/ndc_vm/src/iterator.rs @@ -398,6 +398,11 @@ pub struct CombinationsIter { buffer: Vec, /// `Some` only for lazy (Shared) sources; set to `None` once exhausted. source: Option, + /// Combination size. The `indices` vector is only allocated once the + /// buffer is confirmed to hold at least `k` elements, so an absurdly + /// large `k` (e.g. `i64::MAX`) over a short source yields nothing + /// instead of overflowing the allocator with `(0..k).collect()`. + k: usize, indices: Vec, first: bool, } @@ -413,7 +418,8 @@ impl CombinationsIter { Some(Self { buffer, source, - indices: (0..k).collect(), + k, + indices: Vec::new(), first: true, }) } @@ -445,7 +451,7 @@ impl CombinationsIter { impl VmIterator for CombinationsIter { fn next(&mut self) -> Option { - let k = self.indices.len(); + let k = self.k; if k == 0 { return if self.first { @@ -461,6 +467,10 @@ impl VmIterator for CombinationsIter { if !self.ensure_index(k - 1) { return None; } + // The buffer now holds at least `k` elements, so this allocation + // is bounded by data we actually materialised — no capacity + // overflow even when `k` is enormous. + self.indices = (0..k).collect(); } else { // For lazy sources: pull one more element before choosing the // pivot. Without this, when `indices[k-1]` reaches the end of @@ -510,6 +520,7 @@ impl VmIterator for CombinationsIter { Some(Rc::new(RefCell::new(Self { buffer: self.buffer.clone(), source: source_copy, + k: self.k, indices: self.indices.clone(), first: self.first, }))) diff --git a/tests/functional/programs/900_bugs/bug0027_combinations_huge_k.ndc b/tests/functional/programs/900_bugs/bug0027_combinations_huge_k.ndc new file mode 100644 index 00000000..030f9b65 --- /dev/null +++ b/tests/functional/programs/900_bugs/bug0027_combinations_huge_k.ndc @@ -0,0 +1,13 @@ +// `combinations(seq, k)` built its index vector eagerly with +// `(0..k).collect()`. A huge `k` (e.g. `i64::MAX`) over a short source +// asked the allocator for an `i64::MAX`-element `Vec` and aborted +// with "capacity overflow". The index vector is now only allocated once +// the buffer is confirmed to hold at least `k` elements, so an +// over-large `k` simply yields no combinations instead of panicking. +assert_eq(combinations([1, 2, 3], 9223372036854775807).list(), []); + +// `k` larger than the source length yields nothing (boundary just above n). +assert_eq(combinations([1, 2, 3], 4).list(), []); + +// Valid combinations are still produced correctly. +assert_eq([1, 2, 3, 4].combinations(2).list(), [(1, 2), (1, 3), (1, 4), (2, 3), (2, 4), (3, 4)]); diff --git a/tests/functional/programs/900_bugs/bug0028_combinations_negative_k.ndc b/tests/functional/programs/900_bugs/bug0028_combinations_negative_k.ndc new file mode 100644 index 00000000..9c134ae6 --- /dev/null +++ b/tests/functional/programs/900_bugs/bug0028_combinations_negative_k.ndc @@ -0,0 +1,7 @@ +// `combinations(seq, k)` took `k` as an `i64` and cast it with `k as usize`, +// turning a negative `k` into `usize::MAX` and aborting with "capacity +// overflow" while building the index vector. `k` is now a `usize` argument, +// so a negative value is rejected by argument conversion with a graceful +// error — matching the sibling `windows`/`chunks`/`take` functions. +// expect-error: expected a non-negative integer, but the value was negative +combinations([1, 2, 3], -1); diff --git a/tests/functional/programs/900_bugs/bug0029_permutations_negative_k.ndc b/tests/functional/programs/900_bugs/bug0029_permutations_negative_k.ndc new file mode 100644 index 00000000..64b08879 --- /dev/null +++ b/tests/functional/programs/900_bugs/bug0029_permutations_negative_k.ndc @@ -0,0 +1,5 @@ +// `permutations(seq, k)` cast `k as usize`, silently turning a negative `k` +// into `usize::MAX` and returning an empty list. `k` is now a `usize` +// argument, so a negative value is rejected with a graceful error instead. +// expect-error: expected a non-negative integer, but the value was negative +permutations([1, 2, 3], -1); diff --git a/tests/functional/programs/900_bugs/bug0030_subsequences_negative_length.ndc b/tests/functional/programs/900_bugs/bug0030_subsequences_negative_length.ndc new file mode 100644 index 00000000..b16c05ce --- /dev/null +++ b/tests/functional/programs/900_bugs/bug0030_subsequences_negative_length.ndc @@ -0,0 +1,6 @@ +// `subsequences(seq, length)` cast `length as usize`, silently turning a +// negative `length` into `usize::MAX` and returning an empty list. `length` +// is now a `usize` argument, so a negative value is rejected with a graceful +// error instead. +// expect-error: expected a non-negative integer, but the value was negative +subsequences([1, 2, 3], -1); From baa2d88d09f4b15cb0d16820c101c85354122e99 Mon Sep 17 00:00:00 2001 From: Tim Fennis Date: Tue, 2 Jun 2026 08:42:35 +0200 Subject: [PATCH 2/2] fix(vm): keep CombinationsIter exhausted until indices are initialized Leaving `first = false` after a failed `ensure_index` relied on a subtle non-local invariant (ensure_index nulling the source) to keep the else branch from indexing the still-empty `indices`. Clear `first` only after `indices` is built, so a re-poll re-enters the first-call branch and returns None idempotently. Adds a re-poll regression case. Co-Authored-By: Claude Opus 4.8 (1M context) --- ndc_vm/src/iterator.rs | 9 +++++++-- .../programs/900_bugs/bug0027_combinations_huge_k.ndc | 10 ++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/ndc_vm/src/iterator.rs b/ndc_vm/src/iterator.rs index 6a37667a..36d17ec0 100644 --- a/ndc_vm/src/iterator.rs +++ b/ndc_vm/src/iterator.rs @@ -463,14 +463,19 @@ impl VmIterator for CombinationsIter { } if self.first { - self.first = false; if !self.ensure_index(k - 1) { + // Source can't supply `k` elements: stay exhausted. `first` + // is left set so a re-poll returns `None` again instead of + // falling into the else branch and indexing the still-empty + // `indices`. return None; } // The buffer now holds at least `k` elements, so this allocation // is bounded by data we actually materialised — no capacity - // overflow even when `k` is enormous. + // overflow even when `k` is enormous. Only now is it safe to + // leave the first-call branch. self.indices = (0..k).collect(); + self.first = false; } else { // For lazy sources: pull one more element before choosing the // pivot. Without this, when `indices[k-1]` reaches the end of diff --git a/tests/functional/programs/900_bugs/bug0027_combinations_huge_k.ndc b/tests/functional/programs/900_bugs/bug0027_combinations_huge_k.ndc index 030f9b65..44fb00d0 100644 --- a/tests/functional/programs/900_bugs/bug0027_combinations_huge_k.ndc +++ b/tests/functional/programs/900_bugs/bug0027_combinations_huge_k.ndc @@ -9,5 +9,15 @@ assert_eq(combinations([1, 2, 3], 9223372036854775807).list(), []); // `k` larger than the source length yields nothing (boundary just above n). assert_eq(combinations([1, 2, 3], 4).list(), []); +// Re-polling an exhausted over-large combination stays empty rather than +// indexing a never-initialised index vector. Covers both eager and lazy +// (enumerate) sources, drained twice. +let eager = combinations([1, 2, 3], 5); +assert_eq(eager.list(), []); +assert_eq(eager.list(), []); +let lazy = combinations([1, 2, 3].enumerate(), 5); +assert_eq(lazy.list(), []); +assert_eq(lazy.list(), []); + // Valid combinations are still produced correctly. assert_eq([1, 2, 3, 4].combinations(2).list(), [(1, 2), (1, 3), (1, 4), (2, 3), (2, 4), (3, 4)]);