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

Preliminary cleanup of WitnessPat hoisting/printing #128536

Merged
merged 11 commits into from
Aug 11, 2024

Conversation

Zalathar
Copy link
Contributor

@Zalathar Zalathar commented Aug 2, 2024

Follow-up to #128430.

The eventual goal is to remove print::Pat entirely, but in the course of working towards that I made so many small improvements that it seems wise to let those be reviewed/merged on their own first.

Best reviewed commit-by-commit, most of which should be pretty simple and straightforward.

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 Aug 2, 2024
@rustbot
Copy link
Collaborator

rustbot commented Aug 2, 2024

Some changes occurred in exhaustiveness checking

cc @Nadrieril

@Zalathar Zalathar force-pushed the print-cleanup branch 4 times, most recently from 1c3fdb1 to 9969b1d Compare August 4, 2024 01:39
Copy link
Member

@Nadrieril Nadrieril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was very pleasant to review, tyvm!

compiler/rustc_pattern_analysis/src/rustc.rs Outdated Show resolved Hide resolved
compiler/rustc_pattern_analysis/src/rustc.rs Outdated Show resolved Hide resolved
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 7, 2024
@Zalathar
Copy link
Contributor Author

Zalathar commented Aug 7, 2024

(Rebased before addressing feedback; no changes yet.)

We can replace some tricky iterator-mutation code with a much simpler version
that uses `while let` to shrink a slice.

We also check whether a subpattern would be a wildcard _before_ hoisting it,
which will be very useful when trying to get rid of `print::PatKind` later.
@Zalathar
Copy link
Contributor Author

Zalathar commented Aug 7, 2024

Addressed feedback (diff).

@Nadrieril
Copy link
Member

Ty!

@bors r+

@bors
Copy link
Contributor

bors commented Aug 10, 2024

📌 Commit 482412c has been approved by Nadrieril

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 10, 2024
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 10, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 11, 2024
Preliminary cleanup of `WitnessPat` hoisting/printing

Follow-up to rust-lang#128430.

The eventual goal is to remove `print::Pat` entirely, but in the course of working towards that I made so many small improvements that it seems wise to let those be reviewed/merged on their own first.

Best reviewed commit-by-commit, most of which should be pretty simple and straightforward.

r? `@Nadrieril`
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 11, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#120314 (core: optimise Debug impl for ascii::Char)
 - rust-lang#128536 (Preliminary cleanup of `WitnessPat` hoisting/printing)
 - rust-lang#128592 (Promote aarch64-apple-darwin to Tier 1)
 - rust-lang#128762 (Use more slice patterns inside the compiler)
 - rust-lang#128875 (rm `import.used`)
 - rust-lang#128882 (make LocalWaker::will_wake consistent with Waker::will_wake)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 853255e into rust-lang:master Aug 11, 2024
6 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 11, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 11, 2024
Rollup merge of rust-lang#128536 - Zalathar:print-cleanup, r=Nadrieril

Preliminary cleanup of `WitnessPat` hoisting/printing

Follow-up to rust-lang#128430.

The eventual goal is to remove `print::Pat` entirely, but in the course of working towards that I made so many small improvements that it seems wise to let those be reviewed/merged on their own first.

Best reviewed commit-by-commit, most of which should be pretty simple and straightforward.

r? ``@Nadrieril``
@Zalathar Zalathar deleted the print-cleanup branch August 11, 2024 09:56
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 11, 2024
Remove `print::Pat` from the printing of `WitnessPat`

After the preliminary work done in rust-lang#128536, we can now get rid of `print::Pat` entirely.

- First, we introduce a variant `PatKind::Print(String)`.
- Then we incrementally remove each other variant of `PatKind`, by having the relevant code produce `PatKind::Print` instead.
- Once `PatKind::Print` is the only remaining variant, it becomes easy to remove `print::Pat` and replace it with `String`.

There is more cleanup that I have in mind, but this seemed like a natural stopping point for one PR.

r? `@Nadrieril`
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Aug 16, 2024
Remove `print::Pat` from the printing of `WitnessPat`

After the preliminary work done in rust-lang#128536, we can now get rid of `print::Pat` entirely.

- First, we introduce a variant `PatKind::Print(String)`.
- Then we incrementally remove each other variant of `PatKind`, by having the relevant code produce `PatKind::Print` instead.
- Once `PatKind::Print` is the only remaining variant, it becomes easy to remove `print::Pat` and replace it with `String`.

There is more cleanup that I have in mind, but this seemed like a natural stopping point for one PR.

r? `@Nadrieril`
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Aug 16, 2024
Remove `print::Pat` from the printing of `WitnessPat`

After the preliminary work done in rust-lang#128536, we can now get rid of `print::Pat` entirely.

- First, we introduce a variant `PatKind::Print(String)`.
- Then we incrementally remove each other variant of `PatKind`, by having the relevant code produce `PatKind::Print` instead.
- Once `PatKind::Print` is the only remaining variant, it becomes easy to remove `print::Pat` and replace it with `String`.

There is more cleanup that I have in mind, but this seemed like a natural stopping point for one PR.

r? ``@Nadrieril``
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Aug 16, 2024
Remove `print::Pat` from the printing of `WitnessPat`

After the preliminary work done in rust-lang#128536, we can now get rid of `print::Pat` entirely.

- First, we introduce a variant `PatKind::Print(String)`.
- Then we incrementally remove each other variant of `PatKind`, by having the relevant code produce `PatKind::Print` instead.
- Once `PatKind::Print` is the only remaining variant, it becomes easy to remove `print::Pat` and replace it with `String`.

There is more cleanup that I have in mind, but this seemed like a natural stopping point for one PR.

r? ```@Nadrieril```
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 16, 2024
Rollup merge of rust-lang#128965 - Zalathar:no-pat, r=Nadrieril

Remove `print::Pat` from the printing of `WitnessPat`

After the preliminary work done in rust-lang#128536, we can now get rid of `print::Pat` entirely.

- First, we introduce a variant `PatKind::Print(String)`.
- Then we incrementally remove each other variant of `PatKind`, by having the relevant code produce `PatKind::Print` instead.
- Once `PatKind::Print` is the only remaining variant, it becomes easy to remove `print::Pat` and replace it with `String`.

There is more cleanup that I have in mind, but this seemed like a natural stopping point for one PR.

r? ```@Nadrieril```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants