Skip to content

Comments

Disable non_exhaustive_omitted_patterns within matches! macro#144331

Merged
bors merged 4 commits intorust-lang:masterfrom
jplatte:matches-allow-non_exhaustive_omitted_patterns
Jul 26, 2025
Merged

Disable non_exhaustive_omitted_patterns within matches! macro#144331
bors merged 4 commits intorust-lang:masterfrom
jplatte:matches-allow-non_exhaustive_omitted_patterns

Conversation

@jplatte
Copy link
Contributor

@jplatte jplatte commented Jul 22, 2025 •
edited by rustbot
Loading

Closes #117304.

I believe I can skip all of the bootstrap stuff mentioned in #117304 (comment) due to https://blog.rust-lang.org/inside-rust/2025/05/29/redesigning-the-initial-bootstrap-sequence/, right?

cc @Jules-Bertholet

@rustbot
Copy link
Collaborator

rustbot commented Jul 22, 2025

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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. labels Jul 22, 2025
@jplatte
Copy link
Contributor Author

jplatte commented Jul 22, 2025

Oh, I should probably add a test that enables the lint, uses matches! and passes w/o warnings? I'd be happy about any pointers, but I can probably also figure it out myself when I take another look (for today I'm heading to bed).

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Jules-Bertholet
Copy link
Contributor

Jules-Bertholet commented Jul 22, 2025

You need #[allow_internal_unstable(stmt_expr_attributes)]

@tgross35
Copy link
Contributor

tgross35 commented Jul 22, 2025

Could you add a regression test at library/coretests/tests/macros.rs?

r? @Nadrieril since you expressed some preference to how this is implemented on #117304

This seems like objectively correct behavior but it is still a user-facing adjustment to lints and builtin macros, so we'll also ask somebody from lang to take a look once CI passes.

@rustbot
Copy link
Collaborator

rustbot commented Jul 22, 2025

Nadrieril is currently at their maximum review capacity.
They may take a while to respond.

@jplatte
Copy link
Contributor Author

jplatte commented Jul 23, 2025

You need #[allow_internal_unstable(stmt_expr_attributes)]

Yup, I noticed. I guess ./x check library/core was not enough to actually see whether the macro is okay :)

