while let on iterator is a false positive in some cases?

[gnzlbg]

I have the following code from Vec's implementation:

  fn extend_desugared<I: Iterator<Item = T>>(&mut self, mut iterator: I) {
        while let Some(element) = iterator.next() {
            let len = self.len();
            if len == self.capacity() {
                let (lower, _) = iterator.size_hint();
                self.reserve(lower.saturating_add(1));
            }
            unsafe {
                ::std::ptr::write(self.get_unchecked_mut(len), element);
                // NB can't overflow since we would have had to alloc the
                // address space
                self.move_tail(1);
            }
        }
    }

How should one transform this loop to a for-loop? The help is completely insufficient:

^ help: try: `for element in iterator { .. }`

Note that the while let loop does not move iterator (it does not implement copy) but a for element in iterator { } loop would move iterator, making it unusable in the body.

[oli-obk]

We should not be linting if the iterator is used at all within the loop body

[HMPerson1]

I can't reproduce this. I'm trying on c251120 with:

fn issue_2358<I: Iterator<Item = usize>>(mut i: I) {
    while let Some(e) = i.next() {
        println!("{:?}", i.size_hint());
    }
}

and I don't get a lint.

[HMPerson1]

In fact, the code that checks if the iterator is used inside the loop was added with the lint (#396; 659e7c1), so it's not even that it's an older version of Clippy.

[oli-obk]

@gnzlbg can you provide a repro that works in the playpen?

[gnzlbg]

Sorry, yes this code (live at the playpen):

#![allow(dead_code)]

struct Vec<T> { v: usize, t: T }

impl<T> Vec<T> {

  fn len(&self) -> usize { self.v }
  fn capacity(&self) -> usize { self.v }
  fn reserve(&mut self, _i: usize) {  }
  fn move_tail(&mut self, _i: isize) { }
  unsafe fn get_unchecked_mut(&mut self, _i: usize) -> &mut T { &mut self.t  }

  fn extend_desugared<I: Iterator<Item = T>>(&mut self, mut iterator: I) {
        while let Some(element) = iterator.next() {
            let len = self.len();
            if len == self.capacity() {
                let (lower, _) = iterator.size_hint();
                self.reserve(lower.saturating_add(1));
            }
            unsafe {
                ::std::ptr::write(self.get_unchecked_mut(len), element);
                // NB can't overflow since we would have had to alloc the
                // address space
                self.move_tail(1);
            }
        }
    }
}

fn main() {}

produces this warning:

   Compiling playground v0.0.1 (file:///playground)
warning: this loop could be written as a `for` loop
  --> src/main.rs:14:9
   |
14 | /         while let Some(element) = iterator.next() {
15 | |             let len = self.len();
16 | |             if len == self.capacity() {
17 | |                 let (lower, _) = iterator.size_hint();
...  |
25 | |             }
26 | |         }
   | |_________^ help: try: `for element in iterator { .. }`
   |
   = note: #[warn(while_let_on_iterator)] on by default
   = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/v0.0.182/index.html#while_let_on_iterator

    Finished dev [unoptimized + debuginfo] target(s) in 1.28 secs

[gnzlbg]

Note how iterator is used in the while let's body.