Clippy needless_range_loop even if we don't use all values of the vec?

Hello! Consider:

fn main() {
  let seasons: Vec<&str> = vec!["spring", "summer", "fall", "winter"];
  for i in 0..4 {
      if i == 1 {
          println!("{}", seasons[i]);
      }
  }
}

Clippy says:

    Checking playground v0.0.1 (/playground)
warning: the loop variable `i` is used to index `seasons`
 --> src/main.rs:4:12
  |
4 |   for i in 0..4 {
  |            ^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
  = note: `#[warn(clippy::needless_range_loop)]` on by default
help: consider using an iterator and enumerate()
  |
4 |   for (i, <item>) in seasons.iter().enumerate().take(4) {
  |       ~~~~~~~~~~~    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

warning: `playground` (bin "playground") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.30s

Is Clippy right that I should use an iterator, even if I'm only using one value of the Vec, or wouldn't that be wasteful? Thanks!

Note that the iterator is not allocated on the heap, it is just a few fields in a struct on the stack. So I would not say it is wasteful.

3 Likes

The general rule of thumb for lints in clippy is that many are in clippy and not the compiler because not everyone agrees that they're correct. In this case, if I hit this, I'd regard it as over-zealous hoop-jumping and tell it to get stuffed.

(This is basically why I don't use clippy: every time I do, it annoys me until I turn it off.)

If the bounds you're using are indeed 0..4, then no. The minuscule overhead of the iterator is in no way going to be detectable. Even if the bounds are much larger, remember that the iterator is essentially just incrementing a pointer, doing a length check, and incrementing a counter... which is only marginally more work than using a range (which is incrementing a counter, and doing a length check).

What I would do is replace 4 with either seasons.len() or a const SEASONS: usize = 4;, because magic numbers appearing in code make it that little bit harder to understand.

(And no, not everywhere has four seasons. I lived nearly a decade somewhere that only had two seasons: "so hot you can fry an egg in the shade" and "underwater".)

2 Likes

In Rust it's almost always better to use an Iterator, even if you're just accessing few elements, or even a single element.

Iterators are designed to be easy to optimize, and give the optimizer guarantees that arbitrary loop code doesn't.

Unlike C, where [i] is a very basic unchecked arithmetic, in Rust indexing is generating substantial code with optimization-killing side effects. In this simple code the optimizer is able to deal with [i] well, but it's common for indexing with […] to generate slower and more bloated code than equivalent iterator chains.

5 Likes

Awesome, thanks!