itertools icon indicating copy to clipboard operation
itertools copied to clipboard

`Iterate` advances the source iterator one element ahead

Open TonalidadeHidrica opened this issue 3 years ago • 2 comments

Calling Iterate::next fetches "the next after next" element here then returns the actual "next" element. This behavior sometimes triggers unintended panics when overiteration is intended to be guarded by e.g. TakeWhile.

TonalidadeHidrica avatar Jan 23 '22 04:01 TonalidadeHidrica

Hi! That's a great finding. If you have time, it'd be great if you could supply a minimal example exhibiting the problematic behavior.

phimuemue avatar Jan 23 '22 12:01 phimuemue

An easy-to-understand, artificial and boring example is as follows:

use itertools::{Itertools, iterate};
fn main() {
    let v = iterate(35u32, |x| x - 10).take_while(|&x| x > 10).collect_vec();
    println!("{:?}", v);  // expect [35, 25, 15]
}
   Compiling playground v0.0.1 (/playground)
    Finished dev [unoptimized + debuginfo] target(s) in 1.67s
     Running `target/debug/playground`
thread 'main' panicked at 'attempt to subtract with overflow', /rustc/db9d1b20bba1968c1ec1fc49616d4742c1725b4b/library/core/src/ops/arith.rs:215:1
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

We do not expect a panic because the value 5 yielded from iterate(..) should cause the iterator stop due to .take_while(|x| x > 10), but it internally tries to yield the next element, causing an unexpected panic.

TonalidadeHidrica avatar Jan 23 '22 14:01 TonalidadeHidrica

You certainly have a valid argument. However, I don't see how we could avoid that. We don't assume that the type of the initial value is clonable (and doing so would be quite a breaking change) so to be able to give the initial value as the first item of the iterable, we need to first compute the next value before giving the initial value or we would not be able to compute the second item.

All we could do here is improve the documentation to say to be careful about this, or/and suggest an alternative: core::iter::successors (which in the case of "35/25/15" example seems better to me).

Something like "It internally computes the next value before giving the current value so beware if doing so would trigger a panic. You can alternatively use core::iter::successors.".

Philippe-Cholet avatar Jan 10 '24 10:01 Philippe-Cholet