rust-itertools / itertools

Extra iterator adaptors, iterator methods, free functions, and macros.
https://docs.rs/itertools/
Apache License 2.0
2.69k stars 303 forks source link

`ConsTuples::next_back` is plain wrong #852

Closed Philippe-Cholet closed 7 months ago

Philippe-Cholet commented 7 months ago

I was re-considering specialize ConsTuples::rfold for #755 when I saw something obviously wrong: https://github.com/rust-itertools/itertools/blob/f60fe7e9548ffb047420c3d9ec576aa792d8e9ca/src/cons_tuples_impl.rs#L26-L32

self.iter.next() instead of self.iter.next_back() is one obvious bug here!

ConsTuples is an internal detail of the iproduct macro and the cartesian product does not implement DoubleEndedIterator (and it can't as currently defined) so this is not used inside the crate (and won't). No one noticed (in 8 years) such an obvious error so it's very unlikely used outside of itertools, right?

I suggest we remove the implementation (breaking change?). Or we could fix it, add a test (and specialize rfold).

phimuemue commented 7 months ago

Nice catch! Yes, let’s remove it and see if anyone misses it. A wrong implementation does not help anybody.