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

incremental: refactor out non-query data from tcx #44137

Closed
12 of 13 tasks
nikomatsakis opened this issue Aug 28, 2017 · 3 comments
Closed
12 of 13 tasks

incremental: refactor out non-query data from tcx #44137

nikomatsakis opened this issue Aug 28, 2017 · 3 comments
Labels
A-incr-comp Area: Incremental compilation C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Aug 28, 2017

There are various fields in the tcx of data that is not tracked for incremental compilation, or is tracked in an odd way. Here is a partial list:

In each case, the idea would be to (at least to start) encapsulate the fields so that we are using queries to access those maps. I'll try to write up some mentoring instructions for each one later on.

cc @michaelwoerister

@nikomatsakis nikomatsakis added A-incr-comp Area: Incremental compilation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 28, 2017
@petrochenkov
Copy link
Contributor

Regarding named_region_map, I wanted to move lifetime resolution to AST some time, and then store lifetime resolutions directly in HIR (like resolutions for other names/paths), including various late-bound and elided lifetimes.

arielb1 pushed a commit to arielb1/rust that referenced this issue Aug 29, 2017
…r=eddyb

rustc: Remove `specialization_cache` in favor of a query

This commit removes the `specialization_cache` field of `TyCtxt` by moving it to
a dedicated query, which it turned out was already quite easily structured to do
so!

cc rust-lang#44137
@nikomatsakis
Copy link
Contributor Author

nikomatsakis commented Aug 30, 2017

Here are some other fields that look suspicious to me:

list omitted and moved to the root of the issue

Many of these follow the pattern of "maps produced by resolve that ought to be accessed via queries and ought not to be indexed by NodeId".

alexcrichton added a commit to alexcrichton/rust that referenced this issue Aug 30, 2017
…r=eddyb

rustc: Remove `specialization_cache` in favor of a query

This commit removes the `specialization_cache` field of `TyCtxt` by moving it to
a dedicated query, which it turned out was already quite easily structured to do
so!

cc rust-lang#44137
@shepmaster shepmaster added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Sep 1, 2017
bors added a commit that referenced this issue Sep 3, 2017
rustc: Remove the `used_unsafe` field on TyCtxt

Now that lint levels are available for the entire compilation, this can be an
entirely local lint in `effect.rs`

cc #44137
@nikomatsakis nikomatsakis changed the title incremental: refactor out "constant data" from tcx incremental: refactor out non-query data from tcx Sep 5, 2017
bors added a commit that referenced this issue Sep 7, 2017
Migrate a slew of metadata methods to queries

This PR intends to make more progress on #41417, knocking off some low-hanging fruit.

Closes #44190
cc #44137
bors added a commit that referenced this issue Sep 8, 2017
Migrate a slew of metadata methods to queries

This PR intends to make more progress on #41417, knocking off some low-hanging fruit.

Closes #44190
cc #44137
@alexcrichton
Copy link
Member

The final item here is tracked by #42384, so closing in favor of that

bors added a commit that referenced this issue Oct 18, 2017
…, r=eddyb

remove or encapsulate the remaining non-query data in tcx

I wound up removing the existing cache around inhabitedness since it didn't seem to be adding much value. I reworked const rvalue promotion, but not that much (i.e., I did not split the computation into bits, as @eddyb had tossed out as a suggestion). But it's now demand driven, at least.

cc @michaelwoerister -- see the `forbid_reads` change in last commit

r? @eddyb -- since the trickiest of this PR is the work on const rvalue promotion

cc #44137
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. 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

4 participants