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.

Sign up for GitHub

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

Jump to bottom

Revert Allow destructuring opaque types in their defining scopes #99368

Closed
wants to merge 1 commit into from
Closed

Revert Allow destructuring opaque types in their defining scopes #99368

wants to merge 1 commit into from

Conversation

matthiaskrgr
Copy link
Member

@matthiaskrgr matthiaskrgr commented Jul 17, 2022 •
edited by oli-obk

reverts PR #98582

This reverts commit 6f8fb91, reversing
changes made to 7210e46.

r? @ghost

@matthiaskrgr
Revert "Rollup merge of rust-lang#98582 - oli-obk:unconstrained_opaqu…
63d3cea 
…e_type, r=estebank"

This reverts commit 6f8fb91, reversing
changes made to 7210e46.
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 17, 2022
@rustbot
Copy link
Collaborator

rustbot commented Jul 17, 2022

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@matthiaskrgr
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 17, 2022
@matthiaskrgr
Copy link
Member Author

Ouff sorry for all the pings, is there a way to make rustbot not do that? :/

@bors
Copy link
Contributor

bors commented Jul 17, 2022

⌛ Trying commit 63d3cea with merge f3aa1baa95ecd1fed574085c44f597a1de226da5...

@bors
Copy link
Contributor

bors commented Jul 17, 2022

☀️ Try build successful - checks-actions
Build commit: f3aa1baa95ecd1fed574085c44f597a1de226da5 (f3aa1baa95ecd1fed574085c44f597a1de226da5)

@rust-timer
Copy link
Collaborator

Queued f3aa1baa95ecd1fed574085c44f597a1de226da5 with parent 1cd72b7, future comparison URL.

@RalfJung RalfJung mentioned this pull request Jul 17, 2022
Open
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (f3aa1baa95ecd1fed574085c44f597a1de226da5): comparison url.

Instruction count

  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: 🎉 relevant improvements found
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 N/A N/A 0
Improvements 🎉
(primary)		 -0.6% -2.5% 123
Improvements 🎉
(secondary)		 -1.0% -5.5% 84
All 😿🎉 (primary) -0.6% -2.5% 123

Max RSS (memory usage)

Results
  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: no relevant changes found
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 N/A N/A 0
Improvements 🎉
(primary)		 -1.1% -1.3% 2
Improvements 🎉
(secondary)		 N/A N/A 0
All 😿🎉 (primary) -1.1% -1.3% 2

Cycles

Results
  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 12.9% 12.9% 1
Improvements 🎉
(primary)		 -2.1% -2.3% 6
Improvements 🎉
(secondary)		 -2.7% -3.8% 10
All 😿🎉 (primary) -2.1% -2.3% 6

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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-review -S-waiting-on-perf -perf-regression

Footnotes

  1. the arithmetic mean of the percent change ↩ ↩2 ↩3

  2. number of relevant changes ↩ ↩2 ↩3

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jul 17, 2022
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 17, 2022
Merged
@oli-obk oli-obk reopened this Jul 18, 2022
@oli-obk oli-obk changed the title [perf experiment] Revert "Rollup merge of #98582 - oli-obk:unconstrained_opaque_type, r… Revert Allow destructuring opaque types in their defining scopes Jul 18, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Jul 18, 2022

r? @oli-obk

@oli-obk
Copy link
Contributor

oli-obk commented Jul 18, 2022

@bors r+

I'll undo the revert in a new PR and figure out the perf regression there

@bors
Copy link
Contributor

bors commented Jul 18, 2022

📌 Commit 63d3cea has been approved by oli-obk

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 18, 2022
@oli-obk
Copy link
Contributor

oli-obk commented Jul 18, 2022

Looks like the regression is entirely due to is_useful taking more CPU time

@bors
Copy link
Contributor

bors commented Jul 19, 2022

☔ The latest upstream changes (presumably #99309) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 19, 2022
@oli-obk oli-obk closed this Jul 20, 2022
@oli-obk oli-obk mentioned this pull request Jul 20, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 20, 2022
@bors
Auto merge of rust-lang#99495 - oli-obk:revert_98582, r=oli-obk
d60d88f 
Revert "Rollup merge of rust-lang#98582 - oli-obk:unconstrained_opaque_type, r…

…=estebank"

This reverts commit 6f8fb91, reversing
changes made to 7210e46.

r? `@ghost`

rebase of rust-lang#99368
@oli-obk oli-obk mentioned this pull request Jul 27, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 20, 2022
@bors
Auto merge of rust-lang#99806 - oli-obk:unconstrained_opaque_type, r=…
4136b59 
…estebank

Allow patterns to constrain the hidden type of opaque types

fixes rust-lang#96572

reverts a revert as original PR was a perf regression that was fixed by reverting it: rust-lang#99368 (comment))

TODO:

* check if rust-lang#99685 is avoided
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request Sep 22, 2022
@bors
Auto merge of #99806 - oli-obk:unconstrained_opaque_type, r=estebank
0dc24ca 
Allow patterns to constrain the hidden type of opaque types

fixes #96572

reverts a revert as original PR was a perf regression that was fixed by reverting it: rust-lang/rust#99368 (comment))

TODO:

* check if rust-lang/rust#99685 is avoided
bjorn3 pushed a commit to rust-lang/rustc_codegen_cranelift that referenced this pull request Sep 25, 2022
@bors
Auto merge of #99806 - oli-obk:unconstrained_opaque_type, r=estebank
6eeefe2 
Allow patterns to constrain the hidden type of opaque types

fixes #96572

reverts a revert as original PR was a perf regression that was fixed by reverting it: rust-lang/rust#99368 (comment))

TODO:

* check if rust-lang/rust#99685 is avoided
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@oli-obk oli-obk

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@matthiaskrgr @rustbot @rust-timer @bors @oli-obk