feat(corelib): core::iter::zip

[julio4]

'Zips up' two iterators into a single iterator of pairs.

zip() returns a new iterator that will iterate over two other
iterators, returning a tuple where the first element comes from the
first iterator, and the second element comes from the second iterator.

In other words, it zips two iterators together, into a single one.

If either iterator returns Option::None, next from the zipped iterator
will return Option::None.
If the zipped iterator has no more elements to return then each further attempt to advance
it will first try to advance the first iterator at most one time and if it still yielded an
item try to advance the second iterator at most one time.

Examples

Basic usage:

let mut iter = array![1, 2, 3].into_iter().zip(array![4, 5, 6].into_iter());

assert_eq!(iter.next(), Option::Some((1, 4)));
assert_eq!(iter.next(), Option::Some((2, 5)));
assert_eq!(iter.next(), Option::Some((3, 6)));
assert_eq!(iter.next(), Option::None);

[reviewable-StarkWare]

This change is 

[orizi]

Reviewed 3 of 4 files at r1, all commit messages.
Reviewable status: 3 of 4 files reviewed, 2 unresolved discussions (waiting on @julio4)

---

corelib/src/iter/adapters/zip_adapter.cairo line 70 at r1 (raw file):

    +Destruct<IB>,
> of Iterator<Zip<A, B>> {
    type Item = (IA, IB);

probably preferable:

Suggestion:

impl ZipIterator<
    A,
    B,
    impl IterA: Iterator<A>,
    impl IterB: Iterator<B>,
    +Destruct<A>,
    +Destruct<B>,
    +Destruct<IterA::Item>,
    +Destruct<IterB::Item>,
> of Iterator<Zip<A, B>> {
    type Item = (IterA::Item, IterB::Item);

---

corelib/src/test/iter_test.cairo line 25 at r1 (raw file):

    assert_eq!(iter.next(), Option::Some(((2, 5), 8)));
    assert_eq!(iter.next(), Option::Some(((3, 6), 9)));
    assert_eq!(iter.next(), Option::None);

Suggestion:

    let mut iter = zip(array![1, 2, 3], array![4, 5, 6]);

    assert_eq!(iter.next(), Option::Some((1, 4)));
    assert_eq!(iter.next(), Option::Some((2, 5)));
    assert_eq!(iter.next(), Option::Some((3, 6)));
    assert_eq!(iter.next(), Option::None);

    // Nested zips are also possible:
    let mut iter = zip(zip(array![1, 2, 3], array![4, 5, 6]), array![7, 8, 9]);

    assert_eq!(iter.next(), Option::Some(((1, 4), 7)));
    assert_eq!(iter.next(), Option::Some(((2, 5), 8)));
    assert_eq!(iter.next(), Option::Some(((3, 6), 9)));
    assert_eq!(iter.next(), Option::None);

[orizi]

Reviewable status: 3 of 4 files reviewed, 3 unresolved discussions (waiting on @julio4)

---

corelib/src/iter/adapters/zip_adapter.cairo line 36 at r1 (raw file):

/// let zs = array![7, 8, 9];
///
/// let mut iter = zip(zip(xs, ys), zs);

inline arrays - same as in test.

Code quote:

/// let xs = array![1, 2, 3];
/// let ys = array![4, 5, 6];
///
/// let mut iter = zip(xs, ys);
///
/// assert_eq!(iter.next(), Option::Some((1, 4)));
/// assert_eq!(iter.next(), Option::Some((2, 5)));
/// assert_eq!(iter.next(), Option::Some((3, 6)));
/// assert_eq!(iter.next(), Option::None);
///
/// // Nested zips are also possible:
/// let xs = array![1, 2, 3];
/// let ys = array![4, 5, 6];
/// let zs = array![7, 8, 9];
///
/// let mut iter = zip(zip(xs, ys), zs);

[orizi]

Reviewable status: 3 of 4 files reviewed, 3 unresolved discussions (waiting on @julio4)

---

a discussion (no related file):
@TomerStarkware for 2nd eye

[enitrat]

Reviewable status: 3 of 4 files reviewed, 4 unresolved discussions (waiting on @julio4 and @orizi)

---

corelib/src/iter/adapters/zip_adapter.cairo line 15 at r1 (raw file):

/// Converts the arguments to iterators and zips them.
///

You can pre-emptively link this to [Iterator::zip] as I guess that's going to come next

Code snippet:

/// See the documentation of [`Iterator::zip`] for more.

[julio4]

Reviewable status: 3 of 4 files reviewed, 4 unresolved discussions (waiting on @enitrat and @orizi)

---

corelib/src/iter/adapters/zip_adapter.cairo line 36 at r1 (raw file):

Previously, orizi wrote…

inline arrays - same as in test.

Done.
However I would argue that it's a bit more readable without inlining, especially for nested zip, and it could make sense to make it as readable as possible in the documentation.

---

corelib/src/iter/adapters/zip_adapter.cairo line 70 at r1 (raw file):

Previously, orizi wrote…

probably preferable:

Done.

---

corelib/src/test/iter_test.cairo line 25 at r1 (raw file):

    assert_eq!(iter.next(), Option::Some(((2, 5), 8)));
    assert_eq!(iter.next(), Option::Some(((3, 6), 9)));
    assert_eq!(iter.next(), Option::None);

Done.

[julio4]

Reviewable status: 2 of 4 files reviewed, 4 unresolved discussions (waiting on @enitrat and @orizi)

---

corelib/src/iter/adapters/zip_adapter.cairo line 15 at r1 (raw file):

Previously, enitrat (Mathieu) wrote…

You can pre-emptively link this to [Iterator::zip] as I guess that's going to come next

There's still some issue with default impl so I prefer not blocking thing and I'll add in a separate PR

[orizi]

Reviewed 1 of 2 files at r2, all commit messages.
Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @enitrat)

---

corelib/src/iter/adapters/zip_adapter.cairo line 36 at r1 (raw file):

Previously, julio4 (Julio) wrote…

Done.
However I would argue that it's a bit more readable without inlining, especially for nested zip, and it could make sense to make it as readable as possible in the documentation.

in the nested - possibly - i don't like it much in cases when there's no actual meaning to the arrays themselves.

[julio4]

Reviewable status: 3 of 4 files reviewed, 1 unresolved discussion (waiting on @enitrat and @orizi)

---

corelib/src/iter/adapters/zip_adapter.cairo line 36 at r1 (raw file):

Previously, orizi wrote…

in the nested - possibly - i don't like it much in cases when there's no actual meaning to the arrays themselves.

Ok! Should I still keep the arrays for nested case in the documentation or no need?

[orizi]

Reviewed 1 of 2 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @enitrat)

[julio4]

I still have an issue using +IntoIterator<U> in Iterator::zip.
Using +Iterator<U> works fine, but is less flexible.

Reviewable status: 0 of 6 files reviewed, 1 unresolved discussion (waiting on @enitrat and @orizi)

[orizi]

should mostly work now - please rebase and report.

Reviewed 4 of 6 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @enitrat and @julio4)

---

corelib/src/iter/adapters/zip_adapter.cairo line 36 at r1 (raw file):

Previously, julio4 (Julio) wrote…

Ok! Should I still keep the arrays for nested case in the documentation or no need?

dealer's choice.

---

corelib/src/iter/traits/iterator.cairo line 127 at r4 (raw file):

    /// [`next`]: Iterator::next
    #[inline]
    fn zip<U, +Iterator<U> //, +IntoIterator<U>

why not intoiterator?

Code quote:

    fn zip<U, +Iterator<U> //, +IntoIterator<U>

---

corelib/src/iter/traits/iterator.cairo line 132 at r4 (raw file):

    ) -> Zip<T, U> {
        zipped_iterator(self, other //.into_iter()
        )

Suggestion:

        zipped_iterator(self, other)

[julio4]

Fixed it, see my other comment!
Lastly, would it be possible to add a first simple version of zip! macro that take fixed amount of 2 iterators in argument?
zip!(a: +IntoIterator<A>, b: +IntoIterator<B>) -> a.into_iter().zip(b.into_iter())
It would be possible to extend it to arbitrary arguments count later on without introducing breaking changes.

Reviewable status: 4 of 6 files reviewed, 3 unresolved discussions (waiting on @enitrat and @orizi)

---

corelib/src/iter/traits/iterator.cairo line 127 at r4 (raw file):

Previously, orizi wrote…

why not intoiterator?

I had an issue using IntoIterator where it always failed with 'Error: Compilation failed without any diagnostics.'
I found that it was missing trait bound +Destruct<T>, which was not reported by the LSP and the compiler.

[orizi]

hmm - looking at rust now - zip mostly makes sense in any case to be added as a function (unlike chain) - as multi zip! is much more problematic.

Reviewed 3 of 3 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @enitrat)

---

corelib/src/iter/traits/iterator.cairo line 127 at r4 (raw file):

Previously, julio4 (Julio) wrote…

I had an issue using IntoIterator where it always failed with 'Error: Compilation failed without any diagnostics.'
I found that it was missing trait bound +Destruct<T>, which was not reported by the LSP and the compiler.

the diagnostics should be shown on main now.

[enitrat]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @julio4)

[orizi]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @enitrat)