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

capture diagnostic output in queries #42513

Closed
nikomatsakis opened this issue Jun 7, 2017 · 7 comments
Closed

capture diagnostic output in queries #42513

nikomatsakis opened this issue Jun 7, 2017 · 7 comments
Labels
A-incr-comp Area: Incremental compilation C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

Right now, queries produce results, but as a side-effect they may issue diagnostics. For incremental compilation, if we are going to (e.g.) skip typeck, but still produce the same output, we really need to capture this output so that we can replay it.

Right now, to issue a diagnostic, you basically directly invoke methods on the session, such as span_err and friends. When emit() is called, these just directly dump out to stderr.

To fix this issue, we would want to record those diagnostics. There is already a diagnostics object that is created (and thus can in principle be saved) -- but we'd have to find the right place to save it (in the query tables, presumably).

We also have to figure out how to recover the tcx. The easiest way to handle this would be to check the TLS for the current tcx (using ty::tls::with()) and record it that way. We generally discourage using TLS to get the tcx except in specific scenarios, but this may fall under that category.

I'm tagging this for mentoring without instructions. There is still some amount of pending design work here that will require experimentation. Plus, once the work is done, we'll have to figure out (separately) how to actually save the errors to disk and replay them etc.

@nikomatsakis nikomatsakis added A-incr-comp Area: Incremental compilation E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 7, 2017
@nikomatsakis
Copy link
Contributor Author

cc @eddyb @michaelwoerister

@eddyb
Copy link
Member

eddyb commented Jun 7, 2017

We also have to figure out how to recover the tcx.

No need! Everything already passes tcx.sess, just make it work on tcx instead.
(If that is too cumbersome, Session could also do the tracking for the query engine)

@nikomatsakis
Copy link
Contributor Author

@eddyb well, we issue diagnostics directly on the session sometimes, when the tcx is not available yet, right? I was nervous about that. At minimum, we'd have to move the methods to some new place, so that we can be sure that all existing code uses the tcx methods when it should.

@eddyb
Copy link
Member

eddyb commented Jun 7, 2017

I am aware of nothing that has &Session but no TyCtxt when the former came from the latter.

@nikomatsakis
Copy link
Contributor Author

@eddyb the point is that if you have the methods available on sess, then people might (by accident) do tcx.sess.span_err(), so we don't want that to "skip past" the recording of side-effects.

@eddyb
Copy link
Member

eddyb commented Jun 7, 2017

Then we can have the query engine mess with the Session and manually save/restore diagnostic records.

@nikomatsakis
Copy link
Contributor Author

@eddyb yes that would be fine

@Mark-Simulacrum Mark-Simulacrum added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Jul 27, 2017
alexcrichton added a commit to alexcrichton/rust that referenced this issue Aug 24, 2017
This commit alters the `rustc::ty::maps` implementation to ensure that all
output diagnostics from the compiler are tracked for the duration of each query.
These are then intended to be replayed back the first time a cached value is
loaded, and otherwise the cache should operate the same as it does today.

Closes rust-lang#42513
bors added a commit that referenced this issue Aug 25, 2017
…rister

rustc: Capture diagnostics from all queries

This commit alters the `rustc::ty::maps` implementation to ensure that all
output diagnostics from the compiler are tracked for the duration of each query.
These are then intended to be replayed back the first time a cached value is
loaded, and otherwise the cache should operate the same as it does today.

Closes #42513
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-incr-comp Area: Incremental compilation C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-hard Call for participation: Hard difficulty. Experience needed to fix: A lot. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants