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

comparison_chain false positive when constant depends on generic parameter #13739

Open
clarfonthey opened this issue Nov 27, 2024 · 1 comment

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants