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

False positive in non_exhaustive_omitted_patterns: matches! #135137

Closed
dtolnay opened this issue Jan 5, 2025 · 1 comment
Closed

False positive in non_exhaustive_omitted_patterns: matches! #135137

dtolnay opened this issue Jan 5, 2025 · 1 comment
Labels
A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. F-non_exhaustive_omitted_patterns_lint `#![feature(non_exhaustive_omitted_patterns_lint)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@dtolnay
Copy link
Member

dtolnay commented Jan 5, 2025

#![feature(non_exhaustive_omitted_patterns_lint)]
#![warn(non_exhaustive_omitted_patterns)]

use core::sync::atomic::Ordering;

pub fn is_relaxed(ordering: Ordering) -> bool {
    matches!(ordering, Ordering::Relaxed)
}
warning: some variants are not matched explicitly
 --> src/lib.rs:7:14
  |
7 |     matches!(ordering, Ordering::Relaxed)
  |              ^^^^^^^^ patterns `std::sync::atomic::Ordering::Release`, `std::sync::atomic::Ordering::Acquire`, `std::sync::atomic::Ordering::AcqRel` and 1 more not covered
  |
  = help: ensure that all variants are matched explicitly by adding the suggested match arms
  = note: the matched value is of type `std::sync::atomic::Ordering` and the `non_exhaustive_omitted_patterns` attribute was found
note: the lint level is defined here
 --> src/lib.rs:2:9
  |
2 | #![warn(non_exhaustive_omitted_patterns)]
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I believe the lint should not trigger on this code. For comparison, it correctly does not trigger on this alternative implementation of the same function:

pub fn is_relaxed(ordering: Ordering) -> bool {
    if let Ordering::Relaxed = ordering {
        true
    } else {
        false
    }
}

Conceptually, for the purpose of exhaustiveness, matches! more closely resembles if let than match, even if the standard library implementation currently involves match internally in order to support guard expressions.

The non_exhaustive_omitted_patterns should only trigger on this variation of this function.

pub fn is_relaxed(ordering: Ordering) -> bool {
    match ordering {
        Ordering::Relaxed => true,
        _ => false, // warning: non exhaustive omitted patterns
    }
}
@dtolnay dtolnay added A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. F-non_exhaustive_omitted_patterns_lint `#![feature(non_exhaustive_omitted_patterns_lint)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 5, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 5, 2025
@taiki-e
Copy link
Member

taiki-e commented Jan 6, 2025

I think this is the same issue as #117304.

@dtolnay dtolnay closed this as completed Jan 6, 2025
@jieyouxu jieyouxu removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-exhaustiveness-checking Relating to exhaustiveness / usefulness checking of patterns A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. F-non_exhaustive_omitted_patterns_lint `#![feature(non_exhaustive_omitted_patterns_lint)]` T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants