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

redundant_pattern_matching: if let Err(_) = ... else and if ....is_err() are not the same #14113

Open
KizzyCode opened this issue Jan 30, 2025 · 0 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

Comments

@KizzyCode
Copy link

KizzyCode commented Jan 30, 2025

Summary

The clippy lint is suggesting to replace if let pattern = blurp else with if blurp.is_err(), which are not synonymous. The problem with the drop order is already warned about; however there's another point that is specific to let ... else:

let ... else forces you to handle the else-case by diverging, whereas a normal if-clause does not. Bugs like goto fail (different language, same underlying problem) show that this is a problem in practice. let ... else has different, significantly stronger semantics regarding error handling and bug resilience; and clippy's suggestion to just go if is unfortunate in my opinion.

If people use let ... else, it is not unlikely that they use it intentionally to get those stronger semantics, and I personally consider this warning (and the suggested replacement) a false-positive for let ... else-cases (at least on the default warning level).

Lint Name

redundant_pattern_matching

Reproducer

I tried this code:

if let Err(_) = thread_join_handle.join() {
    // Fail with error if the task panicked
    return Err(format!("Task panicked"));
}

I saw this happen:

redundant pattern matching, consider using `is_err()`
this will change drop order of the result, as well as all temporaries
add `#[allow(clippy::redundant_pattern_matching)]` if this is important
for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching
`#[warn(clippy::redundant_pattern_matching)]` on by default

I expected to see this happen:

<nothing>

Version

rustc 1.84.0 (9fc6b4312 2025-01-07)
binary: rustc
commit-hash: 9fc6b43126469e3858e2fe86cafb4f0fd5068869
commit-date: 2025-01-07
host: aarch64-apple-darwin
release: 1.84.0
LLVM version: 19.1.5

Additional Labels

No response

@KizzyCode KizzyCode 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 Jan 30, 2025
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
Projects
None yet
Development

No branches or pull requests

1 participant