feat(corelib): Iterator::advance_by

[MagisterDallis]

Adds advance_by to Iterator

[reviewable-StarkWare]

This change is 

[orizi]

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

---

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

---

corelib/src/iter/traits/iterator.cairo line 79 at r1 (raw file):

            }
        };
        res

Suggestion:

        ref self: T, mut n: usize,
    ) -> Result<
        (), NonZero<usize>,
    > {
        let mut res = Result::Ok(());
        while let Option::Some(nz_n) = n.try_into() {
            if Self::next(ref self).is_none() {
                res = Result::Err(nz_n);
                break;
            }
            n -= 1;
        }
        res

[orizi]

Reviewed 1 of 3 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @MagisterDallis)

[MagisterDallis]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @MagisterDallis)

---

corelib/src/iter/traits/iterator.cairo line 79 at r1 (raw file):

            }
        };
        res

error: Parameter of trait function Iterator::advance_by can't be defined as mutable.
--> corelib/src/iter/traits/iterator.cairo:68:22
ref self: T, mut n: usize,

n: usize and then let mut n = n ?

[MagisterDallis]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @MagisterDallis)

---

a discussion (no related file):

Previously, orizi wrote…

@TomerStarkware for 2nd eye.

@orizi I'm actually hitting a bigger issue which is that

error: Trait has no implementation in context: core::traits::Drop::<Iterator::Item>.
 --> corelib/src/test/iter_test.cairo:5:21
    assert_eq!(iter.advance_by(2), Result::Ok(()));
                    ^^^^^^^^^^

A workaround would be to have something like:

pub trait IteratorEx<T, impl I: Iterator<T>, +Destruct<T>, +Drop<I::Item>> {
...
}

impl IteratorExImpl<T, impl I: Iterator<T>, +Destruct<T>, +Drop<I::Item>> of IteratorEx<T, I> {}

and expose IteratorEx in the prelude, but perhaps there's better to do than that?

[orizi]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @MagisterDallis)

---

a discussion (no related file):

Previously, MagisterDallis wrote…

@orizi I'm actually hitting a bigger issue which is that

error: Trait has no implementation in context: core::traits::Drop::<Iterator::Item>.
 --> corelib/src/test/iter_test.cairo:5:21
    assert_eq!(iter.advance_by(2), Result::Ok(()));
                    ^^^^^^^^^^

A workaround would be to have something like:

pub trait IteratorEx<T, impl I: Iterator<T>, +Destruct<T>, +Drop<I::Item>> {
...
}

impl IteratorExImpl<T, impl I: Iterator<T>, +Destruct<T>, +Drop<I::Item>> of IteratorEx<T, I> {}

and expose IteratorEx in the prelude, but perhaps there's better to do than that?

are you fully rebased?

---

corelib/src/iter/traits/iterator.cairo line 79 at r1 (raw file):

Previously, MagisterDallis wrote…

error: Parameter of trait function Iterator::advance_by can't be defined as mutable.
--> corelib/src/iter/traits/iterator.cairo:68:22
ref self: T, mut n: usize,

n: usize and then let mut n = n ?

that would be good enough.

[MagisterDallis]

Reviewable status: 2 of 3 files reviewed, all discussions resolved (waiting on @orizi)

---

a discussion (no related file):

Previously, orizi wrote…

are you fully rebased?

On 6837d54

[MagisterDallis]

Reviewable status: 2 of 3 files reviewed, all discussions resolved (waiting on @orizi)

---

corelib/src/iter/traits/iterator.cairo line 79 at r1 (raw file):

Previously, orizi wrote…

that would be good enough.

Done.

[orizi]

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

---

a discussion (no related file):

Previously, MagisterDallis wrote…

On 6837d54

in any case - IteratorEx in prelude won't happen.
we will just fix the underlying issue here.

[orizi]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @MagisterDallis and @TomerStarkware)

---

corelib/src/iter/traits/iterator.cairo line 82 at r3 (raw file):

        };
        res
    }

Suggestion:

fn advance_by<+Destruct<T>, +Destruct<Self::Item>>(
        ref self: T, n: usize,
    ) -> Result<
        (), NonZero<usize>,
    > {
        let mut n = n;
        let mut res = Result::Ok(());
        while let Option::Some(nz_n) = n.try_into() {
            if let Option::Some(_) = Self::next(ref self) {
                n -= 1;
            } else {
                res = Result::Err(nz_n);
                break;
            }
        };
        res
    }

[orizi]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @MagisterDallis and @TomerStarkware)

---

corelib/src/iter/traits/iterator.cairo line 82 at r3 (raw file):

        };
        res
    }

(this allows using Destruct instead of Drop, does not solve the other issue)

[MagisterDallis]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @orizi and @TomerStarkware)

---

a discussion (no related file):

Previously, orizi wrote…

in any case - IteratorEx in prelude won't happen.
we will just fix the underlying issue here.

tracking in #7060

---

corelib/src/iter/traits/iterator.cairo line 82 at r3 (raw file):

Previously, orizi wrote…

(this allows using Destruct instead of Drop, does not solve the other issue)

Done.

[orizi]

Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @MagisterDallis and @TomerStarkware)

---

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

            }
        };
        res

It seems after the fix (#7067) there is still an issue with loops - but this would work instead (and would possibly be just a bit more efficient)

Suggestion:

        if let Option::Some(nz_n) = n.try_into() {
            if let Option::Some(_) = Self::next(ref self) {
                Self:::advance_by(Self::advance_by(ref self, n - 1)
            } else {
                Result::Err(nz_n)
            }
        } else {
            Result::Ok(())
        }

[MagisterDallis]

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

---

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

Previously, orizi wrote…

It seems after the fix (#7067) there is still an issue with loops - but this would work instead (and would possibly be just a bit more efficient)

Done.

[orizi]

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

---

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

---

corelib/src/iter/traits/iterator.cairo line 74 at r5 (raw file):

        (), NonZero<usize>,
    > {
        let mut n = n;

not required.

[MagisterDallis]

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

---

corelib/src/iter/traits/iterator.cairo line 74 at r5 (raw file):

Previously, orizi wrote…

not required.

Done.

[orizi]

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @MagisterDallis)

---

a discussion (no related file):

Previously, orizi wrote…

@TomerStarkware for 2nd eye.

ping

[orizi]

Reviewed 2 of 2 files at r7, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @MagisterDallis)

[TomerStarkware]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @MagisterDallis)