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.

Sign up for GitHub

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

Jump to bottom

comparison_chain false positive when constant depends on generic parameter #13739

Open
clarfonthey opened this issue Nov 27, 2024 · 1 comment · May be fixed by #13762
Open

comparison_chain false positive when constant depends on generic parameter #13739

clarfonthey opened this issue Nov 27, 2024 · 1 comment · May be fixed by #13762

Comments

@clarfonthey
Copy link
Contributor

Description

Constants in patterns cannot depend on generic parameters, so, this should not recommend rewriting into a match when the constants depend on generic parameters.

Example can be found in my hetero crate:

if idx < I::LEN {
    self.init.get_homo(idx)
} else if idx == I::LEN {
    Some(self.last.into())
} else {
    None
}

Lint output:

warning: `if` chain can be rewritten with `match`
  --> src/snoc.rs:89:9
   |
89 | /         if idx < I::LEN {
90 | |             self.init.get_homo(idx)
91 | |         } else if idx == I::LEN {
92 | |             Some(self.last.into())
93 | |         } else {
94 | |             None
95 | |         }
   | |_________^
   |
   = help: consider rewriting the `if` chain to use `cmp` and `match`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain
   = note: `#[warn(clippy::comparison_chain)]` on by default

If rewritten to a match, like the suggestion implies:

match idx {
    0..I::LEN => self.init.get_homo(idx),
    I::LEN => Some(self.last.into()),
    _ => None,
}

The following error is triggered:

error[E0158]: constant pattern depends on a generic parameter
  --> src/snoc.rs:90:16
   |
90 |             0..I::LEN => self.init.get_homo(idx),
   |                ^^^^^^

error[E0158]: constant pattern depends on a generic parameter
  --> src/snoc.rs:91:13
   |
91 |             I::LEN => Some(self.last.into()),
   |             ^^^^^^

Version

rustc 1.85.0-nightly (7db7489f9 2024-11-25)
binary: rustc
commit-hash: 7db7489f9bc274cb60c4956bfa56de0185eb1b9b
commit-date: 2024-11-25
host: x86_64-unknown-linux-gnu
release: 1.85.0-nightly
LLVM version: 19.1.4

Additional Labels

No response
@y21
Copy link
Member

y21 commented Nov 27, 2024

The lint is suggesting

match idx.cmp(&I::LEN) {
    Ordering::Less => self.init.get_homo(idx),
    Ordering::Equal => Some(self.last.into()),
    Ordering::Greater => None
}

which should work. There's a help message in the diagnostic that mentions to use cmp, but I can see how it's easy to miss. Might be a good idea to provide an actual suggestion.

@samueltardieu samueltardieu linked a pull request Nov 30, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Better help message for comparison_chain lint samueltardieu/rust-clippy
2 participants
@clarfonthey @y21