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

non_exhaustive_omitted_patterns should not warn matches! macro #117304

Open
taiki-e opened this issue Oct 28, 2023 · 2 comments
Open

non_exhaustive_omitted_patterns should not warn matches! macro #117304

taiki-e opened this issue Oct 28, 2023 · 2 comments
Labels
A-lint 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-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@taiki-e
Copy link
Member

taiki-e commented Oct 28, 2023

I tried this code:

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

use std::sync::atomic::Ordering; // #[non_exhaustive] enum

pub fn is_seqcst(order: Ordering) -> bool {
    matches!(order, Ordering::SeqCst)
}

I expected to see this happen: no warning

Instead, this happened:

warning: some variants are not matched explicitly
 --> src/lib.rs:7:14
  |
7 |     matches!(order, Ordering::SeqCst)
  |              ^^^^^ patterns `std::sync::atomic::Ordering::Relaxed`, `std::sync::atomic::Ordering::Release`, `std::sync::atomic::Ordering::Acquire` 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

playground

Meta

rustc --version --verbose:

rustc 1.75.0-nightly (2f1bd0729 2023-10-27)
binary: rustc
commit-hash: 2f1bd0729b74787f55d4cbc7818cfd787cd43a99
commit-date: 2023-10-27
host: aarch64-apple-darwin
release: 1.75.0-nightly
LLVM version: 17.0.3

@rustbot label +F-non_exhaustive_omitted_patterns_lint

@taiki-e taiki-e added the C-bug Category: This is a bug. label Oct 28, 2023
@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. F-non_exhaustive_omitted_patterns_lint `#![feature(non_exhaustive_omitted_patterns_lint)]` labels Oct 28, 2023
@clarfonthey
Copy link
Contributor

This feels like something that could be easily solved by just adding an allow for the lint inside the macro. I can't think of a situation where you would want this lint to fire here.

@fmease fmease added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 28, 2023
@Jules-Bertholet
Copy link
Contributor

Jules-Bertholet commented Oct 30, 2023

I attempted to add the allow, but it turns out to not be so simple. Log of my struggles:

  1. Add #[allow(non_exhaustive_omitted_patterns)] above the match, inside the macro: compiler complains that non_exhaustive_omitted_patterns is unstable
  2. Add #[allow_internal_unstable(non_exhaustive_omitted_patterns_lint)]: doesn't change anything
  3. Patched compiler to make allow_internal_unstable work with lints: doesn't help the bootstrap compiler
  4. Use #[cfg_attr(not(bootstrap), allow(non_exhaustive_omitted_patterns)] instead: compiler complains that #![feature(stmt_expr_attributes)] is unstable
  5. Add #[allow_internal_unstable(stmt_expr_attributes)]: but stmt_expr_attributes doesn't work with allow_internal_unstable either
  6. Patch compiler again to make stmt_expr_attributes work with allow_internal_unstable: but that does nothing for the bootstrap compiler
  7. A cfg_attred-out attribute still requires stmt_expr_attributes, so #[cfg_attr(not(bootstrap), ...] can't save us here. The entire macro definition would need to be duplicated and cfg-gated. (I gave up at this point)

@rustbot label -E-easy

@rustbot rustbot removed the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Oct 30, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 26, 2024
…stmt-expr-attributes, r=petrochenkov

Make `#![allow_internal_unstable(..)]` work with `stmt_expr_attributes`

This is a necessary first step to fixing rust-lang#117304, as explained in rust-lang#117304 (comment).

`@rustbot` label T-compiler
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 26, 2024
Rollup merge of rust-lang#117420 - Jules-Bertholet:internal-unstable-stmt-expr-attributes, r=petrochenkov

Make `#![allow_internal_unstable(..)]` work with `stmt_expr_attributes`

This is a necessary first step to fixing rust-lang#117304, as explained in rust-lang#117304 (comment).

`@rustbot` label T-compiler
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Jan 31, 2024
…-attributes, r=petrochenkov

Make `#![allow_internal_unstable(..)]` work with `stmt_expr_attributes`

This is a necessary first step to fixing #117304, as explained in rust-lang/rust#117304 (comment).

`@rustbot` label T-compiler
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
…-attributes, r=petrochenkov

Make `#![allow_internal_unstable(..)]` work with `stmt_expr_attributes`

This is a necessary first step to fixing #117304, as explained in rust-lang/rust#117304 (comment).

`@rustbot` label T-compiler
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
…-attributes, r=petrochenkov

Make `#![allow_internal_unstable(..)]` work with `stmt_expr_attributes`

This is a necessary first step to fixing #117304, as explained in rust-lang/rust#117304 (comment).

`@rustbot` label T-compiler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint 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-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants