Skip to content

Work around a resolve bug in const prop #67631

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

Jump to bottom
Merged
merged 4 commits into from
Dec 30, 2019
Merged

Work around a resolve bug in const prop #67631

merged 4 commits into from
Dec 30, 2019

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Dec 26, 2019 •
edited
Loading

r? @wesleywiser @anp

This isn't exposed right now, but further changes to rustc may start causing bugs without this.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 26, 2019
@wesleywiser
Copy link
Member

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Dec 26, 2019

📌 Commit 652cec1 has been approved by wesleywiser

@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 Dec 26, 2019
@wesleywiser
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 26, 2019

📌 Commit 3ab0663 has been approved by wesleywiser

@anp
Copy link
Member

anp commented Dec 26, 2019

Confirmed that this will unblock #67137. Thank you!!!

@anp anp mentioned this pull request Dec 26, 2019
Merged
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Dec 26, 2019
@Mark-Simulacrum
Rollup merge of rust-lang#67631 - oli-obk:polymorphic_promotion, r=we…
eeaaee8 
…sleywiser

Work around a resolve bug in const prop

r? @wesleywiser @anp

This isn't exposed right now, but further changes to rustc may start causing bugs without this.
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Dec 26, 2019
Closed
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Dec 27, 2019
@JohnTitor
Rollup merge of rust-lang#67631 - oli-obk:polymorphic_promotion, r=we…
9f5259a 
…sleywiser

Work around a resolve bug in const prop

r? @wesleywiser @anp

This isn't exposed right now, but further changes to rustc may start causing bugs without this.
@JohnTitor JohnTitor mentioned this pull request Dec 27, 2019
Closed
@wesleywiser wesleywiser mentioned this pull request Dec 27, 2019
Merged
@wesleywiser
Copy link
Member

@bors r-

@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 Dec 27, 2019
@oli-obk
Copy link
Contributor Author

oli-obk commented Dec 27, 2019

Let's not do the hack, since @wesleywiser has a real fix

@oli-obk oli-obk closed this Dec 27, 2019
@wesleywiser
Fix Instance::resolve() incorrectly returning specialized instances
25a8b5d 
We only want to return specializations when `Reveal::All` is passed, not
when `Reveal::UserFacing` is. Resolving this fixes several issues with
the `ConstProp`, `SimplifyBranches`, and `Inline` MIR optimization
passes.

Fixes rust-lang#66901
@anp
Copy link
Member

anp commented Dec 27, 2019

Ping on #67662 (comment):

I guess we still need my PR for backwards compatibility of @anp's PR. I'll reopen

@oli-obk oli-obk reopened this Dec 27, 2019
@wesleywiser
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 27, 2019

📌 Commit 3ab0663 has been approved by wesleywiser

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 27, 2019
@oli-obk
Copy link
Contributor Author

oli-obk commented Dec 27, 2019

@bors r-

I still want to do the warning thing

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Dec 27, 2019
@oli-obk oli-obk force-pushed the polymorphic_promotion branch from 3ab0663 to 5fd8abd Compare December 28, 2019 23:26
@oli-obk
Copy link
Contributor Author

oli-obk commented Dec 28, 2019 •
edited
Loading

I added one more commit @wesleywiser

It should permit #67137 to not cause a breaking change related to diverging constants, and additionally give us new const_err lints for them.

@anp
Copy link
Member

anp commented Dec 29, 2019

Confirmed that this gets my test passing!

@wesleywiser
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 29, 2019

📌 Commit 5fd8abd has been approved by wesleywiser

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 29, 2019
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Dec 29, 2019
@Dylan-DPC
Rollup merge of rust-lang#67631 - oli-obk:polymorphic_promotion, r=we…
6dc19a3 
…sleywiser

Work around a resolve bug in const prop

r? @wesleywiser @anp

This isn't exposed right now, but further changes to rustc may start causing bugs without this.
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Dec 29, 2019
Closed
@bors
Copy link
Collaborator

bors commented Dec 30, 2019

⌛ Testing commit 5fd8abd with merge 214548b...

bors added a commit that referenced this pull request Dec 30, 2019
@bors
Auto merge of #67631 - oli-obk:polymorphic_promotion, r=wesleywiser
214548b 
Work around a resolve bug in const prop

r? @wesleywiser @anp

This isn't exposed right now, but further changes to rustc may start causing bugs without this.
@bors
Copy link
Collaborator

bors commented Dec 30, 2019

☀️ Test successful - checks-azure
Approved by: wesleywiser
Pushing 214548b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 30, 2019
@bors bors merged commit 5fd8abd into rust-lang:master Dec 30, 2019
@bors bors mentioned this pull request Dec 30, 2019
Closed
@mati865
Copy link
Contributor

mati865 commented Dec 30, 2019

Is this 1% regression (5% on ctfe) expected?
https://perf.rust-lang.org/compare.html?start=2ba0d2acbd6cb01fb07619628882120d5b66bd59&end=214548b8afd94924c17dea456c8077931009b944&stat=instructions:u

@wesleywiser
Copy link
Member

Looks like we're potentially calling const_eval_validated more now?

https://perf.rust-lang.org/detailed-query.html?commit=214548b8afd94924c17dea456c8077931009b944&base_commit=2ba0d2acbd6cb01fb07619628882120d5b66bd59&benchmark=ctfe-stress-4-check&run_name=baseline%20incremental

@oli-obk
Copy link
Contributor Author

oli-obk commented Dec 30, 2019

But only in the base incremental runs. everything else is unaffected.

@mati865
Copy link
Contributor

mati865 commented Dec 30, 2019 •
edited
Loading

patched incremental has also regressed.

@Alexendoo Alexendoo mentioned this pull request Dec 31, 2019
Closed
@oli-obk oli-obk deleted the polymorphic_promotion branch January 1, 2020 19:47
@JohnTitor JohnTitor mentioned this pull request Jan 14, 2020
Merged
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jan 15, 2020
@Dylan-DPC
Rollup merge of rust-lang#68236 - JohnTitor:ice-tests, r=Centril
2039d7e 
Add some regression tests

Closes rust-lang#64848 (fixed by rust-lang#67631)
Closes rust-lang#65918 (ICE is hidden by rust-lang#67000, no longer ICE)
Closes rust-lang#66473 (fixed by rust-lang#68084)
Closes rust-lang#67550 (set mir-opt-level to 3)

r? @Centril
@oli-obk oli-obk mentioned this pull request Feb 10, 2020
Closed
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@wesleywiser wesleywiser wesleywiser approved these changes

Assignees

@wesleywiser wesleywiser

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@oli-obk @wesleywiser @bors @anp @mati865 @rust-highfive