Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pre-RFC: Save and load state for Iterator #2558

Closed
tuxzz opened this issue Oct 7, 2018 · 7 comments
Closed

Pre-RFC: Save and load state for Iterator #2558

tuxzz opened this issue Oct 7, 2018 · 7 comments
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.

Comments

@tuxzz
Copy link

tuxzz commented Oct 7, 2018

On my project, I recently found a problem that I can't implement a comfortable interface that do operation on Iterator directly using existing API.

/* not work because iterator state is changed after each operation */
pub fn do_operateion<'a, T, U>(input_iter: &mut T, output_iter: &mut U)
where
    T: Iterator<Item = &'a f32>,
    U: Iterator<Item = &'a mut f32>,
{
    let temp = input_iter.skip(2).map(...).sum();
    output_iter.step_by(2).zip(input_iter).for_each(|a, b| *a * temp);
    output_iter.skip(1).step_by(2).zip(input_iter).for_each(|a, b| *a * temp * 123.0);
}

Hence the idea of adding StateSavableIterator trait:

pub trait StateSavableIterator: Iterator {
    type IteratorState;

    fn save(&self) -> IteratorState;
    fn load(&mut self, state: &IteratorState) -> Self;
}

So that I can:

pub fn do_operateion<'a, T, U>(input_iter: &mut T, output_iter: &mut U)
where
    T: StateSavableIterator<Item = &'a f32>,
    U: StateSavableIterator<Item = &'a mut f32>,
{
    let input_init = input_iter.save();
    let out_init = output_iter.save();
    let temp = input_iter.load(input_init).skip(2).map(...).sum();
    output_iter.load(out_init).step_by(2).zip(input_iter.load(input_init)).for_each(|a, b| *a * temp);
    output_iter.load(out_init).skip(1).step_by(2).zip(input_iter.load(input_init)).for_each(|a, b| *a * temp * 123.0);
}

We don't have to pass &slice anymore! This trait benefits to writing more generic and readable API.

@SimonSapin
Copy link
Contributor

You can use the Clone trait for this. Require a T: Iterator<Item = …> + Clone bound, then iter.save() is iter.clone() and iter.load(x) is iter = x.

@Centril Centril added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Oct 7, 2018
@tuxzz
Copy link
Author

tuxzz commented Oct 7, 2018

You can use the Clone trait for this. Require a T: Iterator<Item = …> + Clone bound, then iter.save() is iter.clone() and iter.load(x) is iter = x.

Clone does not work on IterMut because of noalias rule.
But an output_iter have to be an iterator that has &mut Item

@SimonSapin
Copy link
Contributor

slice::IterMut<T> could not implement StateSavableIterator for exactly the same reason. Save, call next() to get a &mut T, load, call next again and get a second &mut T aliasing the same item.

@SimonSapin
Copy link
Contributor

To give a more helpful answer, if output_iter in your code is indeed slice::IterMut, consider passing the actual slice instead and calling .iter_mut() on it twice. This will let the borrow checker make sure that the lifetimes of the two iterators do not overlap. (Until NLL lands you’ll also likely need to add a {…} anonymous block to limit the scope of the first iterator.)

@burdges
Copy link

burdges commented Oct 7, 2018

Iterator::by_ref almost sounds like what you want here.

If you actually want to abstract slice::IterMut to restart iteration, then an for<'a> &'a mut T: IntoIterator bound should theoretically work, except right now a compiler bug makes this mostly useless.

In my opinion, the best solution would be FnBorrow* closure traits that supports reborrowing in closures by providing access to the lifetime of the closure's self reference. It'd make tricks like this work.

fn foo<F>(f: F) where F: FnBorrowMut() -> impl Iterator<Item: 'borrow> { ... }

foo(|| my_collection.iter_mut().take(5).filter(bar))

In the short term, foo<B: BorrowMut<T>>(&mut [B]) is more flexible than foo(&mut [T]).

@clarfonthey
Copy link
Contributor

I don't see any case where Clone isn't viable but a load-and-save state is both viable and sound.

@jonas-schievink
Copy link
Contributor

Closing as per the above comments, this sort of interface does not seem to solve the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests

6 participants