Skip to content

Pattern Migration 2024: try to suggest eliding redundant binding modifiers #136577

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 13 commits into from
Feb 7, 2025

Conversation

dianne
Copy link
Contributor

@dianne dianne commented Feb 5, 2025

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

@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. labels Feb 5, 2025
@rustbot
Copy link
Collaborator

rustbot commented Feb 5, 2025

Some changes occurred in match checking

cc @Nadrieril

@rustbot rustbot added A-diagnostics Area: Messages for errors, warnings, and lints A-edition-2024 Area: The 2024 edition A-patterns Relating to patterns and pattern matching labels Feb 5, 2025
@dianne dianne force-pushed the simple-pat-migration-simplification branch from 3da87fa to d29804c Compare February 5, 2025 13:52
@dianne dianne force-pushed the simple-pat-migration-simplification branch from d29804c to 8dcdb3e Compare February 5, 2025 17:29
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.
@dianne
Copy link
Contributor Author

dianne commented Feb 6, 2025

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.

@apiraino
Copy link
Contributor

apiraino commented Feb 6, 2025

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

@rustbot rustbot added the I-compiler-nominated Nominated for discussion during a compiler team meeting. label Feb 6, 2025
@Nadrieril
Copy link
Member

Amazing work, thanks for your reactivity @dianne. I quite like the result, hopefully we can backport this so this hits stable.

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 6, 2025

📌 Commit f1e4d94 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 Feb 6, 2025
@Nadrieril Nadrieril added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Feb 6, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 7, 2025
…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`
@apiraino apiraino removed the I-compiler-nominated Nominated for discussion during a compiler team meeting. label Feb 7, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 7, 2025
…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_fallback` involving 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 Feb 7, 2025
…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_fallback` involving 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
@bors bors merged commit 20f9e97 into rust-lang:master Feb 7, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Feb 7, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 7, 2025
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``
@eric-seppanen
Copy link

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.

@apiraino
Copy link
Contributor

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

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Feb 13, 2025
@cuviper cuviper mentioned this pull request Feb 13, 2025
@cuviper cuviper modified the milestones: 1.86.0, 1.85.0 Feb 13, 2025
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Feb 13, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 13, 2025
[beta] backports

- Pattern Migration 2024: try to suggest eliding redundant binding modifiers rust-lang#136577, rust-lang#136857
- chore: update rustc-hash 2.1.0 to 2.1.1 rust-lang#136605
- Make `AsyncFnOnce`, `AsyncFnMut`, `AsyncFn` non-`#[fundamental]` rust-lang#136724
- fix ensure_monomorphic_enough rust-lang#136839
- Revert "Stabilize `extended_varargs_abi_support`" rust-lang#136897, rust-lang#136934

r? cuviper
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Feb 16, 2025
…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 Feb 17, 2025
…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 Feb 17, 2025
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``
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-edition-2024 Area: The 2024 edition A-patterns Relating to patterns and pattern matching beta-accepted Accepted for backporting to the compiler in the beta channel. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants