From 625c2b9ffb32ef50145f16f9f587db2149a3cb17 Mon Sep 17 00:00:00 2001 From: Takashiidobe Date: Thu, 4 Dec 2025 19:20:57 -0500 Subject: [PATCH 1/6] add double ended iterator tests that make sure using next back after next is correct. Also fix pad_using bug where calling next_back after next would still produce items when it shouldn't --- src/pad_tail.rs | 72 +++++++++++++++++++++++++++++++--------- tests/specializations.rs | 19 +++++++++++ 2 files changed, 75 insertions(+), 16 deletions(-) diff --git a/src/pad_tail.rs b/src/pad_tail.rs index 5595b42ba..587d273b7 100644 --- a/src/pad_tail.rs +++ b/src/pad_tail.rs @@ -1,4 +1,3 @@ -use crate::size_hint; use std::iter::{Fuse, FusedIterator}; /// An iterator adaptor that pads a sequence to a minimum length by filling @@ -13,6 +12,8 @@ pub struct PadUsing { iter: Fuse, min: usize, pos: usize, + back: usize, + total_len: usize, filler: F, } @@ -20,7 +21,7 @@ impl std::fmt::Debug for PadUsing where I: std::fmt::Debug, { - debug_fmt_fields!(PadUsing, iter, min, pos); + debug_fmt_fields!(PadUsing, iter, min, pos, back, total_len); } /// Create a new `PadUsing` iterator. @@ -33,6 +34,8 @@ where iter: iter.fuse(), min, pos: 0, + back: 0, + total_len: min, filler, } } @@ -48,7 +51,7 @@ where fn next(&mut self) -> Option { match self.iter.next() { None => { - if self.pos < self.min { + if self.pos + self.back < self.total_len { let e = Some((self.filler)(self.pos)); self.pos += 1; e @@ -64,8 +67,18 @@ where } fn size_hint(&self) -> (usize, Option) { - let tail = self.min.saturating_sub(self.pos); - size_hint::max(self.iter.size_hint(), (tail, Some(tail))) + let (iter_lower, iter_upper) = self.iter.size_hint(); + let consumed = self.pos.saturating_add(self.back); + + let total_lower = iter_lower.saturating_add(self.pos).max(self.min); + let lower_bound = total_lower.saturating_sub(consumed); + + let upper_bound = iter_upper.map(|iter_upper| { + let total_upper = iter_upper.saturating_add(self.pos).max(self.min); + total_upper.saturating_sub(consumed) + }); + + (lower_bound, upper_bound) } fn fold(self, mut init: B, mut f: G) -> B @@ -87,14 +100,25 @@ where F: FnMut(usize) -> I::Item, { fn next_back(&mut self) -> Option { - if self.min == 0 { - self.iter.next_back() - } else if self.iter.len() >= self.min { - self.min -= 1; - self.iter.next_back() + let current_iter_len = self.iter.len(); + let original_iter_len = current_iter_len.saturating_add(self.pos); + if self.total_len < original_iter_len { + self.total_len = original_iter_len; + } + + if self.pos + self.back >= self.total_len { + return None; + } + + let padding_count = self.total_len.saturating_sub(current_iter_len + self.pos); + + if self.back < padding_count { + let idx = self.total_len - self.back - 1; + self.back += 1; + Some((self.filler)(idx)) } else { - self.min -= 1; - Some((self.filler)(self.min)) + self.back += 1; + self.iter.next_back() } } @@ -102,10 +126,26 @@ where where G: FnMut(B, Self::Item) -> B, { - init = (self.iter.len()..self.min) - .map(self.filler) - .rfold(init, &mut f); - self.iter.rfold(init, f) + let PadUsing { + iter, + min: _, + pos, + back, + mut total_len, + filler, + } = self; + let iter_len = iter.len(); + let original_iter_len = iter_len.saturating_add(pos); + if total_len < original_iter_len { + total_len = original_iter_len; + } + + let start_idx = iter_len + pos; + let end_idx = total_len - back; + + init = (start_idx..end_idx).rev().map(filler).fold(init, &mut f); + + iter.rfold(init, f) } } diff --git a/tests/specializations.rs b/tests/specializations.rs index 26d1f5367..887e1df2d 100644 --- a/tests/specializations.rs +++ b/tests/specializations.rs @@ -131,6 +131,25 @@ where for n in 0..size + 2 { check_specialized!(it, |mut i| i.nth_back(n)); } + + let mut fwd = it.clone(); + let mut bwd = it.clone(); + + for _ in fwd.by_ref() {} + + assert_eq!( + fwd.next_back(), + None, + "iterator leaks elements after consuming forwards" + ); + + while bwd.next_back().is_some() {} + + assert_eq!( + bwd.next(), + None, + "iterator leaks elements after consuming backwards" + ); } quickcheck! { From 979d7a7a4571901362a44c2de87691d31081f64a Mon Sep 17 00:00:00 2001 From: Takashiidobe Date: Fri, 5 Dec 2025 08:47:13 -0500 Subject: [PATCH 2/6] code review feedback: try to remove saturating_ ops from next_back --- src/pad_tail.rs | 89 ++++++++++++++++++++++++++----------------------- 1 file changed, 48 insertions(+), 41 deletions(-) diff --git a/src/pad_tail.rs b/src/pad_tail.rs index 587d273b7..8df13d708 100644 --- a/src/pad_tail.rs +++ b/src/pad_tail.rs @@ -10,10 +10,9 @@ use std::iter::{Fuse, FusedIterator}; #[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub struct PadUsing { iter: Fuse, - min: usize, - pos: usize, - back: usize, - total_len: usize, + elements_from_next: usize, + elements_from_next_back: usize, + elements_required: usize, filler: F, } @@ -21,21 +20,26 @@ impl std::fmt::Debug for PadUsing where I: std::fmt::Debug, { - debug_fmt_fields!(PadUsing, iter, min, pos, back, total_len); + debug_fmt_fields!( + PadUsing, + iter, + elements_from_next, + elements_from_next_back, + elements_required + ); } /// Create a new `PadUsing` iterator. -pub fn pad_using(iter: I, min: usize, filler: F) -> PadUsing +pub fn pad_using(iter: I, elements_required: usize, filler: F) -> PadUsing where I: Iterator, F: FnMut(usize) -> I::Item, { PadUsing { iter: iter.fuse(), - min, - pos: 0, - back: 0, - total_len: min, + elements_from_next: 0, + elements_from_next_back: 0, + elements_required, filler, } } @@ -51,16 +55,16 @@ where fn next(&mut self) -> Option { match self.iter.next() { None => { - if self.pos + self.back < self.total_len { - let e = Some((self.filler)(self.pos)); - self.pos += 1; + if self.elements_from_next + self.elements_from_next_back < self.elements_required { + let e = Some((self.filler)(self.elements_from_next)); + self.elements_from_next += 1; e } else { None } } e => { - self.pos += 1; + self.elements_from_next += 1; e } } @@ -68,13 +72,19 @@ where fn size_hint(&self) -> (usize, Option) { let (iter_lower, iter_upper) = self.iter.size_hint(); - let consumed = self.pos.saturating_add(self.back); + let consumed = self + .elements_from_next + .saturating_add(self.elements_from_next_back); - let total_lower = iter_lower.saturating_add(self.pos).max(self.min); + let total_lower = iter_lower + .saturating_add(self.elements_from_next) + .max(self.elements_required); let lower_bound = total_lower.saturating_sub(consumed); let upper_bound = iter_upper.map(|iter_upper| { - let total_upper = iter_upper.saturating_add(self.pos).max(self.min); + let total_upper = iter_upper + .saturating_add(self.elements_from_next) + .max(self.elements_required); total_upper.saturating_sub(consumed) }); @@ -85,12 +95,12 @@ where where G: FnMut(B, Self::Item) -> B, { - let mut pos = self.pos; + let mut pos = self.elements_from_next; init = self.iter.fold(init, |acc, item| { pos += 1; f(acc, item) }); - (pos..self.min).map(self.filler).fold(init, f) + (pos..self.elements_required).map(self.filler).fold(init, f) } } @@ -100,25 +110,23 @@ where F: FnMut(usize) -> I::Item, { fn next_back(&mut self) -> Option { - let current_iter_len = self.iter.len(); - let original_iter_len = current_iter_len.saturating_add(self.pos); - if self.total_len < original_iter_len { - self.total_len = original_iter_len; - } + let total_consumed = self.elements_from_next + self.elements_from_next_back; - if self.pos + self.back >= self.total_len { + if self.iter.len() == 0 && total_consumed >= self.elements_required { return None; } - let padding_count = self.total_len.saturating_sub(current_iter_len + self.pos); + let elements_remaining = self.elements_required.saturating_sub(total_consumed); + self.elements_from_next_back += 1; - if self.back < padding_count { - let idx = self.total_len - self.back - 1; - self.back += 1; - Some((self.filler)(idx)) + if self.iter.len() < elements_remaining { + Some((self.filler)( + self.elements_required - self.elements_from_next_back, + )) } else { - self.back += 1; - self.iter.next_back() + let e = self.iter.next_back(); + assert!(e.is_some()); + e } } @@ -128,20 +136,19 @@ where { let PadUsing { iter, - min: _, - pos, - back, - mut total_len, + elements_from_next, + elements_from_next_back, + mut elements_required, filler, } = self; let iter_len = iter.len(); - let original_iter_len = iter_len.saturating_add(pos); - if total_len < original_iter_len { - total_len = original_iter_len; + let original_iter_len = iter_len.saturating_add(elements_from_next); + if elements_required < original_iter_len { + elements_required = original_iter_len; } - let start_idx = iter_len + pos; - let end_idx = total_len - back; + let start_idx = iter_len + elements_from_next; + let end_idx = elements_required - elements_from_next_back; init = (start_idx..end_idx).rev().map(filler).fold(init, &mut f); From 8e8b98a20f4a9f52d184232ac05ab7c41583b8d1 Mon Sep 17 00:00:00 2001 From: Takashiidobe Date: Fri, 5 Dec 2025 17:31:11 -0500 Subject: [PATCH 3/6] refactor most implementations of pad_tail to remove saturating ops + clean up rfold --- src/pad_tail.rs | 84 ++++++++++++++++++------------------------------- 1 file changed, 31 insertions(+), 53 deletions(-) diff --git a/src/pad_tail.rs b/src/pad_tail.rs index 8df13d708..a159101ba 100644 --- a/src/pad_tail.rs +++ b/src/pad_tail.rs @@ -53,40 +53,32 @@ where #[inline] fn next(&mut self) -> Option { - match self.iter.next() { - None => { - if self.elements_from_next + self.elements_from_next_back < self.elements_required { - let e = Some((self.filler)(self.elements_from_next)); - self.elements_from_next += 1; - e - } else { - None - } - } - e => { - self.elements_from_next += 1; - e - } + let total_consumed = self.elements_from_next + self.elements_from_next_back; + + if total_consumed >= self.elements_required { + self.iter.next() + } else if let Some(e) = self.iter.next() { + self.elements_from_next += 1; + Some(e) + } else { + let e = (self.filler)(self.elements_from_next); + self.elements_from_next += 1; + Some(e) } } fn size_hint(&self) -> (usize, Option) { - let (iter_lower, iter_upper) = self.iter.size_hint(); - let consumed = self - .elements_from_next - .saturating_add(self.elements_from_next_back); - - let total_lower = iter_lower - .saturating_add(self.elements_from_next) - .max(self.elements_required); - let lower_bound = total_lower.saturating_sub(consumed); - - let upper_bound = iter_upper.map(|iter_upper| { - let total_upper = iter_upper - .saturating_add(self.elements_from_next) - .max(self.elements_required); - total_upper.saturating_sub(consumed) - }); + let total_consumed = self.elements_from_next + self.elements_from_next_back; + + if total_consumed >= self.elements_required { + return self.iter.size_hint(); + } + + let elements_remaining = self.elements_required - total_consumed; + let (low, high) = self.iter.size_hint(); + + let lower_bound = low.max(elements_remaining); + let upper_bound = high.map(|h| h.max(elements_remaining)); (lower_bound, upper_bound) } @@ -112,11 +104,11 @@ where fn next_back(&mut self) -> Option { let total_consumed = self.elements_from_next + self.elements_from_next_back; - if self.iter.len() == 0 && total_consumed >= self.elements_required { - return None; + if total_consumed >= self.elements_required { + return self.iter.next_back(); } - let elements_remaining = self.elements_required.saturating_sub(total_consumed); + let elements_remaining = self.elements_required - total_consumed; self.elements_from_next_back += 1; if self.iter.len() < elements_remaining { @@ -124,9 +116,7 @@ where self.elements_required - self.elements_from_next_back, )) } else { - let e = self.iter.next_back(); - assert!(e.is_some()); - e + self.iter.next_back() } } @@ -134,25 +124,13 @@ where where G: FnMut(B, Self::Item) -> B, { - let PadUsing { - iter, - elements_from_next, - elements_from_next_back, - mut elements_required, - filler, - } = self; - let iter_len = iter.len(); - let original_iter_len = iter_len.saturating_add(elements_from_next); - if elements_required < original_iter_len { - elements_required = original_iter_len; - } - - let start_idx = iter_len + elements_from_next; - let end_idx = elements_required - elements_from_next_back; + let start = self.iter.len() + self.elements_from_next; + let remaining = self.elements_required.max(start); + let end = remaining - self.elements_from_next_back; - init = (start_idx..end_idx).rev().map(filler).fold(init, &mut f); + init = (start..end).rev().map(self.filler).fold(init, &mut f); - iter.rfold(init, f) + self.iter.rfold(init, f) } } From ee3022aa3415cb815c878ac66b711257d196b1cb Mon Sep 17 00:00:00 2001 From: Takashiidobe Date: Sat, 6 Dec 2025 09:11:53 -0500 Subject: [PATCH 4/6] fix pad using's fold to keep track of elements_from_next_back like rfold --- src/pad_tail.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/pad_tail.rs b/src/pad_tail.rs index a159101ba..fed314ca7 100644 --- a/src/pad_tail.rs +++ b/src/pad_tail.rs @@ -87,12 +87,15 @@ where where G: FnMut(B, Self::Item) -> B, { - let mut pos = self.elements_from_next; + let mut start = self.elements_from_next; init = self.iter.fold(init, |acc, item| { - pos += 1; + start += 1; f(acc, item) }); - (pos..self.elements_required).map(self.filler).fold(init, f) + + let end = self.elements_required - self.elements_from_next_back; + + (start..end).map(self.filler).fold(init, f) } } From 5524abc2cf0fcd4885a406fc1441ccf17408f0a9 Mon Sep 17 00:00:00 2001 From: Takashiidobe Date: Sat, 6 Dec 2025 09:27:53 -0500 Subject: [PATCH 5/6] add test for fold as well as rfold to check for double ended iteratorness --- tests/specializations.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/specializations.rs b/tests/specializations.rs index 887e1df2d..c64bc8ffd 100644 --- a/tests/specializations.rs +++ b/tests/specializations.rs @@ -118,6 +118,15 @@ where } } } + check_specialized!(it, |i| { + let mut parameters_from_fold = vec![]; + let fold_result = i.fold(vec![], |mut acc, v: I::Item| { + parameters_from_fold.push((acc.clone(), v.clone())); + acc.push(v); + acc + }); + (parameters_from_fold, fold_result) + }); check_specialized!(it, |i| { let mut parameters_from_rfold = vec![]; let rfold_result = i.rfold(vec![], |mut acc, v: I::Item| { From 537d094edee292bc9c9f96e5d6022cc345738496 Mon Sep 17 00:00:00 2001 From: Takashiidobe Date: Fri, 12 Dec 2025 21:39:00 -0500 Subject: [PATCH 6/6] code review feedback + remove fold + rfold implementations cause we shouldn't need them --- src/pad_tail.rs | 37 +++---------------------------------- tests/specializations.rs | 2 +- 2 files changed, 4 insertions(+), 35 deletions(-) diff --git a/src/pad_tail.rs b/src/pad_tail.rs index fed314ca7..0b66aa83b 100644 --- a/src/pad_tail.rs +++ b/src/pad_tail.rs @@ -51,7 +51,6 @@ where { type Item = I::Item; - #[inline] fn next(&mut self) -> Option { let total_consumed = self.elements_from_next + self.elements_from_next_back; @@ -82,21 +81,6 @@ where (lower_bound, upper_bound) } - - fn fold(self, mut init: B, mut f: G) -> B - where - G: FnMut(B, Self::Item) -> B, - { - let mut start = self.elements_from_next; - init = self.iter.fold(init, |acc, item| { - start += 1; - f(acc, item) - }); - - let end = self.elements_required - self.elements_from_next_back; - - (start..end).map(self.filler).fold(init, f) - } } impl DoubleEndedIterator for PadUsing @@ -111,30 +95,15 @@ where return self.iter.next_back(); } - let elements_remaining = self.elements_required - total_consumed; + let index_from_back = self.elements_required - self.elements_from_next_back - 1; self.elements_from_next_back += 1; - if self.iter.len() < elements_remaining { - Some((self.filler)( - self.elements_required - self.elements_from_next_back, - )) + if index_from_back >= self.iter.len() { + Some((self.filler)(index_from_back)) } else { self.iter.next_back() } } - - fn rfold(self, mut init: B, mut f: G) -> B - where - G: FnMut(B, Self::Item) -> B, - { - let start = self.iter.len() + self.elements_from_next; - let remaining = self.elements_required.max(start); - let end = remaining - self.elements_from_next_back; - - init = (start..end).rev().map(self.filler).fold(init, &mut f); - - self.iter.rfold(init, f) - } } impl ExactSizeIterator for PadUsing diff --git a/tests/specializations.rs b/tests/specializations.rs index c64bc8ffd..9228829b5 100644 --- a/tests/specializations.rs +++ b/tests/specializations.rs @@ -144,7 +144,7 @@ where let mut fwd = it.clone(); let mut bwd = it.clone(); - for _ in fwd.by_ref() {} + while fwd.next().is_some() {} assert_eq!( fwd.next_back(),