Skip to content
Open
52 changes: 52 additions & 0 deletions src/arrays.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
use crate::Itertools;

macro_rules! const_assert_positive {
($N: ty) => {
trait StaticAssert<const N: usize> {
const ASSERT: bool;
}

impl<const N: usize> StaticAssert<N> for () {
const ASSERT: bool = {
assert!(N > 0);
true
};
}

assert!(<() as StaticAssert<N>>::ASSERT);
};
}

/// An iterator that groups the items in arrays of const generic size `N`.
///
/// See [`.next_array()`](crate::Itertools::next_array) for details.
#[derive(Debug, Clone)]
pub struct Arrays<I: Iterator, const N: usize> {
iter: I,
}
Comment on lines +24 to +26

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It strikes me as a code smell that N doesn't appear in the definition here. Shouldn't partial be an ArrayBuilder<T, N>?

@ronnodas ronnodas Mar 5, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making it ArrayBuilder<T, N> a priori makes sense, but I had trouble with the bounds for the impl Clone in that case. Note that MaybeUninit<T>: Clone requires T: Copy.

Maybe it could also be array::IntoIter<T, N> or a hypothetical ArrayBuilder::IntoIter<T, N> (this is close to what the unstable Iterator::ArrayChunks does) but I think the unsafe code for that would need to be more careful.

The state you need to keep track of doesn't really depend on N (except partial.len() <= N but this is sort of incidental). However Arrays needs to have N as a parameter for Arrays::Item to be well-defined.

Suggestions?

@phimuemue phimuemue Jul 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, should be ArrayBuilder, because this brings it closer to no_alloc.

Maybe this array builder could also be re-used in next.

Can you remember/share the exact problem?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@phimuemue If it holds an ArrayBuilder<T> then it won't be Clone without T : Copy. But we should have Arrays<I, N>: Clone when I: Clone and I::Item : Clone.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Then, let's implement it without partial/remaining for now.

(Because if we extend our ArrayBuilder further and further, we land at (a less tested version of) ArrayVec.)


impl<I: Iterator, const N: usize> Arrays<I, N> {
pub(crate) fn new(iter: I) -> Self {
const_assert_positive!(N);

// TODO should we use iter.fuse() instead?
Self { iter }
}
}

impl<I: Iterator, const N: usize> Iterator for Arrays<I, N> {
type Item = [I::Item; N];

fn next(&mut self) -> Option<Self::Item> {
self.iter.next_array()
}

fn size_hint(&self) -> (usize, Option<usize>) {
// also verified in `new()`
const_assert_positive!(N);
let (lo, hi) = self.iter.size_hint();
Comment thread
ronnodas marked this conversation as resolved.
(lo / N, hi.map(|hi| hi / N))
}
}

impl<I: ExactSizeIterator, const N: usize> ExactSizeIterator for Arrays<I, N> {}
41 changes: 41 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ pub mod structs {
};
pub use crate::all_equal_value_err::AllEqualValueError;
pub use crate::array_impl::{ArrayWindows, CircularArrayWindows};
pub use crate::arrays::Arrays;
#[cfg(feature = "use_alloc")]
pub use crate::combinations::{ArrayCombinations, Combinations};
#[cfg(feature = "use_alloc")]
Expand Down Expand Up @@ -179,6 +180,7 @@ pub use crate::with_position::Position;
pub use crate::ziptuple::multizip;
mod adaptors;
mod array_impl;
mod arrays;
mod either_or_both;
pub use crate::either_or_both::EitherOrBoth;
#[doc(hidden)]
Expand Down Expand Up @@ -790,6 +792,45 @@ pub trait Itertools: Iterator {
groupbylazy::new_chunks(self, size)
}

/// Return an iterator that groups the items in arrays of const generic size `N`.
///
/// `N == 0` is a compile-time (but post-monomorphization) error.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really important for users that it is post-monomorphization?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It won't be caught by cargo check, for example, which seems relevant to users.

///
/// See also the method [`.next_array()`](Itertools::next_array).
///
/// ```rust
/// use itertools::Itertools;
/// let mut v = Vec::new();
/// for [a, b] in (1..5).arrays() {
/// v.push([a, b]);
/// }
/// assert_eq!(v, vec![[1, 2], [3, 4]]);
///
/// // this requires a type hint
/// let it = (1..7).arrays::<3>();
/// itertools::assert_equal(it, vec![[1, 2, 3], [4, 5, 6]]);
///
/// // you can also specify the complete type

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not encourage people to spell iterator types

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that such an example is included in the docs for tuple_windows(), tuples(), tuple_combinations() and array_combinations(). Should I still remove this or keep it for consistency?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you wish.

/// use itertools::Arrays;
/// use std::ops::Range;
///
/// let it: Arrays<Range<u32>, 3> = (1..7).arrays();
/// itertools::assert_equal(it, vec![[1, 2, 3], [4, 5, 6]]);
/// ```
///
/// ```compile_fail
/// use itertools::Itertools;
///
/// let mut it = (1..5).arrays::<0>();
/// assert_eq!(Some([]), it.next());
/// ```
fn arrays<const N: usize>(self) -> Arrays<Self, N>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO array_chunks is the better name, since it differentiates it from array_windows.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To add context, I was trying to avoid triggering the unstable-name-collisions lint, because of rust-lang/rust#100450. Should I change it back to array_chunks?

where
Self: Sized,
{
Arrays::new(self)
}

/// Return an iterator over all contiguous windows producing tuples of
/// a specific size (up to 12).
///
Expand Down
10 changes: 10 additions & 0 deletions tests/quick.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1373,6 +1373,16 @@ quickcheck! {
}
}

quickcheck! {
fn arrays_exact_size_1(a: Vec<u8>) -> bool {
exact_size(a.iter().arrays::<1>())
}

fn arrays_exact_size_4(a: Vec<u8>) -> bool {
exact_size(a.iter().arrays::<4>())
}
}

// with_position
quickcheck! {
fn with_position_exact_size_1(a: Vec<u8>) -> bool {
Expand Down
Loading