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

Bettinabettine

Your link does not provide a snippet. That aside, the compiler seems to provide all the information you need. – Mcnew

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… – Sedgewinn

added embedded code – Bettinabettine

@Mcnew how do i fix the problem? – Bettinabettine

@Mcnew 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. – Chamblee

Answer 1

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().

Chamblee

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. – Spline

@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. – Chamblee