Question about `pad_using`

[Takashiidobe]

Reading the docs for pad_using:

Return an iterator adaptor that pads the sequence to a minimum length of min by filling missing elements using a closure f.

And the examples:

let it = (0..5).pad_using(10, |i| 2*i);
itertools::assert_equal(it, vec![0, 1, 2, 3, 4, 10, 12, 14, 16, 18]);

It seems like if the padding is greater than the iterator's length, it's padded up to a minimum of those items with the given function. That makes sense.

I tried out pad_using, and padded to an iterator of [1]:

use itertools::Itertools;

#[test]
fn pad_tail_bug() {
    let mut iter = vec![].into_iter().pad_using(1, |_| 1);

    itertools::assert_equal(iter.clone(), vec![1]);

    assert_eq!(iter.next(), Some(1));
    assert_eq!(iter.next_back(), None);
}

This is the error message I get running this test:

running 1 test

thread 'pad_tail_bug' panicked at tests/pad_using_bug.rs:10:5:
assertion `left == right` failed
  left: Some(1)
 right: None
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test pad_tail_bug ... FAILED

failures:

failures:
    pad_tail_bug

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

error: test failed, to rerun pass `--test pad_using_bug`

I would assume that calling next and then next_back should return Some(1) for an iterator with only one element and then the next item should be None, no?

[Takashiidobe]

If you only use next_back or next exclusively, it works fine:

use itertools::Itertools;

#[test]
fn pad_using_is_fine() {
    let mut iter = vec![].into_iter().pad_using(1, |_| 1);

    assert_eq!(iter.next_back(), Some(1));
    assert_eq!(iter.next_back(), None);
}

use itertools::Itertools;

#[test]
fn pad_using_is_fine() {
    let mut iter = vec![].into_iter().pad_using(1, |_| 1);

    assert_eq!(iter.next(), Some(1));
    assert_eq!(iter.next(), None);
}

Both of these run fine for me:

running 1 test
test pad_using_is_fine ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

[phimuemue]

Good catch, I think you're right.

From skimming, I think next_back is missing a check for pos.

I wonder if our representation of PadUsing is as simple as possible, and why this problem was not caught by tests.

[Takashiidobe]

Yup, it's not counting against the total length of the iterator.

For the testing bit, I do see testing for the double endedness of the iterator, and this pulls from the iterator in alternating nexts and next_backs.

The test itself pads the iterator to 10 items.

It seems like this is testing that nth_back and rfold are correct, but it doesn't check whether or not you get the right behavior consuming with next or next_back.

I added a bit of code at the end of test_double_ended_specializations and re-ran the specializations file:

diff --git a/tests/specializations.rs b/tests/specializations.rs
index 26d1f53..887e1df 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() {} // clippy suggested this, but its while fwd.next().is_some()
+
+    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! {

Which catches just this bug:

failures:

---- pad_using stdout ----

thread 'pad_using' panicked at tests/specializations.rs:140:5:
assertion `left == right` failed: iterator leaks elements after consuming forwards
  left: Some(45)
 right: None
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

thread 'pad_using' panicked at tests/specializations.rs:140:5:
assertion `left == right` failed: iterator leaks elements after consuming forwards
  left: Some(45)
 right: None

thread 'pad_using' panicked at /home/takashi/.local/share/mise/installs/rust/1.90.0/registry/src/localhost-52466c1a82b6d709/quickcheck-0.9.2/src/tester.rs:178:28:
[quickcheck] TEST FAILED (runtime error). Arguments: ([])
Error: "assertion `left == right` failed: iterator leaks elements after consuming forwards\n  left: Some(45)\n right: None"

failures:
    pad_using

I'm not sure if this is a specialization, so maybe it doesn't fit here, but i think there should be a test that verifies the properties of double ended iteration work to catch bugs like this, and that would be a good feature to prevent new bugs from cropping up. Maybe there's other bugs lurking?