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

Remove print::Pat from the printing of WitnessPat #128965

Merged
merged 11 commits into from
Aug 16, 2024
Merged

Conversation

Zalathar
Copy link
Contributor

After the preliminary work done in #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

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

rustbot commented Aug 11, 2024

Some changes occurred in exhaustiveness checking

cc @Nadrieril

@Nadrieril
Copy link
Member

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 11, 2024
@bors
Copy link
Contributor

bors commented Aug 11, 2024

⌛ Trying commit fc55129 with merge 55758d8...

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`
@bors
Copy link
Contributor

bors commented Aug 11, 2024

☀️ Try build successful - checks-actions
Build commit: 55758d8 (55758d88e5dd0bac956f928d284726054881642b)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (55758d8): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 2.3%, secondary -0.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
2.3% [2.3%, 2.3%] 1
Regressions ❌
(secondary)
4.7% [4.7%, 4.7%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.1% [-5.0%, -1.3%] 2
All ❌✅ (primary) 2.3% [2.3%, 2.3%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 757.742s -> 757.892s (0.02%)
Artifact size: 339.25 MiB -> 339.27 MiB (0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 11, 2024
@Nadrieril
Copy link
Member

Looks good! Thanks for the very incremental commits.

@bors r+

@bors
Copy link
Contributor

bors commented Aug 11, 2024

📌 Commit fc55129 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 Aug 11, 2024
@Noratrieb
Copy link
Member

@bors rollup=maybe

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``
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 16, 2024
…kingjubilee

Rollup of 10 pull requests

Successful merges:

 - rust-lang#128064 (Improve docs for Waker::noop and LocalWaker::noop)
 - rust-lang#128922 (rust-analyzer: use in-tree `pattern_analysis` crate)
 - rust-lang#128965 (Remove `print::Pat` from the printing of `WitnessPat`)
 - rust-lang#128977 (Only try to modify file times of a writable file on Windows)
 - rust-lang#129018 (Migrate `rlib-format-packed-bundled-libs` and `native-link-modifier-bundle` `run-make` tests to rmake)
 - rust-lang#129037 (Port `run-make/libtest-json` and `run-make/libtest-junit` to rmake)
 - rust-lang#129078 (`ParamEnvAnd::fully_perform`: we have an `ocx`, use it)
 - rust-lang#129110 (Add a comment explaining the return type of `Ty::kind`.)
 - rust-lang#129111 (Port the `sysroot-crates-are-unstable` Python script to rmake)
 - rust-lang#129135 (crashes: more tests)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 16, 2024
…kingjubilee

Rollup of 9 pull requests

Successful merges:

 - rust-lang#128064 (Improve docs for Waker::noop and LocalWaker::noop)
 - rust-lang#128922 (rust-analyzer: use in-tree `pattern_analysis` crate)
 - rust-lang#128965 (Remove `print::Pat` from the printing of `WitnessPat`)
 - rust-lang#129018 (Migrate `rlib-format-packed-bundled-libs` and `native-link-modifier-bundle` `run-make` tests to rmake)
 - rust-lang#129037 (Port `run-make/libtest-json` and `run-make/libtest-junit` to rmake)
 - rust-lang#129078 (`ParamEnvAnd::fully_perform`: we have an `ocx`, use it)
 - rust-lang#129110 (Add a comment explaining the return type of `Ty::kind`.)
 - rust-lang#129111 (Port the `sysroot-crates-are-unstable` Python script to rmake)
 - rust-lang#129135 (crashes: more tests)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 5858329 into rust-lang:master Aug 16, 2024
7 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 16, 2024
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```
@Zalathar Zalathar deleted the no-pat branch August 16, 2024 07:48
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.

6 participants