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

Clippy: Wrongly suggests Option::if_let_else in const function #7902

Closed
Mythra opened this issue Oct 30, 2021 · 3 comments
Closed

Clippy: Wrongly suggests Option::if_let_else in const function #7902

Mythra opened this issue Oct 30, 2021 · 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 I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@Mythra
Copy link

Mythra commented Oct 30, 2021

Lint name: option_if_let_else

I tried this code:

	#[must_use]
	pub const fn new_from_parts(db_version: [u8; 10], tx_version: Option<[u8; 2]>) -> Self {
		let database_version: [u8; 8] = [
			db_version[0],
			db_version[1],
			db_version[2],
			db_version[3],
			db_version[4],
			db_version[5],
			db_version[6],
			db_version[7],
		];
		let batch_version: [u8; 2] = [db_version[8], db_version[9]];

		Self {
			database_version,
			batch_version,
			tx_version: if let Some(txv) = tx_version {
				([MaybeUninit::new(txv[0]), MaybeUninit::new(txv[1])], true)
			} else {
				([MaybeUninit::uninit(); 2], false)
			},
		}
	}

Then clippy generated a warning:

error: use Option::map_or_else instead of an if let/else
  --> <project>/src/db.rs:77:16
   |
77 |               tx_version: if let Some(txv) = tx_version {
   |  _________________________^
78 | |                 ([MaybeUninit::new(txv[0]), MaybeUninit::new(txv[1])], true)
79 | |             } else {
80 | |                 ([MaybeUninit::uninit(); 2], false)
81 | |             },
   | |_____________^ help: try: `tx_version.map_or_else(|| ([MaybeUninit::uninit(); 2], false), |txv| ([MaybeUninit::new(txv[0]), MaybeUninit::new(txv[1])], true))`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#option_if_let_else

I'm generally not a fan of this lint anyone, but I do it to hopefully be consistent with the rest of the ecosystem and juggling lints is really hard to do. However, replacing this particular code results in a failure to compile on stable as you cannot call if_let_else in a constant function. Meaning the resulting checked code fails to compile.

Meta

Rust version (rustc -Vv):

rustc 1.56.0 (09c42c458 2021-10-18)
binary: rustc
commit-hash: 09c42c45858d5f3aedfa670698275303a3d19afa
commit-date: 2021-10-18
host: x86_64-unknown-linux-gnu
release: 1.56.0
LLVM version: 13.0.0
@Mythra Mythra 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 Oct 30, 2021
@xFrednet
Copy link
Member

@rustbot label +I-suggestion-causes-error

@rustbot rustbot added the I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied label Oct 30, 2021
@giraffate
Copy link
Contributor

The option_if_let_else was fixed not to warn in const function by #7573. Can you try currently beta (1.57.0)?

@Mythra
Copy link
Author

Mythra commented Nov 2, 2021

Thanks! Sorry I missed that I did try looking for already existing PRs or issues but I guess not thoroughly enough! Seems to work for me on beta, thanks for the help'

@Mythra Mythra closed this as completed Nov 2, 2021
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 I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
Development

No branches or pull requests

4 participants