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

Const to op simplification #58511

Merged
merged 11 commits into from
Feb 24, 2019
Merged

Const to op simplification #58511

merged 11 commits into from
Feb 24, 2019

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Feb 16, 2019

r? @RalfJung

alternative to #58486

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

I love this. :)

r=me with the nit fixed.

However, there are other parts of #58486 that are still viable, right? Like, some of the simplifications in const_eval?

@RalfJung
Copy link
Member

RalfJung commented Feb 16, 2019

Hm, I thought op_to_const could be removed/simplified, but having something like that makes sense. I guess the FIXME there can go away now?

It is a bit sad that we need the may_normalized: bool argument there. I think it would be clearer to rename that to force_indirect (with the opposite meaning of may_normalize), because that's really what this is about (for statics/promoteds, we need them to be ByRef). That could make the code clearer, and we could add some assertions to make sure the OpTy is really indirect.

@RalfJung
Copy link
Member

RalfJung commented Feb 16, 2019

Awesome. I love this PR. :)

@bors r+

@bors
Copy link
Contributor

bors commented Feb 16, 2019

📌 Commit 1fe7eb0 has been approved by RalfJung

@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 Feb 16, 2019
@bors
Copy link
Contributor

bors commented Feb 16, 2019

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Feb 16, 2019

📌 Commit 1fe7eb0 has been approved by RalfJung

Centril added a commit to Centril/rust that referenced this pull request Feb 20, 2019
Centril added a commit to Centril/rust that referenced this pull request Feb 23, 2019
Centril added a commit to Centril/rust that referenced this pull request Feb 24, 2019
Centril added a commit to Centril/rust that referenced this pull request Feb 24, 2019
bors added a commit that referenced this pull request Feb 24, 2019
Rollup of 6 pull requests

Successful merges:

 - #57364 (Improve parsing diagnostic for negative supertrait bounds)
 - #58183 (Clarify guarantees for `Box` allocation)
 - #58442 (Simplify the unix `Weak` functionality)
 - #58454 (Refactor Windows stdio and remove stdin double buffering )
 - #58511 (Const to op simplification)
 - #58642 (rustdoc: support methods on primitives in intra-doc links)

Failed merges:

r? @ghost
@bors bors merged commit 1fe7eb0 into rust-lang:master Feb 24, 2019
@oli-obk oli-obk deleted the const_to_op branch March 16, 2021 12:13
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants