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

Make the rustc driver and interface demand driven #56732

Merged
merged 1 commit into from
Mar 10, 2019
Merged

Make the rustc driver and interface demand driven #56732

merged 1 commit into from
Mar 10, 2019

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Dec 12, 2018

This introduces a new crate rustc_interface which is the canonical interface for creating and using the compiler. It allows you to access a Compiler type in a closure and that types have methods to run passes on demand. The interesting parts are found here (defining the queries) and here (methods to create a Compiler).

cc @rust-lang/compiler @rust-lang/dev-tools @rust-lang/rustdoc

@rust-highfive
Copy link
Collaborator

r? @estebank

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 12, 2018
@rust-highfive

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
@Dylan-DPC-zz

This comment has been minimized.

Sign in to view
@Dylan-DPC-zz Dylan-DPC-zz 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-review Status: Awaiting review from the assignee but also interested parties. labels Jan 14, 2019
@rust-highfive

This comment has been minimized.

Sign in to view
@Zoxc Zoxc force-pushed the rustc-interface branch 2 times, most recently from 6a75222 to 32b4fb5 Compare January 20, 2019 08:01
@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@oli-obk
Copy link
Contributor

oli-obk commented Jan 21, 2019

Since this will impact rls, clippy and miri quite strongly, could you open PRs against them showing how those changes will impact them?

I would assume rls and miri to be easy (since they don't change compilation and just need access to the TyCtxt to start their business), but clippy inserts new lints, which I'm not sure how to do with the new interface (looks to me like rustdoc has the same problem, but the FIXME shows that you are aware of this already).

@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
@rust-highfive

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
This was referenced Mar 10, 2019
Closed
Closed
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #56732!

Tested on commit 913ad6d.
Direct link to PR: #56732

💔 clippy-driver on windows: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk @phansch, @rust-lang/infra).
💔 clippy-driver on linux: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk @phansch, @rust-lang/infra).
💔 miri on windows: test-pass → build-fail (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra).
💔 miri on linux: test-pass → build-fail (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra).
💔 rls on windows: test-pass → build-fail (cc @nrc @Xanewok, @rust-lang/infra).
💔 rls on linux: test-pass → build-fail (cc @nrc @Xanewok, @rust-lang/infra).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Mar 10, 2019
@rust-highfive
📣 Toolstate changed by rust-lang/rust#56732!
f2b6610 
Tested on commit rust-lang/rust@913ad6d.
Direct link to PR: <rust-lang/rust#56732>

💔 clippy-driver on windows: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk @phansch, @rust-lang/infra).
💔 clippy-driver on linux: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk @phansch, @rust-lang/infra).
💔 miri on windows: test-pass → build-fail (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra).
💔 miri on linux: test-pass → build-fail (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra).
💔 rls on windows: test-pass → build-fail (cc @nrc @Xanewok, @rust-lang/infra).
💔 rls on linux: test-pass → build-fail (cc @nrc @Xanewok, @rust-lang/infra).
This was referenced Mar 10, 2019
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Closed
Merged
@Manishearth
Copy link
Member

I'm not entirely clear on how this changes the driver -- mind patching clippy's driver to work again? Use rustup-toolchain-install-master -n master -f to get a master toolchain so that you can build clippy.

I think what we need to do is invoke parsing, attach the lints, and then continue with compilation. But I'm not sure.

Overall it seems like this makes the driver API more brittle since driver consumers need to know what steps to invoke (as opposed to running the driver with some callbacks, where any change in the compilation process can still be picked up by clippy). There aren't many consumers of the driver interface, but this was supposed to be the path forward for things like clippy. Perhaps an interface which is "does the rustc compilation steps, but with callbacks" can still be exposed?

@Zoxc
Copy link
Contributor Author

Zoxc commented Mar 10, 2019

@Manishearth See rust-lang/rust-clippy#3733

@Manishearth
Copy link
Member

Oh, I didn't notice that, thanks so much!

While trying to implement this I didn't realize Callbacks existed now, I retract my objection about the driver api being more brittle 😄

Manishearth added a commit to rust-lang/rust-clippy that referenced this pull request Mar 10, 2019
@Manishearth
Rustup to rustc 1.35.0-nightly (913ad6d 2019-03-10)
332e244 
Updates clippy to work with new driver stuff from rust-lang/rust#56732

See #3733
bors added a commit to rust-lang/rust-clippy that referenced this pull request Mar 10, 2019
@bors
Auto merge of #3733 - Zoxc:rustc-interface, r=Manishearth
1a8d19a 
Use the new rustc interface

Shows the changes required to compile with rust-lang/rust#56732
Xanewok added a commit to rust-lang/rls that referenced this pull request Mar 10, 2019
@Xanewok
Adapt rustc shim code for the new rustc_interface revamp
69864d9 
Which was done as a part of rust-lang/rust#56732.
bors added a commit to rust-lang/rust-clippy that referenced this pull request Mar 10, 2019
@bors
Auto merge of #3733 - Zoxc:rustc-interface, r=oli-obk
016d92d 
Use the new rustc interface

Shows the changes required to compile with rust-lang/rust#56732
Xanewok added a commit to rust-lang/rls that referenced this pull request Mar 10, 2019
@Xanewok
Adapt rustc shim code for the new rustc_interface revamp
f2818a4 
Which was done as a part of rust-lang/rust#56732.
Xanewok added a commit to rust-lang/rls that referenced this pull request Mar 10, 2019
@Xanewok
Adapt rustc shim code for the new rustc_interface revamp
05fa70e 
Which was done as a part of rust-lang/rust#56732.
@Zoxc Zoxc deleted the rustc-interface branch March 10, 2019 22:43
bors added a commit that referenced this pull request Mar 11, 2019
@bors
Auto merge of #59073 - Xanewok:rustup-rustc-interface, r=Zoxc
de5c3c4 
Update RLS and Clippy due to #56732 (rustc_interface crate)

Closes #59060.

In addition to plain submodule bumps, this also contains update to rls-rustc. The in-tree, from the RLS monorepo, version is used instead of the crates.io one (@nrc I think we might stop publishing `rls-rustc` altogether, right? It's only there to work around passing `-Zsave-analysis` to stable `rustc` and meant to be used only by RLS, IIRC).

@Zoxc also due to how we need to access the expanded AST still from the RLS side in order to pass save analysis data in-memory, I delayed the AST drop after the `after_analysis` callback if the `-Zsave-analysis` is passed.

It'd be also good if you could take a look at the changes inside the `rls` and `rls-rustc`: rust-lang/rls@6a1b5a9...6840dd6. The `rls-rustc` is based on your [PR](rust-dev-tools/rls-rustc#11) but I also had to change some bits in the RLS itself.

r? @Zoxc / @Manishearth
@Zoxc Zoxc mentioned this pull request Mar 11, 2019
Merged
Xanewok added a commit to Xanewok/rls that referenced this pull request Mar 13, 2019
@Xanewok
Remove now-unnecessary link_args
4da16c8 
Removed in rust-lang/rust#56732.
@Xanewok Xanewok mentioned this pull request Mar 13, 2019
Merged
@nnethercote nnethercote mentioned this pull request Sep 5, 2019
Merged
@pnkfelix pnkfelix mentioned this pull request Sep 23, 2019
Closed
@Zoxc Zoxc mentioned this pull request Feb 7, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk left review comments

Assignees

@oli-obk oli-obk

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.