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

Potential improvement to reversed_empty_ranges #2477

Open
pzmarzly opened this issue Feb 22, 2018 · 3 comments
Open

Potential improvement to reversed_empty_ranges #2477

pzmarzly opened this issue Feb 22, 2018 · 3 comments
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages

Comments

@pzmarzly
Copy link

pzmarzly commented Feb 22, 2018

While it is stated that this lint cannot catch loops over dynamically defined ranges, it could be improved to catch the following common pattern:

let x = 5u32;
for i in x..0 { ... }

Note that it is a range over unsigned values, so it makes no sense for the right side to be zero (as the range will always be empty then). Similar check could be done for signed integers by iXXX::MIN const.

@mxxo
Copy link

mxxo commented Mar 27, 2020

I don't want to hijack this issue, but clippy also misses adapters on invalid ranges like:

// skips loop 
for i in (10..0).rev() {
         println!("{}", i);
}

@flip1995 flip1995 added the C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages label Mar 27, 2020
@Zalathar
Copy link

Zalathar commented Nov 22, 2021

I ran into a similar issue with a backwards loop that modifies a vector:

// Wrong!
for i in (vec.len() - 1)..=0 {
    // Code that structurally modifies vec,
    // so we need backwards traversal
    // and can’t use a slice iterator.
}

The correct range is (0..vec.len()).rev(), which also conveniently avoids unsigned overflow when vec is empty.

@giraffate giraffate changed the title Potential improvement to reverse_range_loop Potential improvement to reversed_empty_ranges Nov 29, 2021
@giraffate
Copy link
Contributor

reverse_range_loop is now included in reversed_empty_ranges, so I changed this title.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages
Projects
None yet
Development

No branches or pull requests

5 participants