From c09eac898681e7c872b6faaed8f348e6a51b7d5f Mon Sep 17 00:00:00 2001 From: Huterenok Date: Wed, 2 Jul 2025 01:05:49 +0300 Subject: [PATCH 1/7] feat: ExactSizeIterator impl for IntoChunks --- src/groupbylazy.rs | 42 ++++++++++++++++++++++++++++++++++++++++++ src/size_hint.rs | 22 ++++++++++++++++++++++ tests/test_std.rs | 42 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+) diff --git a/src/groupbylazy.rs b/src/groupbylazy.rs index 5847c8f7d..3ea38c431 100644 --- a/src/groupbylazy.rs +++ b/src/groupbylazy.rs @@ -1,6 +1,8 @@ use alloc::vec::{self, Vec}; use std::cell::{Cell, RefCell}; +use crate::size_hint; + /// A trait to unify `FnMut` for `ChunkBy` with the chunk key in `IntoChunks` trait KeyFunction { type Key; @@ -526,6 +528,27 @@ where } } +impl IntoChunks +where + I: ExactSizeIterator, +{ + fn len(&self) -> usize { + // chunk_size cannot be null as it is checked in `Itertools::chunks` + let chunk_size = self.inner.borrow().key.size; + debug_assert!(chunk_size != 0); + + let iter_len = self.inner.borrow().iter.len(); + // TODO: MSRV of itertools is 1.63 when `uint::div_ceil` was stabilized in 1.73 + let d = iter_len / chunk_size; + let r = iter_len % chunk_size; + if r > 0 { + d + 1 + } else { + d + } + } +} + impl<'a, I> IntoIterator for &'a IntoChunks where I: Iterator, @@ -572,11 +595,30 @@ where first: Some(elt), }) } + + fn size_hint(&self) -> (usize, Option) { + // chunk_size cannot be null as it is checked in `Itertools::chunks` + let chunk_size = self.parent.inner.borrow().key.size; + debug_assert!(chunk_size != 0); + + size_hint::div_ceil_scalar(self.parent.inner.borrow().iter.size_hint(), chunk_size) + } +} + +impl<'a, I> ExactSizeIterator for Chunks<'a, I> +where + I: ExactSizeIterator + std::fmt::Debug, + I::Item: 'a + std::fmt::Debug, +{ + fn len(&self) -> usize { + self.parent.len() + } } /// An iterator for the elements in a single chunk. /// /// Iterator element type is `I::Item`. +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct Chunk<'a, I> where I: Iterator + 'a, diff --git a/src/size_hint.rs b/src/size_hint.rs index 6cfead7f2..55f1ecac3 100644 --- a/src/size_hint.rs +++ b/src/size_hint.rs @@ -57,6 +57,21 @@ pub fn mul_scalar(sh: SizeHint, x: usize) -> SizeHint { (low, hi) } +/// Correct ceiling division by `x` with a `SizeHint`. +#[inline] +#[track_caller] +pub fn div_ceil_scalar(sh: SizeHint, x: usize) -> SizeHint { + let (low, hi) = sh; + let (dlow, dhi) = (low / x, hi.map(|hi| hi / x)); + let (rlow, rhi) = (low % x, hi.map(|hi| hi % x)); + + let low = if rlow > 0 { dlow + 1 } else { dlow }; + let hi = dhi + .and_then(|dhi| rhi.map(|rhi| (dhi, rhi))) + .map(|(dhi, rhi)| if rhi > 0 { dhi + 1 } else { dhi }); + (low, hi) +} + /// Return the maximum #[inline] pub fn max(a: SizeHint, b: SizeHint) -> SizeHint { @@ -92,3 +107,10 @@ fn mul_size_hints() { assert_eq!(mul((3, Some(4)), (usize::MAX, None)), (usize::MAX, None)); assert_eq!(mul((3, None), (0, Some(0))), (0, Some(0))); } + +#[test] +fn div_ceil_size_scalar() { + assert_eq!(div_ceil_scalar((3, Some(4)), 2), (2, Some(2))); + assert_eq!(div_ceil_scalar((3, Some(4)), usize::MAX), (1, Some(1))); + assert_eq!(div_ceil_scalar((3, None), 2), (2, None)); +} diff --git a/tests/test_std.rs b/tests/test_std.rs index c0ee373aa..6fe17f427 100644 --- a/tests/test_std.rs +++ b/tests/test_std.rs @@ -1034,6 +1034,48 @@ fn chunks() { } } +#[test] +fn chunks_len() { + const TEST_CHUNK_SIZE: usize = 2; + + // test `::size_hint` + // (buffer, len of chunks) + [ + (vec![], (0, Some(0))), + (vec![1], (1, Some(1))), + (vec![1, 2], (1, Some(1))), + (vec![1, 2, 3], (2, Some(2))), + (vec![1, 2, 3, 4], (2, Some(2))), + ] + .iter() + .for_each(|(buf, expected_hint)| { + assert_eq!( + buf.into_iter() + .chunks(TEST_CHUNK_SIZE) + .into_iter() + .size_hint(), + *expected_hint + ) + }); + + // test `::len` + // (buffer, len of chunks) + [ + (vec![], 0), + (vec![1], 1), + (vec![1, 2], 1), + (vec![1, 2, 3], 2), + (vec![1, 2, 3, 4], 2), + ] + .iter() + .for_each(|(buf, expected_len)| { + assert_eq!( + buf.into_iter().chunks(TEST_CHUNK_SIZE).into_iter().len(), + *expected_len + ) + }); +} + #[test] fn concat_empty() { let data: Vec> = Vec::new(); From b462d329b5d06cdc18dbd150786c80e7f7e90049 Mon Sep 17 00:00:00 2001 From: Huterenok Date: Wed, 2 Jul 2025 11:25:10 +0300 Subject: [PATCH 2/7] fix: remove redundant Debug bounds --- src/groupbylazy.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/groupbylazy.rs b/src/groupbylazy.rs index 3ea38c431..779334121 100644 --- a/src/groupbylazy.rs +++ b/src/groupbylazy.rs @@ -607,8 +607,8 @@ where impl<'a, I> ExactSizeIterator for Chunks<'a, I> where - I: ExactSizeIterator + std::fmt::Debug, - I::Item: 'a + std::fmt::Debug, + I: ExactSizeIterator, + I::Item: 'a, { fn len(&self) -> usize { self.parent.len() From 05961fad5105215775f9dc2a58095148f91b3845 Mon Sep 17 00:00:00 2001 From: Huterenok Date: Fri, 4 Jul 2025 01:04:34 +0300 Subject: [PATCH 3/7] feat: impl ExactSizeIterator for Chunk --- src/groupbylazy.rs | 45 +++++++++++++++++++++++++++++++++ tests/test_std.rs | 62 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) diff --git a/src/groupbylazy.rs b/src/groupbylazy.rs index 779334121..c168553fa 100644 --- a/src/groupbylazy.rs +++ b/src/groupbylazy.rs @@ -652,4 +652,49 @@ where } self.parent.step(self.index) } + + fn size_hint(&self) -> (usize, Option) { + let inner = self.parent.inner.borrow(); + let (low, hi) = inner.iter.size_hint(); + // we add 1 because when generating `Chunk` from `::next` + // a step is taken forward and one element is stored in `first` + let (low, hi) = (low, hi.map(|hi| hi + 1)); + let chunk_size = inner.key.size; + + let retrieve_hint = |hint| { + // after each chunk `remaining_lan` will get smaller by `chunk_size` + if hint >= chunk_size { + // chunk is full + chunk_size + } else { + // last chunk which is not full + hint % chunk_size + } + }; + + (retrieve_hint(low), hi.map(retrieve_hint)) + } +} + +impl<'a, I> ExactSizeIterator for Chunk<'a, I> +where + I: ExactSizeIterator, + I::Item: 'a, +{ + fn len(&self) -> usize { + let inner = self.parent.inner.borrow(); + // we add 1 because when generating `Chunk` from `::next` + // a step is taken forward and one element is stored in `first` + let remaining_len = inner.iter.len() + 1; + let chunk_size = inner.key.size; + + // after each chunk `remaining_lan` will get smaller by `chunk_size` + if remaining_len >= chunk_size { + // chunk is full + chunk_size + } else { + // last chunk which is not full + remaining_len % chunk_size + } + } } diff --git a/tests/test_std.rs b/tests/test_std.rs index 6fe17f427..c7019eaf6 100644 --- a/tests/test_std.rs +++ b/tests/test_std.rs @@ -1074,6 +1074,68 @@ fn chunks_len() { *expected_len ) }); + + // test `::size_hint` + // (buffer, len of every chunk) + [ + (vec![], vec![]), + (vec![1], vec![1]), + (vec![1, 2], vec![2]), + (vec![1, 2, 3], vec![2, 1]), + (vec![1, 2, 3, 4], vec![2, 2]), + ] + .iter() + .for_each(|(buf, expected_lens)| { + assert_eq!( + buf.iter().chunks(TEST_CHUNK_SIZE).into_iter().len(), + expected_lens.len() + ); + + buf.into_iter() + .chunks(TEST_CHUNK_SIZE) + .into_iter() + .zip(expected_lens) + .for_each(|(chunk, &expected_len)| { + assert_eq!( + chunk.len(), + expected_len, + "chunk={:?} expected_len={:?}", + chunk.collect::>(), + expected_len + ) + }); + }); + + // test `::len` + // (buffer, len of every chunk) + [ + (vec![], vec![]), + (vec![1], vec![1]), + (vec![1, 2], vec![2]), + (vec![1, 2, 3], vec![2, 1]), + (vec![1, 2, 3, 4], vec![2, 2]), + ] + .iter() + .for_each(|(buf, expected_lens)| { + assert_eq!( + buf.iter().chunks(TEST_CHUNK_SIZE).into_iter().len(), + expected_lens.len() + ); + + buf.into_iter() + .chunks(TEST_CHUNK_SIZE) + .into_iter() + .zip(expected_lens) + .for_each(|(chunk, &expected_len)| { + assert_eq!( + chunk.len(), + expected_len, + "chunk={:?} expected_len={:?}", + chunk.collect::>(), + expected_len + ) + }); + }); } #[test] From 319647acb4e3f7a21d01aa5bba22dc1486b96cf7 Mon Sep 17 00:00:00 2001 From: Huterenok Date: Fri, 4 Jul 2025 03:29:37 +0300 Subject: [PATCH 4/7] fix: remove ExactSizeIterator for Chunk --- src/groupbylazy.rs | 81 +++++------------------------------ src/size_hint.rs | 22 ++++++++-- tests/quick.rs | 18 ++++++++ tests/test_std.rs | 104 --------------------------------------------- 4 files changed, 47 insertions(+), 178 deletions(-) diff --git a/src/groupbylazy.rs b/src/groupbylazy.rs index 0da718ebd..064b18882 100644 --- a/src/groupbylazy.rs +++ b/src/groupbylazy.rs @@ -585,27 +585,6 @@ where } } -impl IntoChunks -where - I: ExactSizeIterator, -{ - fn len(&self) -> usize { - // chunk_size cannot be null as it is checked in `Itertools::chunks` - let chunk_size = self.inner.borrow().key.size; - debug_assert!(chunk_size != 0); - - let iter_len = self.inner.borrow().iter.len(); - // TODO: MSRV of itertools is 1.63 when `uint::div_ceil` was stabilized in 1.73 - let d = iter_len / chunk_size; - let r = iter_len % chunk_size; - if r > 0 { - d + 1 - } else { - d - } - } -} - impl<'a, I> IntoIterator for &'a IntoChunks where I: Iterator, @@ -662,11 +641,19 @@ where } fn size_hint(&self) -> (usize, Option) { + let inner = self.parent.inner.borrow(); // chunk_size cannot be null as it is checked in `Itertools::chunks` - let chunk_size = self.parent.inner.borrow().key.size; + let chunk_size = inner.key.size; debug_assert!(chunk_size != 0); - size_hint::div_ceil_scalar(self.parent.inner.borrow().iter.size_hint(), chunk_size) + match self.parent.index.get() { + // inner iter has not been used yet + 0 => size_hint::div_ceil_scalar(inner.iter.size_hint(), chunk_size), + // inner iter has already been used and its length is < original_length - 1 + // so we need to return `div_ceil_scalar` - 1 but due to `GroupInnner::step_current` + // logic we need to cover chunk_size * n case to avoid overflow + _ => size_hint::div_scalar(inner.iter.size_hint(), chunk_size), + } } } @@ -675,9 +662,6 @@ where I: ExactSizeIterator, I::Item: 'a, { - fn len(&self) -> usize { - self.parent.len() - } } /// An iterator for the elements in a single chunk. @@ -718,49 +702,4 @@ where } self.parent.step(self.index) } - - fn size_hint(&self) -> (usize, Option) { - let inner = self.parent.inner.borrow(); - let (low, hi) = inner.iter.size_hint(); - // we add 1 because when generating `Chunk` from `::next` - // a step is taken forward and one element is stored in `first` - let (low, hi) = (low, hi.map(|hi| hi + 1)); - let chunk_size = inner.key.size; - - let retrieve_hint = |hint| { - // after each chunk `remaining_lan` will get smaller by `chunk_size` - if hint >= chunk_size { - // chunk is full - chunk_size - } else { - // last chunk which is not full - hint % chunk_size - } - }; - - (retrieve_hint(low), hi.map(retrieve_hint)) - } -} - -impl<'a, I> ExactSizeIterator for Chunk<'a, I> -where - I: ExactSizeIterator, - I::Item: 'a, -{ - fn len(&self) -> usize { - let inner = self.parent.inner.borrow(); - // we add 1 because when generating `Chunk` from `::next` - // a step is taken forward and one element is stored in `first` - let remaining_len = inner.iter.len() + 1; - let chunk_size = inner.key.size; - - // after each chunk `remaining_lan` will get smaller by `chunk_size` - if remaining_len >= chunk_size { - // chunk is full - chunk_size - } else { - // last chunk which is not full - remaining_len % chunk_size - } - } } diff --git a/src/size_hint.rs b/src/size_hint.rs index 55f1ecac3..5657c6bd0 100644 --- a/src/size_hint.rs +++ b/src/size_hint.rs @@ -72,6 +72,13 @@ pub fn div_ceil_scalar(sh: SizeHint, x: usize) -> SizeHint { (low, hi) } +/// Correct division by `x` with a `SizeHint`. +#[inline] +#[track_caller] +pub fn div_scalar((low, hi): SizeHint, x: usize) -> SizeHint { + (low.saturating_div(x), hi.map(|hi| hi.saturating_div(x))) +} + /// Return the maximum #[inline] pub fn max(a: SizeHint, b: SizeHint) -> SizeHint { @@ -110,7 +117,16 @@ fn mul_size_hints() { #[test] fn div_ceil_size_scalar() { - assert_eq!(div_ceil_scalar((3, Some(4)), 2), (2, Some(2))); - assert_eq!(div_ceil_scalar((3, Some(4)), usize::MAX), (1, Some(1))); - assert_eq!(div_ceil_scalar((3, None), 2), (2, None)); + assert_eq!(div_ceil_scalar((4, Some(4)), 2), (2, Some(2))); + assert_eq!(div_ceil_scalar((3, Some(3)), 2), (2, Some(2))); + assert_eq!(div_ceil_scalar((4, Some(4)), usize::MAX), (1, Some(1))); + assert_eq!(div_ceil_scalar((4, None), 2), (2, None)); +} + +#[test] +fn div_size_scalar() { + assert_eq!(div_scalar((4, Some(4)), 2), (2, Some(2))); + assert_eq!(div_scalar((3, Some(3)), 2), (1, Some(1))); + assert_eq!(div_scalar((4, Some(4)), usize::MAX), (0, Some(0))); + assert_eq!(div_scalar((4, None), 2), (2, None)); } diff --git a/tests/quick.rs b/tests/quick.rs index 0af73778a..b1924a42d 100644 --- a/tests/quick.rs +++ b/tests/quick.rs @@ -1116,6 +1116,24 @@ quickcheck! { } } +quickcheck! { + fn chunks_exact_size(a: Vec, size: u8) -> bool { + let mut size = size; + if size == 0 { + size += 1; + } + exact_size(a.into_iter().chunks(size as usize).into_iter()) + } + + fn chunks_correct_size_hint(a: Vec, size: u8) -> bool { + let mut size = size; + if size == 0 { + size += 1; + } + correct_size_hint(a.into_iter().chunks(size as usize).into_iter()) + } +} + // tuple iterators quickcheck! { fn equal_circular_tuple_windows_1(a: Vec) -> bool { diff --git a/tests/test_std.rs b/tests/test_std.rs index c7019eaf6..c0ee373aa 100644 --- a/tests/test_std.rs +++ b/tests/test_std.rs @@ -1034,110 +1034,6 @@ fn chunks() { } } -#[test] -fn chunks_len() { - const TEST_CHUNK_SIZE: usize = 2; - - // test `::size_hint` - // (buffer, len of chunks) - [ - (vec![], (0, Some(0))), - (vec![1], (1, Some(1))), - (vec![1, 2], (1, Some(1))), - (vec![1, 2, 3], (2, Some(2))), - (vec![1, 2, 3, 4], (2, Some(2))), - ] - .iter() - .for_each(|(buf, expected_hint)| { - assert_eq!( - buf.into_iter() - .chunks(TEST_CHUNK_SIZE) - .into_iter() - .size_hint(), - *expected_hint - ) - }); - - // test `::len` - // (buffer, len of chunks) - [ - (vec![], 0), - (vec![1], 1), - (vec![1, 2], 1), - (vec![1, 2, 3], 2), - (vec![1, 2, 3, 4], 2), - ] - .iter() - .for_each(|(buf, expected_len)| { - assert_eq!( - buf.into_iter().chunks(TEST_CHUNK_SIZE).into_iter().len(), - *expected_len - ) - }); - - // test `::size_hint` - // (buffer, len of every chunk) - [ - (vec![], vec![]), - (vec![1], vec![1]), - (vec![1, 2], vec![2]), - (vec![1, 2, 3], vec![2, 1]), - (vec![1, 2, 3, 4], vec![2, 2]), - ] - .iter() - .for_each(|(buf, expected_lens)| { - assert_eq!( - buf.iter().chunks(TEST_CHUNK_SIZE).into_iter().len(), - expected_lens.len() - ); - - buf.into_iter() - .chunks(TEST_CHUNK_SIZE) - .into_iter() - .zip(expected_lens) - .for_each(|(chunk, &expected_len)| { - assert_eq!( - chunk.len(), - expected_len, - "chunk={:?} expected_len={:?}", - chunk.collect::>(), - expected_len - ) - }); - }); - - // test `::len` - // (buffer, len of every chunk) - [ - (vec![], vec![]), - (vec![1], vec![1]), - (vec![1, 2], vec![2]), - (vec![1, 2, 3], vec![2, 1]), - (vec![1, 2, 3, 4], vec![2, 2]), - ] - .iter() - .for_each(|(buf, expected_lens)| { - assert_eq!( - buf.iter().chunks(TEST_CHUNK_SIZE).into_iter().len(), - expected_lens.len() - ); - - buf.into_iter() - .chunks(TEST_CHUNK_SIZE) - .into_iter() - .zip(expected_lens) - .for_each(|(chunk, &expected_len)| { - assert_eq!( - chunk.len(), - expected_len, - "chunk={:?} expected_len={:?}", - chunk.collect::>(), - expected_len - ) - }); - }); -} - #[test] fn concat_empty() { let data: Vec> = Vec::new(); From 6115b85f306490d1e6762c55ede641c517f8d343 Mon Sep 17 00:00:00 2001 From: Huterenok Date: Fri, 4 Jul 2025 03:53:40 +0300 Subject: [PATCH 5/7] docs: change comments in Chunks::size_hint --- src/groupbylazy.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/groupbylazy.rs b/src/groupbylazy.rs index 064b18882..095b684f0 100644 --- a/src/groupbylazy.rs +++ b/src/groupbylazy.rs @@ -646,13 +646,14 @@ where let chunk_size = inner.key.size; debug_assert!(chunk_size != 0); - match self.parent.index.get() { + if self.parent.index.get() == 0 { // inner iter has not been used yet - 0 => size_hint::div_ceil_scalar(inner.iter.size_hint(), chunk_size), - // inner iter has already been used and its length is < original_length - 1 + size_hint::div_ceil_scalar(inner.iter.size_hint(), chunk_size) + } else { + // inner iter has already been used and its length is < original length // so we need to return `div_ceil_scalar` - 1 but due to `GroupInnner::step_current` // logic we need to cover chunk_size * n case to avoid overflow - _ => size_hint::div_scalar(inner.iter.size_hint(), chunk_size), + size_hint::div_scalar(inner.iter.size_hint(), chunk_size) } } } From 779fb2632f9c971e10cd40787617aff8596d7b23 Mon Sep 17 00:00:00 2001 From: Huterenok Date: Fri, 4 Jul 2025 09:00:55 +0300 Subject: [PATCH 6/7] docs: change comments in Chunks::size_hint --- src/groupbylazy.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/groupbylazy.rs b/src/groupbylazy.rs index 095b684f0..b3b6a1581 100644 --- a/src/groupbylazy.rs +++ b/src/groupbylazy.rs @@ -651,8 +651,7 @@ where size_hint::div_ceil_scalar(inner.iter.size_hint(), chunk_size) } else { // inner iter has already been used and its length is < original length - // so we need to return `div_ceil_scalar` - 1 but due to `GroupInnner::step_current` - // logic we need to cover chunk_size * n case to avoid overflow + // so we need to return result of floor division size_hint::div_scalar(inner.iter.size_hint(), chunk_size) } } From a3cf04d98ebbdbb2a7a1fe3c485f9f2ace6acede Mon Sep 17 00:00:00 2001 From: Huterenok Date: Fri, 4 Jul 2025 09:24:00 +0300 Subject: [PATCH 7/7] docs: change comments in Chunks::size_hint --- src/groupbylazy.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/groupbylazy.rs b/src/groupbylazy.rs index b3b6a1581..61d502261 100644 --- a/src/groupbylazy.rs +++ b/src/groupbylazy.rs @@ -651,7 +651,9 @@ where size_hint::div_ceil_scalar(inner.iter.size_hint(), chunk_size) } else { // inner iter has already been used and its length is < original length - // so we need to return result of floor division + // so we need to return result of floor division because of `IntoChunks::step` + // impl which consumes only 1 element on first `next` and then `chunk_size` + // every nedxt time size_hint::div_scalar(inner.iter.size_hint(), chunk_size) } }