Repository: rust-itertools/itertools (language: rust)

Issue:
`Iterate` advances the source iterator one element ahead
Calling `Iterate::next` fetches "the next after next" element [here](https://github.com/rust-itertools/itertools/blob/master/src/sources.rs#L159) then returns the actual "next" element.  This behavior sometimes triggers unintended panics when overiteration is intended to be guarded by e.g. `TakeWhile`.


Discussion / hints:
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.
An easy-to-understand, artificial and boring example is as follows:
```rust
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.
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`._".

Fix patch:
diff --git a/src/sources.rs b/src/sources.rs
index bd520c21d..a7f51e52f 100644
--- a/src/sources.rs
+++ b/src/sources.rs
@@ -177,8 +177,20 @@ where
 /// ```
 /// use itertools::iterate;
 ///
-/// itertools::assert_equal(iterate(1, |&i| i * 3).take(5), vec![1, 3, 9, 27, 81]);
+/// itertools::assert_equal(iterate(1, |i| i % 3 + 1).take(5), vec![1, 2, 3, 1, 2]);
 /// ```
+///
+/// **Panics** if compute the next value does.
+///
+/// ```should_panic
+/// # use itertools::iterate;
+/// let mut it = iterate(25u32, |x| x - 10).take_while(|&x| x > 10);
+/// assert_eq!(it.next(), Some(25)); // `Iterate` holds 15.
+/// assert_eq!(it.next(), Some(15)); // `Iterate` holds 5.
+/// it.next(); // `5 - 10` overflows.
+/// ```
+///
+/// You can alternatively use [`core::iter::successors`] as it better describes a finite iterator.
 pub fn iterate<St, F>(initial_value: St, f: F) -> Iterate<St, F>
 where
     F: FnMut(&St) -> St,


Test patch: