Access iterator inside its for loop

Question

I am testing a custom bi-directional iterator in rust but i ran into error

error[E0382]: borrow of moved value: `iter`
   --> src/main.rs:40:13
    |
37  |     let mut iter = BiIterator::from(vec![1, 2, 3]);
    |         -------- move occurs because `iter` has type `BiIterator<i32>`, which does not implement the `Copy` trait
38  |     for i in iter {
    |              ----
    |              |
    |              `iter` moved due to this implicit call to `.into_iter()`
    |              help: consider borrowing to avoid moving into the for loop: `&iter`
39  |         if i == 3 {
40  |             iter.position(0);
    |             ^^^^^^^^^^^^^^^^ value borrowed here after move
    |
note: this function takes ownership of the receiver `self`, which moves `iter`

reproducable code

Your link does not provide a snippet. That aside, the compiler seems to provide all the information you need. — Masklinn, Jan 27 '22 at 08:03
In order to post a link to your code on the playground, you need to click the "Share" button. Otherwise you get a link to an empty playground… — Jmb, Jan 27 '22 at 08:06
@Masklinn The compiler correctly identifies the problem, but doesn't provide a hint how to fix it. The suggestion it provides to use `for i in &iter` is not useful because it doesn't compile (as `&iter` is not iterable), and wouldn't allow invoking `position()` inside the loop anyway. The proper solution is obvious to someone experienced in Rust, but not so to a beginner. — user4815162342, Jan 27 '22 at 08:23

user4815162342 · Accepted Answer · 2022-01-27T19:36:35.463

The for loop is taking ownership of the iterator. To use the iterator inside the loop body, you need to desugar the for loop into while let:

while let Some(i) = iter.next() {
    if i == 3 {
        iter.position(0);
    }
    println!("{}", i);
}

If you want to make your iterator usable from a for loop, you'll need to invest a bit of extra effort. You can implement Iterator for &BiIterator, and use interior mutability for pos, so position() can take &self:

// don't need RefCell because we're just mutating a number
use std::cell::Cell;

struct BiIterator<T> {
    values: Vec<T>,
    pos: Cell<usize>,
}

impl<T: Clone> Iterator for &BiIterator<T> {
    type Item = T;
    fn next(&mut self) -> Option<Self::Item> {
        self.pos.set(self.pos.get() + 1);
        self.values.get(self.pos.get() - 1).cloned()
    }
}

impl<T> BiIterator<T> {
    pub fn position(&self, new_pos: usize) {
        self.pos.set(new_pos);
    }
    pub fn prev(&mut self) {
        self.pos.set(self.pos.get() - 1);
    }
}

impl<T> std::convert::From<Vec<T>> for BiIterator<T> {
    fn from(input: Vec<T>) -> Self {
        Self {
            values: input,
            pos: Cell::new(0),
        }
    }
}

With these changes you can finally use for i in &iter as per the compiler's original suggestion:

fn main() {
    let iter = BiIterator::from(vec![1, 2, 3]);
    for i in &iter {
        if i == 3 {
            iter.position(0);
        }
        println!("{}", i);
    }
}

The above implements some additional changes:

no need for the Copy bound on T, since you're only cloning it. Any T that is Copy is automatically Clone, and cloning it can be expected to just perform the cheap copy.
bounds are almost never needed on the struct; put them just on the impl instead.
replace the if/else if let/else chain with a match or, better yet, with Option::cloned().

Side note: While `Copy: Clone`, `v.clone()` where `v` is `Copy` is **not necessarily the same** as copying. And while different behaviors are strongly discouraged, compilation may take more time because it needs to optimize `clone()` into a copy and may even not succeed in that. — Chayim Friedman, Jan 27 '22 at 10:27
@ChayimFriedman Good point! I've adjusted the wording of the last paragraph to reflect that possibility (small though it might be). As for the second part, I fully expect optimizing `clone()` into a copy to work for all types where `Clone` is derived along with `Copy`.But in any case, requiring *both* `Copy` and `Clone` probably doesn't make sense - if the code wanted to require `Copy` for better optimization, then it should just use `*val` rather than `val.clone()`, and omit the `Clone` bound. — user4815162342, Jan 27 '22 at 10:34

Access iterator inside its for loop

1 Answers1