Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support owned iterators for GroupBy and IntoChunks into_iter() #500

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 108 additions & 46 deletions src/groupbylazy.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use alloc::rc::Rc;
use alloc::vec::{self, Vec};
use std::cell::{Cell, RefCell};
use core::cell::{Cell, RefCell};
use core::ops::Deref;

/// A trait to unify `FnMut` for `ChunkBy` with the chunk key in `IntoChunks`
trait KeyFunction<A> {
Expand Down Expand Up @@ -117,7 +119,8 @@
if elt.is_none() && client == self.oldest_buffered_group {
// FIXME: VecDeque is unfortunately not zero allocation when empty,
// so we do this job manually.
// `bottom_group..oldest_buffered_group` is unused, and if it's large enough, erase it.
// `bottom_group..oldest_buffered_group` is unused, and if it's large enough,
// erase it.
self.oldest_buffered_group += 1;
// skip forward further empty queues too
while self
Expand Down Expand Up @@ -202,7 +205,8 @@
}

fn push_next_group(&mut self, group: Vec<I::Item>) {
// When we add a new buffered group, fill up slots between oldest_buffered_group and top_group
// When we add a new buffered group, fill up slots between oldest_buffered_group
// and top_group
while self.top_group - self.bottom_group > self.buffer.len() {
if self.buffer.is_empty() {
self.bottom_group += 1;
Expand Down Expand Up @@ -353,46 +357,73 @@
}
}

impl<K, I, F> IntoIterator for ChunkBy<K, I, F>
where
I: Iterator,
F: FnMut(&I::Item) -> K,
K: PartialEq,
{
type Item = (K, Group<K, I, F, Rc<Self>>);
type IntoIter = Groups<K, I, F, Rc<Self>>;

fn into_iter(self) -> Self::IntoIter {
Groups {
parent: Rc::new(self),
}
}
}

impl<'a, K, I, F> IntoIterator for &'a ChunkBy<K, I, F>
where
I: Iterator,
I::Item: 'a,
F: FnMut(&I::Item) -> K,
K: PartialEq,
{
type Item = (K, Group<'a, K, I, F>);
type IntoIter = Groups<'a, K, I, F>;
type Item = (K, Group<K, I, F, Self>);
type IntoIter = Groups<K, I, F, Self>;

fn into_iter(self) -> Self::IntoIter {
Groups { parent: self }
}
}

impl<K, I, F> ChunkBy<K, I, F>
where
I: Iterator,
F: FnMut(&I::Item) -> K,
K: PartialEq,
{
/// This is pretty much the same as `.into_iter()`, except it uses
/// references in the underlying iterators instead of reference counts,
/// resulting in one less allocation. You may however hit lifetime
/// errors if you require full ownership.
pub fn borrowed_iter(&self) -> <&Self as IntoIterator>::IntoIter {
self.into_iter()
}
}

/// An iterator that yields the Group iterators.
///
/// Iterator element type is `(K, Group)`:
/// the group's key `K` and the group's iterator.
///
/// See [`.chunk_by()`](crate::Itertools::chunk_by) for more information.
#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
pub struct Groups<'a, K, I, F>
pub struct Groups<K, I, F, D: Deref<Target = ChunkBy<K, I, F>>>
where
I: Iterator + 'a,
I::Item: 'a,
K: 'a,
F: 'a,
I: Iterator,
{
parent: &'a ChunkBy<K, I, F>,
parent: D,
}

impl<'a, K, I, F> Iterator for Groups<'a, K, I, F>
impl<K, I, F, D> Iterator for Groups<K, I, F, D>
where
I: Iterator,
I::Item: 'a,
F: FnMut(&I::Item) -> K,
K: PartialEq,
D: Deref<Target = ChunkBy<K, I, F>> + Clone,
{
type Item = (K, Group<'a, K, I, F>);
type Item = (K, Group<K, I, F, D>);

#[inline]
fn next(&mut self) -> Option<Self::Item> {
Expand All @@ -404,7 +435,7 @@
(
key,
Group {
parent: self.parent,
parent: self.parent.clone(),
index,
first: Some(elt),
},
Expand All @@ -416,34 +447,32 @@
/// An iterator for the elements in a single group.
///
/// Iterator element type is `I::Item`.
pub struct Group<'a, K, I, F>
pub struct Group<K, I, F, D>
where
I: Iterator + 'a,
I::Item: 'a,
K: 'a,
F: 'a,
I: Iterator,
D: Deref<Target = ChunkBy<K, I, F>>,
{
parent: &'a ChunkBy<K, I, F>,
parent: D,
index: usize,
first: Option<I::Item>,
}

impl<'a, K, I, F> Drop for Group<'a, K, I, F>
impl<K, I, F, D> Drop for Group<K, I, F, D>
where
I: Iterator,
I::Item: 'a,
D: Deref<Target = ChunkBy<K, I, F>>,
{
fn drop(&mut self) {
self.parent.drop_group(self.index);
}
}

impl<'a, K, I, F> Iterator for Group<'a, K, I, F>
impl<K, I, F, D> Iterator for Group<K, I, F, D>
where
I: Iterator,
I::Item: 'a,
F: FnMut(&I::Item) -> K,
K: PartialEq,
D: Deref<Target = ChunkBy<K, I, F>>,
{
type Item = I::Item;
#[inline]
Expand Down Expand Up @@ -526,81 +555,114 @@
}
}

impl<I> IntoIterator for IntoChunks<I>
where
I: Iterator,
{
type Item = Chunk<I, Rc<Self>>;
type IntoIter = Chunks<I, Rc<Self>>;

fn into_iter(self) -> Self::IntoIter {
Chunks {
parent: Rc::new(self),
}
}
}

impl<'a, I> IntoIterator for &'a IntoChunks<I>
where
I: Iterator,
I::Item: 'a,
{
type Item = Chunk<'a, I>;
type IntoIter = Chunks<'a, I>;
type Item = Chunk<I, Self>;
type IntoIter = Chunks<I, Self>;

fn into_iter(self) -> Self::IntoIter {
Chunks { parent: self }
}
}

impl<I> IntoChunks<I>
where
I: Iterator,
{
/// This is pretty much the same as `.into_iter()`, except it uses
/// references in the underlying iterators instead of reference counts,
/// resulting in one less allocation. You may however hit lifetime
/// errors if you require full ownership.
pub fn borrowed_iter(&self) -> <&Self as IntoIterator>::IntoIter {
self.into_iter()
}

Check warning on line 594 in src/groupbylazy.rs

View check run for this annotation

Codecov / codecov/patch

src/groupbylazy.rs#L592-L594

Added lines #L592 - L594 were not covered by tests
}

/// An iterator that yields the Chunk iterators.
///
/// Iterator element type is `Chunk`.
///
/// See [`.chunks()`](crate::Itertools::chunks) for more information.
#[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
#[derive(Clone)]
pub struct Chunks<'a, I>
pub struct Chunks<I, D>
where
I: Iterator + 'a,
I::Item: 'a,
I: Iterator,
D: Deref<Target = IntoChunks<I>>,
{
parent: &'a IntoChunks<I>,
parent: D,
}

impl<'a, I> Iterator for Chunks<'a, I>
impl<I, D> Iterator for Chunks<I, D>
where
I: Iterator,
I::Item: 'a,
D: Deref<Target = IntoChunks<I>> + Clone,
{
type Item = Chunk<'a, I>;
type Item = Chunk<I, D>;

#[inline]
fn next(&mut self) -> Option<Self::Item> {
let index = self.parent.index.get();
self.parent.index.set(index + 1);
let inner = &mut *self.parent.inner.borrow_mut();
inner.step(index).map(|elt| Chunk {
parent: self.parent,
parent: self.parent.clone(),
index,
first: Some(elt),
})
}
}

impl<I, D> Clone for Chunks<I, D>
where
I: Iterator,
D: Deref<Target = IntoChunks<I>> + Clone,
{
clone_fields!(parent);
}

/// An iterator for the elements in a single chunk.
///
/// Iterator element type is `I::Item`.
pub struct Chunk<'a, I>
pub struct Chunk<I, D>
where
I: Iterator + 'a,
I::Item: 'a,
I: Iterator,
D: Deref<Target = IntoChunks<I>>,
{
parent: &'a IntoChunks<I>,
parent: D,
index: usize,
first: Option<I::Item>,
}

impl<'a, I> Drop for Chunk<'a, I>
impl<I, D> Drop for Chunk<I, D>
where
I: Iterator,
I::Item: 'a,
D: Deref<Target = IntoChunks<I>>,
{
fn drop(&mut self) {
self.parent.drop_group(self.index);
}
}

impl<'a, I> Iterator for Chunk<'a, I>
impl<I, D> Iterator for Chunk<I, D>
where
I: Iterator,
I::Item: 'a,
D: Deref<Target = IntoChunks<I>>,
{
type Item = I::Item;
#[inline]
Expand Down
7 changes: 3 additions & 4 deletions tests/quick.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
use itertools::free::{
cloned, enumerate, multipeek, peek_nth, put_back, put_back_n, rciter, zip, zip_eq,
};
use itertools::Itertools;
use itertools::{iproduct, izip, multizip, EitherOrBoth};
use itertools::{iproduct, izip, multizip, EitherOrBoth, Itertools};
use quickcheck as qc;
use std::cmp::{max, min, Ordering};
use std::collections::{HashMap, HashSet};
Expand Down Expand Up @@ -1049,8 +1048,8 @@ quickcheck! {
quickcheck! {
fn fuzz_chunk_by_lazy_duo(data: Vec<u8>, order: Vec<(bool, bool)>) -> bool {
let grouper = data.iter().chunk_by(|k| *k / 3);
let mut chunks1 = grouper.into_iter();
let mut chunks2 = grouper.into_iter();
let mut chunks1 = grouper.borrowed_iter();
let mut chunks2 = grouper.borrowed_iter();
let mut elts = Vec::<&u8>::new();
let mut old_chunks = Vec::new();

Expand Down