From d831b6e8104df55ba53081b2541d541241e1904d Mon Sep 17 00:00:00 2001 From: Philippe-Cholet Date: Wed, 3 Jan 2024 18:18:39 +0100 Subject: [PATCH 01/16] `MultiProduct`: fix documentation --- src/adaptors/multi_product.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/adaptors/multi_product.rs b/src/adaptors/multi_product.rs index 74cb06d0e..eee246119 100644 --- a/src/adaptors/multi_product.rs +++ b/src/adaptors/multi_product.rs @@ -9,7 +9,7 @@ use alloc::vec::Vec; /// An iterator adaptor that iterates over the cartesian product of /// multiple iterators of type `I`. /// -/// An iterator element type is `Vec`. +/// An iterator element type is `Vec`. /// /// See [`.multi_cartesian_product()`](crate::Itertools::multi_cartesian_product) /// for more information. From dbd47a2910a5b2ff1ef3a6a9948c288f4975e3ad Mon Sep 17 00:00:00 2001 From: Philippe-Cholet Date: Wed, 3 Jan 2024 18:24:51 +0100 Subject: [PATCH 02/16] `MultiProduct`: fix debug name --- src/adaptors/multi_product.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/adaptors/multi_product.rs b/src/adaptors/multi_product.rs index eee246119..05c193f51 100644 --- a/src/adaptors/multi_product.rs +++ b/src/adaptors/multi_product.rs @@ -24,7 +24,7 @@ where I: Iterator + Clone + std::fmt::Debug, I::Item: Clone + std::fmt::Debug, { - debug_fmt_fields!(CoalesceBy, 0); + debug_fmt_fields!(MultiProduct, 0); } /// Create a new cartesian product iterator over an arbitrary number From 32fe7f588a49caf6a14a2eee3f512d8ec9df09ff Mon Sep 17 00:00:00 2001 From: Philippe-Cholet Date: Wed, 3 Jan 2024 12:17:49 +0100 Subject: [PATCH 03/16] `MultiProduct`: start over --- src/adaptors/multi_product.rs | 174 +--------------------------------- 1 file changed, 1 insertion(+), 173 deletions(-) diff --git a/src/adaptors/multi_product.rs b/src/adaptors/multi_product.rs index 05c193f51..f9091dd0b 100644 --- a/src/adaptors/multi_product.rs +++ b/src/adaptors/multi_product.rs @@ -1,8 +1,5 @@ #![cfg(feature = "use_alloc")] -use crate::size_hint; -use crate::Itertools; - use alloc::vec::Vec; #[derive(Clone)] @@ -52,87 +49,10 @@ where I: Iterator + Clone, I::Item: Clone, { - cur: Option, iter: I, iter_orig: I, } -/// Holds the current state during an iteration of a `MultiProduct`. -#[derive(Debug)] -enum MultiProductIterState { - StartOfIter, - MidIter { on_first_iter: bool }, -} - -impl MultiProduct -where - I: Iterator + Clone, - I::Item: Clone, -{ - /// Iterates the rightmost iterator, then recursively iterates iterators - /// to the left if necessary. - /// - /// Returns true if the iteration succeeded, else false. - fn iterate_last( - multi_iters: &mut [MultiProductIter], - mut state: MultiProductIterState, - ) -> bool { - use self::MultiProductIterState::*; - - if let Some((last, rest)) = multi_iters.split_last_mut() { - let on_first_iter = match state { - StartOfIter => { - let on_first_iter = !last.in_progress(); - state = MidIter { on_first_iter }; - on_first_iter - } - MidIter { on_first_iter } => on_first_iter, - }; - - if !on_first_iter { - last.iterate(); - } - - if last.in_progress() { - true - } else if Self::iterate_last(rest, state) { - last.reset(); - last.iterate(); - // If iterator is None twice consecutively, then iterator is - // empty; whole product is empty. - last.in_progress() - } else { - false - } - } else { - // Reached end of iterator list. On initialisation, return true. - // At end of iteration (final iterator finishes), finish. - match state { - StartOfIter => false, - MidIter { on_first_iter } => on_first_iter, - } - } - } - - /// Returns the unwrapped value of the next iteration. - fn curr_iterator(&self) -> Vec { - self.0 - .iter() - .map(|multi_iter| multi_iter.cur.clone().unwrap()) - .collect() - } - - /// Returns true if iteration has started and has not yet finished; false - /// otherwise. - fn in_progress(&self) -> bool { - if let Some(last) = self.0.last() { - last.in_progress() - } else { - false - } - } -} - impl MultiProductIter where I: Iterator + Clone, @@ -140,27 +60,10 @@ where { fn new(iter: I) -> Self { Self { - cur: None, iter: iter.clone(), iter_orig: iter, } } - - /// Iterate the managed iterator. - fn iterate(&mut self) { - self.cur = self.iter.next(); - } - - /// Reset the managed iterator. - fn reset(&mut self) { - self.iter = self.iter_orig.clone(); - } - - /// Returns true if the current iterator has been started and has not yet - /// finished; false otherwise. - fn in_progress(&self) -> bool { - self.cur.is_some() - } } impl Iterator for MultiProduct @@ -171,81 +74,6 @@ where type Item = Vec; fn next(&mut self) -> Option { - if Self::iterate_last(&mut self.0, MultiProductIterState::StartOfIter) { - Some(self.curr_iterator()) - } else { - None - } - } - - fn count(self) -> usize { - if self.0.is_empty() { - return 0; - } - - if !self.in_progress() { - return self - .0 - .into_iter() - .fold(1, |acc, multi_iter| acc * multi_iter.iter.count()); - } - - self.0.into_iter().fold( - 0, - |acc, - MultiProductIter { - iter, - iter_orig, - cur: _, - }| { - let total_count = iter_orig.count(); - let cur_count = iter.count(); - acc * total_count + cur_count - }, - ) - } - - fn size_hint(&self) -> (usize, Option) { - // Not ExactSizeIterator because size may be larger than usize - if self.0.is_empty() { - return (0, Some(0)); - } - - if !self.in_progress() { - return self.0.iter().fold((1, Some(1)), |acc, multi_iter| { - size_hint::mul(acc, multi_iter.iter.size_hint()) - }); - } - - self.0.iter().fold( - (0, Some(0)), - |acc, - MultiProductIter { - iter, - iter_orig, - cur: _, - }| { - let cur_size = iter.size_hint(); - let total_size = iter_orig.size_hint(); - size_hint::add(size_hint::mul(acc, total_size), cur_size) - }, - ) - } - - fn last(self) -> Option { - let iter_count = self.0.len(); - - let lasts: Self::Item = self - .0 - .into_iter() - .map(|multi_iter| multi_iter.iter.last()) - .while_some() - .collect(); - - if lasts.len() == iter_count { - Some(lasts) - } else { - None - } + todo!() } } From 45e1245f7800aa862ee345220dfd96ca0872707c Mon Sep 17 00:00:00 2001 From: Philippe-Cholet Date: Wed, 3 Jan 2024 12:24:08 +0100 Subject: [PATCH 04/16] `MultiProduct`: add `MultiProductInner` By wrapping "inner" in an option, I'll be able to fuse the iterator. Keep the current value of each iterator in `cur`: none at the beginning, some later. Previously, each `MultiProductIter` remembered its own element. Now, we have a vector of them. That way, we can update `cur` in-place and clone it to generate the item, I think that's simpler (less unwrap) and maybe more efficient. But we have two different vectors instead of a single bigger vector. --- src/adaptors/multi_product.rs | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/src/adaptors/multi_product.rs b/src/adaptors/multi_product.rs index f9091dd0b..bdedd5748 100644 --- a/src/adaptors/multi_product.rs +++ b/src/adaptors/multi_product.rs @@ -11,11 +11,21 @@ use alloc::vec::Vec; /// See [`.multi_cartesian_product()`](crate::Itertools::multi_cartesian_product) /// for more information. #[must_use = "iterator adaptors are lazy and do nothing unless consumed"] -pub struct MultiProduct(Vec>) +pub struct MultiProduct(Option>) where I: Iterator + Clone, I::Item: Clone; +#[derive(Clone)] +struct MultiProductInner +where + I: Iterator + Clone, + I::Item: Clone, +{ + iters: Vec>, + cur: Option>, +} + impl std::fmt::Debug for MultiProduct where I: Iterator + Clone + std::fmt::Debug, @@ -24,6 +34,14 @@ where debug_fmt_fields!(MultiProduct, 0); } +impl std::fmt::Debug for MultiProductInner +where + I: Iterator + Clone + std::fmt::Debug, + I::Item: Clone + std::fmt::Debug, +{ + debug_fmt_fields!(MultiProductInner, iters, cur); +} + /// Create a new cartesian product iterator over an arbitrary number /// of iterators of the same type. /// @@ -35,11 +53,13 @@ where ::IntoIter: Clone, ::Item: Clone, { - MultiProduct( - iters + let inner = MultiProductInner { + iters: iters .map(|i| MultiProductIter::new(i.into_iter())) .collect(), - ) + cur: None, + }; + MultiProduct(Some(inner)) } #[derive(Clone, Debug)] @@ -74,6 +94,7 @@ where type Item = Vec; fn next(&mut self) -> Option { + let inner = self.0.as_mut()?; todo!() } } From 113f4bb3d102622d0fd1974f4ad807827d339983 Mon Sep 17 00:00:00 2001 From: Philippe-Cholet Date: Wed, 3 Jan 2024 14:36:44 +0100 Subject: [PATCH 05/16] `MultiProduct::next` Co-Authored-By: Jakob Degen --- src/adaptors/multi_product.rs | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/adaptors/multi_product.rs b/src/adaptors/multi_product.rs index bdedd5748..9e20af0c8 100644 --- a/src/adaptors/multi_product.rs +++ b/src/adaptors/multi_product.rs @@ -95,6 +95,32 @@ where fn next(&mut self) -> Option { let inner = self.0.as_mut()?; - todo!() + match &mut inner.cur { + Some(values) => { + for (iter, item) in inner.iters.iter_mut().zip(values.iter_mut()).rev() { + if let Some(new) = iter.iter.next() { + *item = new; + return Some(values.clone()); + } else { + iter.iter = iter.iter_orig.clone(); + // `cur` is not none so the untouched `iter_orig` can not be empty. + *item = iter.iter.next().unwrap(); + } + } + // The iterator ends. + self.0 = None; + None + } + // Only the first time. + None => { + let next: Option> = inner.iters.iter_mut().map(|i| i.iter.next()).collect(); + if next.is_some() { + inner.cur = next.clone(); + } else { + self.0 = None; + } + next + } + } } } From e34d52b39e4c7fe959f7f3f55e8450a9aab7b8aa Mon Sep 17 00:00:00 2001 From: Philippe-Cholet Date: Wed, 3 Jan 2024 15:17:01 +0100 Subject: [PATCH 06/16] `MultiProduct`: test the bugfix An empty product is supposed to generate a single empty vector, test this. Co-Authored-By: Jakob Degen --- tests/quick.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/quick.rs b/tests/quick.rs index dffcd22f6..954655a43 100644 --- a/tests/quick.rs +++ b/tests/quick.rs @@ -450,6 +450,12 @@ quickcheck! { assert_eq!(answer.into_iter().last(), a.multi_cartesian_product().last()); } + fn correct_empty_multi_product() -> () { + let empty = Vec::>::new().into_iter().multi_cartesian_product(); + assert!(correct_size_hint(empty.clone())); + itertools::assert_equal(empty, std::iter::once(Vec::new())) + } + #[allow(deprecated)] fn size_step(a: Iter, s: usize) -> bool { let mut s = s; From 2f288efb847c191c2ffccc462e53fc71a63a6032 Mon Sep 17 00:00:00 2001 From: Philippe-Cholet Date: Wed, 3 Jan 2024 15:20:19 +0100 Subject: [PATCH 07/16] `MultiProduct` is now fused --- src/adaptors/multi_product.rs | 7 +++++++ src/lib.rs | 3 ++- tests/specializations.rs | 1 - 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/adaptors/multi_product.rs b/src/adaptors/multi_product.rs index 9e20af0c8..5bf7dc334 100644 --- a/src/adaptors/multi_product.rs +++ b/src/adaptors/multi_product.rs @@ -124,3 +124,10 @@ where } } } + +impl std::iter::FusedIterator for MultiProduct +where + I: Iterator + Clone, + I::Item: Clone, +{ +} diff --git a/src/lib.rs b/src/lib.rs index a9977e1f1..126eb2221 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1162,10 +1162,11 @@ pub trait Itertools: Iterator { /// the product of iterators yielding multiple types, use the /// [`iproduct`] macro instead. /// - /// /// The iterator element type is `Vec`, where `T` is the iterator element /// of the subiterators. /// + /// Note that the iterator is fused. + /// /// ``` /// use itertools::Itertools; /// let mut multi_prod = (0..3).map(|i| (i * 2)..(i * 2 + 2)) diff --git a/tests/specializations.rs b/tests/specializations.rs index e14b1b669..fd8801e4e 100644 --- a/tests/specializations.rs +++ b/tests/specializations.rs @@ -163,7 +163,6 @@ quickcheck! { TestResult::passed() } - #[ignore] // It currently fails because `MultiProduct` is not fused. fn multi_cartesian_product(a: Vec, b: Vec, c: Vec) -> TestResult { if a.len() * b.len() * c.len() > 100 { return TestResult::discard(); From 131cbbdad4f199c25bfedb2b5be6047e14f553d8 Mon Sep 17 00:00:00 2001 From: Philippe-Cholet Date: Wed, 3 Jan 2024 16:46:37 +0100 Subject: [PATCH 08/16] `MultiProduct::last` --- src/adaptors/multi_product.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/adaptors/multi_product.rs b/src/adaptors/multi_product.rs index 5bf7dc334..3b146e812 100644 --- a/src/adaptors/multi_product.rs +++ b/src/adaptors/multi_product.rs @@ -123,6 +123,31 @@ where } } } + + fn last(self) -> Option { + let MultiProductInner { iters, cur } = self.0?; + if let Some(values) = cur { + let mut count = iters.len(); + let last = iters + .into_iter() + .zip(values) + .map(|(i, value)| { + i.iter.last().unwrap_or_else(|| { + count -= 1; + value + }) + }) + .collect(); + if count == 0 { + // `values` was the last item. + None + } else { + Some(last) + } + } else { + iters.into_iter().map(|i| i.iter.last()).collect() + } + } } impl std::iter::FusedIterator for MultiProduct From 9a03092bd898f2a70dc862250d4d2e4fa5d647f5 Mon Sep 17 00:00:00 2001 From: Philippe-Cholet Date: Wed, 3 Jan 2024 16:03:19 +0100 Subject: [PATCH 09/16] `MultiProduct::count` `count` is generally not a cheap operation so I avoid it when the result would be zero anyway. --- src/adaptors/multi_product.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/adaptors/multi_product.rs b/src/adaptors/multi_product.rs index 3b146e812..f95a60039 100644 --- a/src/adaptors/multi_product.rs +++ b/src/adaptors/multi_product.rs @@ -124,6 +124,34 @@ where } } + fn count(self) -> usize { + match self.0 { + None => 0, + Some(MultiProductInner { iters, cur }) => { + if cur.is_none() { + iters + .into_iter() + .map(|iter| iter.iter_orig.count()) + .try_fold(1, |product, count| { + if count == 0 { + None + } else { + Some(product * count) + } + }) + .unwrap_or_default() + } else { + iters.into_iter().fold(0, |mut acc, iter| { + if acc != 0 { + acc *= iter.iter_orig.count(); + } + acc + iter.iter.count() + }) + } + } + } + } + fn last(self) -> Option { let MultiProductInner { iters, cur } = self.0?; if let Some(values) = cur { From da5e54c5dcfcd2e9912e7c58895eb81a4083a084 Mon Sep 17 00:00:00 2001 From: Philippe-Cholet Date: Wed, 3 Jan 2024 16:11:20 +0100 Subject: [PATCH 10/16] `MultiProduct::size_hint` Similar to `count` but with "size hint" operations. --- src/adaptors/multi_product.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/adaptors/multi_product.rs b/src/adaptors/multi_product.rs index f95a60039..9c8ed1988 100644 --- a/src/adaptors/multi_product.rs +++ b/src/adaptors/multi_product.rs @@ -2,6 +2,8 @@ use alloc::vec::Vec; +use crate::size_hint; + #[derive(Clone)] /// An iterator adaptor that iterates over the cartesian product of /// multiple iterators of type `I`. @@ -152,6 +154,27 @@ where } } + fn size_hint(&self) -> (usize, Option) { + match &self.0 { + None => (0, Some(0)), + Some(MultiProductInner { iters, cur }) => { + if cur.is_none() { + iters + .iter() + .map(|iter| iter.iter_orig.size_hint()) + .fold((1, Some(1)), size_hint::mul) + } else if let [first, tail @ ..] = &iters[..] { + tail.iter().fold(first.iter.size_hint(), |mut sh, iter| { + sh = size_hint::mul(sh, iter.iter_orig.size_hint()); + size_hint::add(sh, iter.iter.size_hint()) + }) + } else { + (0, Some(0)) + } + } + } + } + fn last(self) -> Option { let MultiProductInner { iters, cur } = self.0?; if let Some(values) = cur { From b9f22c5381bf3b01c42e98dd6a788defa959fb60 Mon Sep 17 00:00:00 2001 From: Philippe-Cholet Date: Sun, 7 Jan 2024 16:22:05 +0100 Subject: [PATCH 11/16] `MultiProduct`: add some documentation --- src/adaptors/multi_product.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/adaptors/multi_product.rs b/src/adaptors/multi_product.rs index 9c8ed1988..f0a2e3c8a 100644 --- a/src/adaptors/multi_product.rs +++ b/src/adaptors/multi_product.rs @@ -13,18 +13,24 @@ use crate::size_hint; /// See [`.multi_cartesian_product()`](crate::Itertools::multi_cartesian_product) /// for more information. #[must_use = "iterator adaptors are lazy and do nothing unless consumed"] -pub struct MultiProduct(Option>) +pub struct MultiProduct( + /// `None` once the iterator has ended. + Option>, +) where I: Iterator + Clone, I::Item: Clone; #[derive(Clone)] +/// Internals for `MultiProduct`. struct MultiProductInner where I: Iterator + Clone, I::Item: Clone, { + /// Holds the iterators. iters: Vec>, + /// It is `None` at the beginning then it holds the current item of each iterator. cur: Option>, } From 059221c1d2a0cc700296160c218e480b49d9c8d5 Mon Sep 17 00:00:00 2001 From: Philippe-Cholet Date: Mon, 8 Jan 2024 19:31:50 +0100 Subject: [PATCH 12/16] `MultiProduct`: ends after the nullary product While adding comments, I realized I could set the iterator as finished in the case of the nullary cartesian product. It adds a simple invariant. I thought of making a comment but a debug check is better. --- src/adaptors/multi_product.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/adaptors/multi_product.rs b/src/adaptors/multi_product.rs index f0a2e3c8a..53ca8f64b 100644 --- a/src/adaptors/multi_product.rs +++ b/src/adaptors/multi_product.rs @@ -105,6 +105,7 @@ where let inner = self.0.as_mut()?; match &mut inner.cur { Some(values) => { + debug_assert!(!inner.iters.is_empty()); for (iter, item) in inner.iters.iter_mut().zip(values.iter_mut()).rev() { if let Some(new) = iter.iter.next() { *item = new; @@ -122,10 +123,11 @@ where // Only the first time. None => { let next: Option> = inner.iters.iter_mut().map(|i| i.iter.next()).collect(); - if next.is_some() { - inner.cur = next.clone(); - } else { + if next.is_none() || inner.iters.is_empty() { + // This cartesian product had at most one item to generate and now ends. self.0 = None; + } else { + inner.cur = next.clone(); } next } @@ -175,7 +177,8 @@ where size_hint::add(sh, iter.iter.size_hint()) }) } else { - (0, Some(0)) + // Since `cur` is some, this cartesian product has started so `iters` is not empty. + unreachable!() } } } From 64a5bdb37c3700df38e5e4ac1e5653b2cc2ccaf2 Mon Sep 17 00:00:00 2001 From: Philippe-Cholet Date: Mon, 8 Jan 2024 19:57:16 +0100 Subject: [PATCH 13/16] `MultiProduct`: add comments --- src/adaptors/multi_product.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/adaptors/multi_product.rs b/src/adaptors/multi_product.rs index 53ca8f64b..55fd0dbf3 100644 --- a/src/adaptors/multi_product.rs +++ b/src/adaptors/multi_product.rs @@ -102,10 +102,13 @@ where type Item = Vec; fn next(&mut self) -> Option { + // This fuses the iterator. let inner = self.0.as_mut()?; match &mut inner.cur { Some(values) => { debug_assert!(!inner.iters.is_empty()); + // Find (from the right) a non-finished iterator and + // reset the finished ones encountered. for (iter, item) in inner.iters.iter_mut().zip(values.iter_mut()).rev() { if let Some(new) = iter.iter.next() { *item = new; @@ -136,9 +139,12 @@ where fn count(self) -> usize { match self.0 { - None => 0, + None => 0, // The cartesian product has ended. Some(MultiProductInner { iters, cur }) => { if cur.is_none() { + // The iterator is fresh so the count is the product of the length of each iterator: + // - If one of them is empty, stop counting. + // - Less `count()` calls than the general case. iters .into_iter() .map(|iter| iter.iter_orig.count()) @@ -151,6 +157,7 @@ where }) .unwrap_or_default() } else { + // The general case. iters.into_iter().fold(0, |mut acc, iter| { if acc != 0 { acc *= iter.iter_orig.count(); @@ -164,7 +171,7 @@ where fn size_hint(&self) -> (usize, Option) { match &self.0 { - None => (0, Some(0)), + None => (0, Some(0)), // The cartesian product has ended. Some(MultiProductInner { iters, cur }) => { if cur.is_none() { iters @@ -186,6 +193,7 @@ where fn last(self) -> Option { let MultiProductInner { iters, cur } = self.0?; + // Collect the last item of each iterator of the product. if let Some(values) = cur { let mut count = iters.len(); let last = iters @@ -193,6 +201,7 @@ where .zip(values) .map(|(i, value)| { i.iter.last().unwrap_or_else(|| { + // The iterator is empty, use its current `value`. count -= 1; value }) From cb2127249318a8a0a69b384212109b3a7167327b Mon Sep 17 00:00:00 2001 From: Philippe-Cholet Date: Thu, 18 Jan 2024 17:29:56 +0100 Subject: [PATCH 14/16] `MultiProduct` state: comment with code --- src/adaptors/multi_product.rs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/adaptors/multi_product.rs b/src/adaptors/multi_product.rs index 55fd0dbf3..f18eaafb7 100644 --- a/src/adaptors/multi_product.rs +++ b/src/adaptors/multi_product.rs @@ -1,4 +1,5 @@ #![cfg(feature = "use_alloc")] +use Option::{self as State, None as ProductEnded, Some as ProductInProgress}; use alloc::vec::Vec; @@ -13,10 +14,7 @@ use crate::size_hint; /// See [`.multi_cartesian_product()`](crate::Itertools::multi_cartesian_product) /// for more information. #[must_use = "iterator adaptors are lazy and do nothing unless consumed"] -pub struct MultiProduct( - /// `None` once the iterator has ended. - Option>, -) +pub struct MultiProduct(State>) where I: Iterator + Clone, I::Item: Clone; @@ -67,7 +65,7 @@ where .collect(), cur: None, }; - MultiProduct(Some(inner)) + MultiProduct(ProductInProgress(inner)) } #[derive(Clone, Debug)] @@ -119,8 +117,7 @@ where *item = iter.iter.next().unwrap(); } } - // The iterator ends. - self.0 = None; + self.0 = ProductEnded; None } // Only the first time. @@ -128,7 +125,7 @@ where let next: Option> = inner.iters.iter_mut().map(|i| i.iter.next()).collect(); if next.is_none() || inner.iters.is_empty() { // This cartesian product had at most one item to generate and now ends. - self.0 = None; + self.0 = ProductEnded; } else { inner.cur = next.clone(); } @@ -139,8 +136,8 @@ where fn count(self) -> usize { match self.0 { - None => 0, // The cartesian product has ended. - Some(MultiProductInner { iters, cur }) => { + ProductEnded => 0, + ProductInProgress(MultiProductInner { iters, cur }) => { if cur.is_none() { // The iterator is fresh so the count is the product of the length of each iterator: // - If one of them is empty, stop counting. @@ -171,8 +168,8 @@ where fn size_hint(&self) -> (usize, Option) { match &self.0 { - None => (0, Some(0)), // The cartesian product has ended. - Some(MultiProductInner { iters, cur }) => { + ProductEnded => (0, Some(0)), + ProductInProgress(MultiProductInner { iters, cur }) => { if cur.is_none() { iters .iter() From 76984fd7840ca30e895cdd07090e35fa6f6b57d2 Mon Sep 17 00:00:00 2001 From: Philippe-Cholet Date: Thu, 18 Jan 2024 17:41:54 +0100 Subject: [PATCH 15/16] `MultiProduct` values: comment with code (1) --- src/adaptors/multi_product.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/adaptors/multi_product.rs b/src/adaptors/multi_product.rs index f18eaafb7..724fd4fd0 100644 --- a/src/adaptors/multi_product.rs +++ b/src/adaptors/multi_product.rs @@ -1,5 +1,6 @@ #![cfg(feature = "use_alloc")] use Option::{self as State, None as ProductEnded, Some as ProductInProgress}; +use Option::{self as CurrentItems, None as NotYetPopulated, Some as Populated}; use alloc::vec::Vec; @@ -28,8 +29,8 @@ where { /// Holds the iterators. iters: Vec>, - /// It is `None` at the beginning then it holds the current item of each iterator. - cur: Option>, + /// Not populated at the beginning then it holds the current item of each iterator. + cur: CurrentItems>, } impl std::fmt::Debug for MultiProduct @@ -63,7 +64,7 @@ where iters: iters .map(|i| MultiProductIter::new(i.into_iter())) .collect(), - cur: None, + cur: NotYetPopulated, }; MultiProduct(ProductInProgress(inner)) } @@ -103,7 +104,7 @@ where // This fuses the iterator. let inner = self.0.as_mut()?; match &mut inner.cur { - Some(values) => { + Populated(values) => { debug_assert!(!inner.iters.is_empty()); // Find (from the right) a non-finished iterator and // reset the finished ones encountered. @@ -113,7 +114,7 @@ where return Some(values.clone()); } else { iter.iter = iter.iter_orig.clone(); - // `cur` is not none so the untouched `iter_orig` can not be empty. + // `cur` is populated so the untouched `iter_orig` can not be empty. *item = iter.iter.next().unwrap(); } } @@ -121,7 +122,7 @@ where None } // Only the first time. - None => { + NotYetPopulated => { let next: Option> = inner.iters.iter_mut().map(|i| i.iter.next()).collect(); if next.is_none() || inner.iters.is_empty() { // This cartesian product had at most one item to generate and now ends. @@ -181,7 +182,7 @@ where size_hint::add(sh, iter.iter.size_hint()) }) } else { - // Since `cur` is some, this cartesian product has started so `iters` is not empty. + // Since it is populated, this cartesian product has started so `iters` is not empty. unreachable!() } } @@ -191,7 +192,7 @@ where fn last(self) -> Option { let MultiProductInner { iters, cur } = self.0?; // Collect the last item of each iterator of the product. - if let Some(values) = cur { + if let Populated(values) = cur { let mut count = iters.len(); let last = iters .into_iter() From 085990c86a8d43e4b3bdd792ecc243f0d0240e48 Mon Sep 17 00:00:00 2001 From: Philippe-Cholet Date: Thu, 18 Jan 2024 17:48:10 +0100 Subject: [PATCH 16/16] `MultiProduct` values: comment with code (2) Separate `Populated/NotYetPopulated` in `match` blocks. Indents change quite a bit. --- src/adaptors/multi_product.rs | 70 +++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 32 deletions(-) diff --git a/src/adaptors/multi_product.rs b/src/adaptors/multi_product.rs index 724fd4fd0..d551c0afc 100644 --- a/src/adaptors/multi_product.rs +++ b/src/adaptors/multi_product.rs @@ -138,45 +138,51 @@ where fn count(self) -> usize { match self.0 { ProductEnded => 0, - ProductInProgress(MultiProductInner { iters, cur }) => { - if cur.is_none() { - // The iterator is fresh so the count is the product of the length of each iterator: - // - If one of them is empty, stop counting. - // - Less `count()` calls than the general case. - iters - .into_iter() - .map(|iter| iter.iter_orig.count()) - .try_fold(1, |product, count| { - if count == 0 { - None - } else { - Some(product * count) - } - }) - .unwrap_or_default() - } else { - // The general case. - iters.into_iter().fold(0, |mut acc, iter| { - if acc != 0 { - acc *= iter.iter_orig.count(); - } - acc + iter.iter.count() - }) + // The iterator is fresh so the count is the product of the length of each iterator: + // - If one of them is empty, stop counting. + // - Less `count()` calls than the general case. + ProductInProgress(MultiProductInner { + iters, + cur: NotYetPopulated, + }) => iters + .into_iter() + .map(|iter| iter.iter_orig.count()) + .try_fold(1, |product, count| { + if count == 0 { + None + } else { + Some(product * count) + } + }) + .unwrap_or_default(), + // The general case. + ProductInProgress(MultiProductInner { + iters, + cur: Populated(_), + }) => iters.into_iter().fold(0, |mut acc, iter| { + if acc != 0 { + acc *= iter.iter_orig.count(); } - } + acc + iter.iter.count() + }), } } fn size_hint(&self) -> (usize, Option) { match &self.0 { ProductEnded => (0, Some(0)), - ProductInProgress(MultiProductInner { iters, cur }) => { - if cur.is_none() { - iters - .iter() - .map(|iter| iter.iter_orig.size_hint()) - .fold((1, Some(1)), size_hint::mul) - } else if let [first, tail @ ..] = &iters[..] { + ProductInProgress(MultiProductInner { + iters, + cur: NotYetPopulated, + }) => iters + .iter() + .map(|iter| iter.iter_orig.size_hint()) + .fold((1, Some(1)), size_hint::mul), + ProductInProgress(MultiProductInner { + iters, + cur: Populated(_), + }) => { + if let [first, tail @ ..] = &iters[..] { tail.iter().fold(first.iter.size_hint(), |mut sh, iter| { sh = size_hint::mul(sh, iter.iter_orig.size_hint()); size_hint::add(sh, iter.iter.size_hint())