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

const_prop: detect and avoid catching Miri errors that require allocation #66832

Merged
merged 1 commit into from
Dec 1, 2019
Merged

const_prop: detect and avoid catching Miri errors that require allocation #66832

merged 1 commit into from
Dec 1, 2019
+38 −14

Conversation

RalfJung
Copy link
Member

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 28, 2019
@oli-obk
Copy link
Contributor

oli-obk commented Nov 28, 2019

@bors try

@bors
Copy link
Contributor

bors commented Nov 28, 2019

⌛ Trying commit 5d23518 with merge 3e6312fd10ef1f314ce953e2d049ff70f159dca6...

@oli-obk
Copy link
Contributor

oli-obk commented Nov 28, 2019

@rust-timer build 3e6312fd10ef1f314ce953e2d049ff70f159dca6

@rust-timer
Copy link
Collaborator

Queued 3e6312fd10ef1f314ce953e2d049ff70f159dca6 with parent 96ad8e5, future comparison URL.

@bors
Copy link
Contributor

bors commented Nov 28, 2019

☀️ Try build successful - checks-azure
Build commit: 3e6312fd10ef1f314ce953e2d049ff70f159dca6 (3e6312fd10ef1f314ce953e2d049ff70f159dca6)

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 3e6312fd10ef1f314ce953e2d049ff70f159dca6, comparison URL.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 28, 2019

Maybe up to 0.9% improvement, but heavier const evaluable MIR than what is tested may have shown more effect. Still, explicitly having an unsupported variant for const prop seems good

@bors r+

@bors
Copy link
Contributor

bors commented Nov 28, 2019

📌 Commit 5d23518 has been approved by oli-obk

@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 Nov 28, 2019
@RalfJung
Copy link
Member Author

I also added this in preparation for cleaning up our error types, where I will remove many of them -- and thanks to this check I should notice when I remove one that we are catching (and thus that I should not remove).

wesleywiser
wesleywiser approved these changes Nov 28, 2019
View reviewed changes
Copy link
Member

@wesleywiser wesleywiser left a comment

Choose a reason for hiding this comment

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

looks great!

@bors
Copy link
Contributor

bors commented Nov 30, 2019

⌛ Testing commit 5d23518 with merge f661f54b4c9bb73ddfa9f5c32b538e913cfcded6...

@rust-highfive

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Nov 30, 2019

💔 Test failed - checks-azure

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 30, 2019
@RalfJung
Copy link
Member Author

Chocolatey was down. @bors retry

@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 Nov 30, 2019
@bors
Copy link
Contributor

bors commented Nov 30, 2019

⌛ Testing commit 5d23518 with merge ae46fabd1584c9b56dd05cadb6776de5807af77c...

@rust-highfive

This comment has been minimized.

Sign in to view
@bors
Copy link
Contributor

bors commented Nov 30, 2019

💔 Test failed - checks-azure

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 30, 2019
@bors bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 30, 2019
@dtolnay
Copy link
Member

dtolnay commented Nov 30, 2019

Still down. https://status.chocolatey.org/issues/2019-11-29-slow-and-intermittent-connection-issues/

@Centril
Copy link
Contributor

Centril commented Nov 30, 2019

@bors retry spurious

@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 Nov 30, 2019
Centril added a commit to Centril/rust that referenced this pull request Dec 1, 2019
@Centril
Rollup merge of rust-lang#66832 - RalfJung:const-prop-no-alloc, r=oli…
d91e63b 
…-obk

const_prop: detect and avoid catching Miri errors that require allocation

r? @wesleywiser @oli-obk
@Centril Centril mentioned this pull request Dec 1, 2019
Merged
bors added a commit that referenced this pull request Dec 1, 2019
@bors
Auto merge of #66917 - Centril:rollup-xj2enik, r=Centril
4007d4e 
Rollup of 9 pull requests

Successful merges:

 - #66503 (More useful test error messages on should_panic(expected=...) mismatch)
 - #66662 (Miri: run panic-catching tests in liballoc)
 - #66679 (Improve lifetime errors with implicit trait object lifetimes)
 - #66726 (Use recursion_limit for const eval stack limit)
 - #66790 (Do `min_const_fn` checks for `SetDiscriminant`s target)
 - #66832 (const_prop: detect and avoid catching Miri errors that require allocation)
 - #66880 (Add long error code explanation message for E0203)
 - #66890 (Format liballoc with rustfmt)
 - #66896 (pass Queries to compiler callbacks)

Failed merges:

r? @ghost
@bors bors merged commit 5d23518 into rust-lang:master Dec 1, 2019
@RalfJung RalfJung deleted the const-prop-no-alloc branch December 1, 2019 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wesleywiser wesleywiser wesleywiser approved these changes

Assignees

@wesleywiser wesleywiser

Labels
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.

None yet
8 participants
@RalfJung @oli-obk @bors @rust-timer @rust-highfive @dtolnay @Centril @wesleywiser