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

Clean up check_consts now that new promotion pass is implemented #65839

Merged
merged 5 commits into from
Oct 27, 2019

Conversation

ecstatic-morse
Copy link
Contributor

@ecstatic-morse ecstatic-morse commented Oct 26, 2019

check_consts::resolver contained a layer of abstraction (QualifResolver) to allow the existing, eager style of qualif propagation to work with either a dataflow results cursor or by applying the transfer function directly (if dataflow was not needed e.g. for promotion). However, #63812 uses a different, lazy paradigm for checking promotability, which makes this unnecessary. This PR cleans up check_consts::validation to use FlowSensitiveResolver directly, instead of through the now obselete QualifResolver API.

Also, this contains a few commits (the first four) that address some FIXMEs in #63812 regarding code duplication. They could be split out, but I think they will be relatively noncontroversial? Notably, validation::Mode is renamed to ConstKind and used in promote_consts to denote what kind of item we are in.

This is best reviewed commit-by-commit and is low priority.

r? @eddyb

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 26, 2019
This is a relic from earlier attempts at dataflow-based const validation
that attempted to do promotion at the same time. rust-lang#63812 takes a
different approach: `IsNotPromotable` is no longer a `Qualif` and is
computed lazily instead of eagerly. As a result, there's no need for an
eager `TempPromotionResolver`, and we can use the only implementer of
`QualifResolver` directly instead of through a trait.
@eddyb
Copy link
Member

eddyb commented Oct 26, 2019

@bors r+ This is great, thanks!

@bors
Copy link
Contributor

bors commented Oct 26, 2019

📌 Commit b93cdbc has been approved by eddyb

@bors
Copy link
Contributor

bors commented Oct 26, 2019

🌲 The tree is currently closed for pull requests below priority 1000, this pull request will be tested once the tree is reopened

@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 Oct 26, 2019
Centril added a commit to Centril/rust that referenced this pull request Oct 27, 2019
…r=eddyb

Clean up `check_consts` now that new promotion pass is implemented

`check_consts::resolver` contained a layer of abstraction (`QualifResolver`) to allow the existing, eager style of qualif propagation to work with either a dataflow results cursor or by applying the transfer function directly (if dataflow was not needed e.g. for promotion). However, rust-lang#63812 uses a different, lazy paradigm for checking promotability, which makes this unnecessary. This PR cleans up `check_consts::validation` to use `FlowSensitiveResolver` directly, instead of through the now obselete `QualifResolver` API.

Also, this contains a few commits (the first four) that address some FIXMEs in rust-lang#63812 regarding code duplication. They could be split out, but I think they will be relatively noncontroversial? Notably, `validation::Mode` is renamed to `ConstKind` and used in `promote_consts` to denote what kind of item we are in.

This is best reviewed commit-by-commit and is low priority.

r? @eddyb
bors added a commit that referenced this pull request Oct 27, 2019
Rollup of 6 pull requests

Successful merges:

 - #65566 (Use heuristics to suggest assignment)
 - #65738 (Coherence should allow fundamental types to impl traits when they are local)
 - #65777 (Don't ICE for completely unexpandable `impl Trait` types)
 - #65834 (Remove lint callback from driver)
 - #65839 (Clean up `check_consts` now that new promotion pass is implemented)
 - #65855 (Add long error explaination for E0666)

Failed merges:

r? @ghost
@bors bors merged commit b93cdbc into rust-lang:master Oct 27, 2019
@ecstatic-morse ecstatic-morse deleted the promo-sanity-fixes branch October 6, 2020 01:42
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