rust-clippy icon indicating copy to clipboard operation
rust-clippy copied to clipboard

Regarding range_plus_one

Open leonardo-m opened this issue 3 years ago • 3 comments

Summary

This code:

#![warn(clippy::pedantic)]

fn foo(i: u32) {
    for i in 0 .. i + 1 {
        println!("{}", i);
    }
}

Gives:

warning: an inclusive range would be more readable
 --> src/main.rs:4:14
  |
4 |     for i in 0 .. i + 1 {
  |              ^^^^^^^^^^ help: use: `0..=i`
  |
note: the lint level is defined here
 --> src/main.rs:1:9
  |
1 | #![warn(clippy::pedantic)]
  |         ^^^^^^^^^^^^^^^^
  = note: `#[warn(clippy::range_plus_one)]` implied by `#[warn(clippy::pedantic)]`
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#range_plus_one

But ..= isn't performant, especially in inner loops. So for me this seems a bad warning.

Lint Name

No response

Reproducer

I tried this code:

<code>

I saw this happen:

<output>

I expected to see this happen:

Version

No response

Additional Labels

No response

leonardo-m avatar Jan 20 '22 10:01 leonardo-m

Godbolt

I'm not sure why ..= produces different output than ..n + 1, we'll have to fill a report on the compiler directly, as this isn't a Clippy issues.

blyxyas avatar Oct 06 '24 16:10 blyxyas

This lint was downgraded to pedantic in response to https://github.com/rust-lang/rust-clippy/issues/2217. So I suppose this issue is about whether it's okay for clippy to suggest deoptimizations in pedantic.

saethlin avatar Oct 06 '24 20:10 saethlin

Godbolt

I'm not sure why ..= produces different output than ..n + 1, we'll have to fill a report on the compiler directly, as this isn't a Clippy issues.

This is a pretty late reply, but for reference, the output is different because the behavior is different between the 2 versions. Using 0..i+1 will cause a overflow if i == $integer::MAX. The 0..=i versions will work for all values for i. Here is a playground you can test this in. Running the code in debug will trigger a overflow panic, while running in release will end up printing nothing (i+1 overflows to 0 meaning you iterate over 0..0).

In my opinion the lint is fine. Very rarely do you want or expect the overflow behavior. In case you really need it you can always use wrapping_add

Brezak avatar Oct 17 '24 19:10 Brezak