Pattern Migration 2024: try to suggest eliding redundant binding modifiers by dianne · Pull Request #136577 · rust-lang/rust (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation16 Commits13 Checks6 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
This is based on #136475. Only the last commit is new.
This is a simpler, more restrictive alternative to #136496, meant to partially address #136047. If a pattern can be migrated to Rust 2024 solely by removing redundant binding modifiers, this will make that suggestion; otherwise, it uses the old suggestion of making the pattern fully explicit.
Relevant tracking issue: #131414
@rustbot label A-diagnostics A-patterns A-edition-2024
r? @Nadrieril
This aligns the main error message a bit more with the phrasing in the Edition Guide and provides a bit more information on the labels to (hopefully!) aid in understanding.
Status: Awaiting review from the assignee but also interested parties.
Relevant to the compiler team, which will review and decide on the PR/issue.
labels
Some changes occurred in match checking
cc @Nadrieril
Most of these are meant to test possible future improvements, but since they cover cases the existing test suite didn't, I figure including them now may be helpful.
I've pulled in the tests from #136496. Most of them are targeting changes made in that PR, but the first two apply here too: the first tests that we do rewrite in a case that we can, and the second that we don't in a case that we shouldn't. I figure having the rest is nice too, just to be sure everything here (and in #136475) is working as intended; the existing tests didn't have much coverage involving both deep nesting and multiple bindings, and none involving bindings with subpatterns.
Following the discussion on Zulip I am going to nominate for the team compiler meeting to check the vibe about this
@rustbot label +I-compiler-nominated
Amazing work, thanks for your reactivity @dianne. I quite like the result, hopefully we can backport this so this hits stable.
@bors r+
📌 Commit f1e4d94 has been approved by Nadrieril
It is now in the queue for this repository.
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
This was referenced
Feb 7, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…fication, r=Nadrieril
Pattern Migration 2024: try to suggest eliding redundant binding modifiers
This is based on rust-lang#136475. Only the last commit is new.
This is a simpler, more restrictive alternative to rust-lang#136496, meant to partially address rust-lang#136047. If a pattern can be migrated to Rust 2024 solely by removing redundant binding modifiers, this will make that suggestion; otherwise, it uses the old suggestion of making the pattern fully explicit.
Relevant tracking issue: rust-lang#131414
@rustbot label A-diagnostics A-patterns A-edition-2024
r? @Nadrieril
bors added a commit to rust-lang-ci/rust that referenced this pull request
…iaskrgr
Rollup of 8 pull requests
Successful merges:
- rust-lang#134367 (Stabilize
feature(trait_upcasting)) - rust-lang#135354 ([Debuginfo] Add MSVC Synthetic and Summary providers to LLDB)
- rust-lang#135940 (Update toolstate maintainers)
- rust-lang#135945 (Remove some unnecessary parens in
assert!conditions) - rust-lang#136577 (Pattern Migration 2024: try to suggest eliding redundant binding modifiers)
- rust-lang#136598 (Fix suggestion for
dependency_on_unit_never_type_fallbackinvolving closures + format args expansions) - rust-lang#136653 (Remove dead code from rustc_codegen_llvm and the LLVM wrapper)
- rust-lang#136664 (replace one
.map_or(true, ...)with.is_none_or(...))
Failed merges:
- rust-lang#136193 (Implement pattern type ffi checks)
r? @ghost
@rustbot modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request
…iaskrgr
Rollup of 7 pull requests
Successful merges:
- rust-lang#134367 (Stabilize
feature(trait_upcasting)) - rust-lang#135940 (Update toolstate maintainers)
- rust-lang#135945 (Remove some unnecessary parens in
assert!conditions) - rust-lang#136577 (Pattern Migration 2024: try to suggest eliding redundant binding modifiers)
- rust-lang#136598 (Fix suggestion for
dependency_on_unit_never_type_fallbackinvolving closures + format args expansions) - rust-lang#136653 (Remove dead code from rustc_codegen_llvm and the LLVM wrapper)
- rust-lang#136664 (replace one
.map_or(true, ...)with.is_none_or(...))
r? @ghost
@rustbot modify labels: rollup
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#136577 - dianne:simple-pat-migration-simplification, r=Nadrieril
Pattern Migration 2024: try to suggest eliding redundant binding modifiers
This is based on rust-lang#136475. Only the last commit is new.
This is a simpler, more restrictive alternative to rust-lang#136496, meant to partially address rust-lang#136047. If a pattern can be migrated to Rust 2024 solely by removing redundant binding modifiers, this will make that suggestion; otherwise, it uses the old suggestion of making the pattern fully explicit.
Relevant tracking issue: rust-lang#131414
@rustbot label A-diagnostics A-patterns A-edition-2024
r? @Nadrieril
This was referenced
Feb 10, 2025
Some field data, for anyone considering the beta backport:
In a workspace with ~4000 source files, the rust-2024-incompatible-pat lint fires in 17 places. Of those, 9 are instances of the form Variant(ref x) that were being "fixed" to &Variant(ref x) but after this change are changed to Variant(x).
I find this a meaningful improvement.
Beta backport accepted as per compiler team on Zulip but we are still a bit on the fence here, see also previous discussion on Zulip.
Probably the most important thing is that if we notice something behaving funny, we should be ready with a .1 backout. And If we don't, we should do a .1 after "enough testing" with this change.
(Backport labels handled by T-release)
@rustbot label +beta-accepted
bors added a commit to rust-lang-ci/rust that referenced this pull request
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request
…tion, r=Nadrieril
Pattern Migration 2024: clean up and comment
This follows up on rust-lang#136577 by moving the pattern migration logic to its own module, removing a bit of unnecessary complexity, and adding comments. Since there's quite a bit of pattern migration logic now (and potentially more in rust-lang#136496), I think it makes sense to keep it separate from THIR construction, at least as much as is convenient.
r? @Nadrieril
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request
…tion, r=Nadrieril
Pattern Migration 2024: clean up and comment
This follows up on rust-lang#136577 by moving the pattern migration logic to its own module, removing a bit of unnecessary complexity, and adding comments. Since there's quite a bit of pattern migration logic now (and potentially more in rust-lang#136496), I think it makes sense to keep it separate from THIR construction, at least as much as is convenient.
r? @Nadrieril
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request
Rollup merge of rust-lang#136817 - dianne:clean-and-comment-pat-migration, r=Nadrieril
Pattern Migration 2024: clean up and comment
This follows up on rust-lang#136577 by moving the pattern migration logic to its own module, removing a bit of unnecessary complexity, and adding comments. Since there's quite a bit of pattern migration logic now (and potentially more in rust-lang#136496), I think it makes sense to keep it separate from THIR construction, at least as much as is convenient.
r? @Nadrieril
Labels
Area: Messages for errors, warnings, and lints
Area: The 2024 edition
Relating to patterns and pattern matching
Accepted for backporting to the compiler in the beta channel.
Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Relevant to the compiler team, which will review and decide on the PR/issue.