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

rustc_mir: hardcode pass list internally and remove premature pluggability. #45916

Merged
merged 4 commits into from
Nov 14, 2017

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Nov 10, 2017

Fixes #41712 by moving the MIR pass lists from rustc_driver to rustc_mir.
The application of the passes is done with the rustc_mir::transform::run_passes macro, which is public, as are all the passes AFAIK, and can be used to apply MIR passes outside of rustc_mir.

With the ability to override query providers through the rustc_driver (orthogonal to, and not included in this PR), custom drivers will be able to substitute the entire pass list if they want to.
EDIT: the aforementioned ability is added by #45944.

r? @nikomatsakis

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 10, 2017
@nikomatsakis
Copy link
Contributor

r? @nikomatsakis

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Nov 11, 2017

📌 Commit 9eadc36 has been approved by nikomatsakis

@bjorn3
Copy link
Member

bjorn3 commented Nov 12, 2017

Please dont do this as this would make it very hard to update holyjit for the latest nightly. Currently it just requires a rustc patch of a few lines.

@eddyb
Copy link
Member Author

eddyb commented Nov 12, 2017

@bjorn3 See this PR's description for the appropriate course of action. Being able to replace the provider for, e.g., the optimized_mir query, and then reusing the macro from rustc_mir, with a slightly modified set of passes, should allow pretty much everything.
There are no composition rules for MIR passes, nor is the idea of a MIR pass permanent, so it's not really meaningful to "just add another pass".

@bjorn3
Copy link
Member

bjorn3 commented Nov 12, 2017

I see. Should have read the description completely.

@nbp
Copy link
Contributor

nbp commented Nov 12, 2017

@bjorn3 Thanks for your concern for HolyJit. Yes this patch is going to break the ability to simply rebase a patch on top of rustc, but HolyJit is a prototype, and I have been warned and explicitly been suggested against using a plugin approach from the beginning.

I still went with the plugin approach because this is something I understand and which was close enough to what is currently documented for making syntax plugins.

@kennytm kennytm 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 12, 2017
@bors
Copy link
Contributor

bors commented Nov 14, 2017

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

@eddyb
Copy link
Member Author

eddyb commented Nov 14, 2017

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Nov 14, 2017

📌 Commit d6aa56f has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Nov 14, 2017

⌛ Testing commit d6aa56f with merge 24840da...

bors added a commit that referenced this pull request Nov 14, 2017
rustc_mir: hardcode pass list internally and remove premature pluggability.

Fixes #41712 by moving the MIR pass lists from `rustc_driver` to `rustc_mir`.
The application of the passes is done with the `rustc_mir::transform::run_passes` macro, which is public, as are all the passes AFAIK, and can be used to apply MIR passes outside of `rustc_mir`.

With the ability to override query providers through the `rustc_driver` (orthogonal to, and not included in this PR), custom drivers will be able to substitute the entire pass list if they want to.
**EDIT**: the aforementioned ability is added by #45944.

r? @nikomatsakis
@bors
Copy link
Contributor

bors commented Nov 14, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 24840da to master...

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.

6 participants