(I'm guessing because that unstable feature is enabled by core, so uses of the macro within core are fine)

@Nadrieril
Copy link
Member

Nadrieril commented Jul 23, 2025

This looks good. I also don't think this is relevant to lang, it's an adjustment to a nightly feature, no?

@tgross35
Copy link
Contributor

tgross35 commented Jul 23, 2025

You're right, missed that this nightly features were required to get this flagged in the first place.

@jplatte jplatte force-pushed the matches-allow-non_exhaustive_omitted_patterns branch from 9454aa2 to ab6a52a Compare July 23, 2025 12:34
@rustbot

This comment has been minimized.

Sign in to view
@jplatte
Copy link
Contributor Author

jplatte commented Jul 23, 2025

I've added a test and made sure it failed (to compile coretests) when the new #[allow] is commented out.

@jplatte jplatte force-pushed the matches-allow-non_exhaustive_omitted_patterns branch from ab6a52a to 0e30629 Compare July 23, 2025 12:36
@jplatte jplatte mentioned this pull request Jul 23, 2025
Merged
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Nadrieril
Copy link
Member

Nadrieril commented Jul 26, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 26, 2025

📌 Commit 25ed288 has been approved by Nadrieril

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 26, 2025
@jplatte
Copy link
Contributor Author

jplatte commented Jul 26, 2025

So, after CI failed initially I updated the test expectations of the failing mir-opt tests (by hand, because I have no clue how to programatically 'bless' the new output). All that changed is the addition of an empty scope N {}, so I think that's probably fine?

@Nadrieril
Copy link
Member

Nadrieril commented Jul 26, 2025 •
edited
Loading

Yeah I looked at the diff output it's fine. I think such scopes are added if needed for lints which is what's happening here.

Did you try something like ./x.py test tests/mir-opt --bless? That's what I use.

@matthiaskrgr matthiaskrgr mentioned this pull request Jul 26, 2025
Merged
bors added a commit that referenced this pull request Jul 26, 2025
@bors
Auto merge of #144502 - matthiaskrgr:rollup-o81uk9m, r=matthiaskrgr
133798f 
Rollup of 10 pull requests

Successful merges:

 - #144331 (Disable non_exhaustive_omitted_patterns within matches! macro)
 - #144376 (Suggest unwrapping when private method name is available in inner type)
 - #144421 (Call `is_parsed_attribute` rather than keeping track of a list of parsed attributes manually)
 - #144424 (Allow setting `release-blog-post` label with rustbot)
 - #144427 (rename ext_tool_checks to extra_checks and use mod.rs)
 - #144435 (rustc-dev-guide subtree update)
 - #144448 (Limit defaultness query to impl of trait)
 - #144462 (Allow pretty printing paths with `-Zself-profile-events=args`)
 - #144463 (change_tracker: fix a typo)
 - #144468 (resolve: Do not create `NameResolutions` on access unless necessary)

r? `@ghost`
`@rustbot` modify labels: rollup
@jplatte
Copy link
Contributor Author

jplatte commented Jul 26, 2025

Nah, I had no clue what to even try and it wasn't that hard to do by hand. I am used to the error message telling me what to do 😅

@bors bors merged commit c9541a2 into rust-lang:master Jul 26, 2025
10 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jul 26, 2025
rust-timer added a commit that referenced this pull request Jul 26, 2025
@rust-timer
Unrolled build for #144331
85aab14 
Rollup merge of #144331 - jplatte:matches-allow-non_exhaustive_omitted_patterns, r=Nadrieril

Disable non_exhaustive_omitted_patterns within matches! macro

Closes #117304.

I believe I can skip all of the bootstrap stuff mentioned in #117304 (comment) due to https://blog.rust-lang.org/inside-rust/2025/05/29/redesigning-the-initial-bootstrap-sequence/, right?

cc `@Jules-Bertholet`
@jplatte jplatte deleted the matches-allow-non_exhaustive_omitted_patterns branch July 26, 2025 17:28
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Jul 28, 2025
@bors
Auto merge of #144502 - matthiaskrgr:rollup-o81uk9m, r=matthiaskrgr
8f5abb7 
Rollup of 10 pull requests

Successful merges:

 - rust-lang/rust#144331 (Disable non_exhaustive_omitted_patterns within matches! macro)
 - rust-lang/rust#144376 (Suggest unwrapping when private method name is available in inner type)
 - rust-lang/rust#144421 (Call `is_parsed_attribute` rather than keeping track of a list of parsed attributes manually)
 - rust-lang/rust#144424 (Allow setting `release-blog-post` label with rustbot)
 - rust-lang/rust#144427 (rename ext_tool_checks to extra_checks and use mod.rs)
 - rust-lang/rust#144435 (rustc-dev-guide subtree update)
 - rust-lang/rust#144448 (Limit defaultness query to impl of trait)
 - rust-lang/rust#144462 (Allow pretty printing paths with `-Zself-profile-events=args`)
 - rust-lang/rust#144463 (change_tracker: fix a typo)
 - rust-lang/rust#144468 (resolve: Do not create `NameResolutions` on access unless necessary)

r? `@ghost`
`@rustbot` modify labels: rollup
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Jul 30, 2025
@matthiaskrgr
Rollup merge of rust-lang#144331 - jplatte:matches-allow-non_exhausti…
3da3651 
…ve_omitted_patterns, r=Nadrieril

Disable non_exhaustive_omitted_patterns within matches! macro

Closes rust-lang#117304.

I believe I can skip all of the bootstrap stuff mentioned in rust-lang#117304 (comment) due to https://blog.rust-lang.org/inside-rust/2025/05/29/redesigning-the-initial-bootstrap-sequence/, right?

cc `@Jules-Bertholet`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@Nadrieril Nadrieril

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

Projects

None yet

Milestone

1.90.0

Development

Successfully merging this pull request may close these issues.

non_exhaustive_omitted_patterns should not warn matches! macro

7 participants

@jplatte @rustbot @rust-log-analyzer @Jules-Bertholet @tgross35 @Nadrieril @bors