Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regarding range_plus_one #8317

Open
leonardo-m opened this issue Jan 20, 2022 · 3 comments
Open

Regarding range_plus_one #8317

leonardo-m opened this issue Jan 20, 2022 · 3 comments
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@leonardo-m
Copy link

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 leonardo-m added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Jan 20, 2022
@blyxyas
Copy link
Member

blyxyas commented Oct 6, 2024

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.

@saethlin
Copy link
Member

saethlin commented Oct 6, 2024

This lint was downgraded to pedantic in response to #2217. So I suppose this issue is about whether it's okay for clippy to suggest deoptimizations in pedantic.

@Brezak
Copy link

Brezak commented Oct 17, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

No branches or pull requests

4 participants