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

False positive in reversed_empty_ranges used with Vec::splice #5689

Closed
dtolnay opened this issue Jun 6, 2020 · 1 comment · Fixed by #5692
Closed

False positive in reversed_empty_ranges used with Vec::splice #5689

dtolnay opened this issue Jun 6, 2020 · 1 comment · Fixed by #5692
Labels
C-bug Category: Clippy is not doing the correct thing good-first-issue These issues are a good way to get started with Clippy I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@dtolnay
Copy link
Member

dtolnay commented Jun 6, 2020

fn main() {
    let mut v = vec![3, 2, 1];
    v.splice(0..0, std::iter::repeat(0).take(4));
    println!("{:?}", v);  // [0, 0, 0, 0, 3, 2, 1]
}
error: this range is empty so it will yield no values
 --> src/main.rs:3:14
  |
3 |     v.splice(0..0, std::iter::repeat(0).take(4));
  |              ^^^^
  |
  = note: `#[deny(clippy::reversed_empty_ranges)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges

I believe this is a correct use of an empty range.
https://doc.rust-lang.org/std/vec/struct.Vec.html#method.splice

Mentioning @ebroto @yaahc who touched this lint recently in #5583.


clippy 0.0.212 (826cb06 2020-06-05)

@ebroto
Copy link
Member

ebroto commented Jun 6, 2020

Hey @dtolnay, thanks for opening the issue.

So, this is the second issue (the other one being #5628) related to N..N being used intentionally. A quick github search shows that the lint is already being ignored and in most cases it's because of the same pattern, which makes me think that it's more idiomatic that we originally thought.

The original spirit of the lint was to avoid confusion when the developer uses inverted bounds, both the result of the range being empty and a runtime panic in case of indexing a slice. N..N is kind of an edge case, and probably less prone to confusion (and does not panic).

I would be now in favor of not linting N..N, maybe just when used as an argument to a for loop, for backwards compatibility with reverse_range_loop which was superseded by this lint.

@flip1995 flip1995 added good-first-issue These issues are a good way to get started with Clippy C-bug Category: Clippy is not doing the correct thing labels Jun 6, 2020
@bors bors closed this as completed in ff0993c Jun 8, 2020
@rustbot rustbot added the I-false-positive Issue: The lint was triggered on code it shouldn't have label Sep 13, 2021
blyxyas added a commit to blyxyas/rust-clippy that referenced this issue Jan 6, 2023
…or` loop.

Reading the documentation for the lint, one could expect that the lint works in all cases that `X == Y`. This is false.

While the lint was updated, the documentation wasn't.

More information about the `N..N` problem in rust-lang#5689 and rust-lang#5628
bors added a commit that referenced this issue Jan 6, 2023
[#10167] Clarify that the lint only works if x eq. y in a `for` loop.

Reading the documentation for the lint, one could expect that the lint works in all cases that `X == Y`. This is false.

While the lint was updated, the documentation wasn't.

More information about the `N..N` problem in #5689 and #5628

---

Fixes #10167
changelog: [`reversed_empty_ranges`]: Update and clarify documentation
blyxyas added a commit to blyxyas/rust-clippy that referenced this issue Jan 12, 2023
…or` loop.

Reading the documentation for the lint, one could expect that the lint works in all cases that `X == Y`. This is false.

While the lint was updated, the documentation wasn't.

More information about the `N..N` problem in rust-lang#5689 and rust-lang#5628
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 good-first-issue These issues are a good way to get started with Clippy I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants