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

Miscellaneous refactors around how lints and typeck interact #39434

Merged
merged 8 commits into from
Feb 4, 2017

Conversation

nikomatsakis
Copy link
Contributor

@nikomatsakis nikomatsakis commented Jan 31, 2017

This is preparation for making incr. comp. skip typeck. The main gist of is trying to rationalize the outputs from typeck that are not part of tables:

  • one bit of output is the used_trait_imports set, which becomes something we track for dependencies
  • the other big of output are various lints; we used to store these into a table on sess, but this work stores them into theTypeckTables, and then makes the lint pass consult that
    • I think it probably makes sense to handle errors similarly, eventually, but that's not necessary now

r? @eddyb

Fixes #39495

@eddyb
Copy link
Member

eddyb commented Jan 31, 2017

@bors r+

@bors
Copy link
Contributor

bors commented Jan 31, 2017

📌 Commit f3aa085 has been approved by eddyb

Otherwise they are a "hidden output"
It is pretty suspect to insert an entry twice.
The previous way was not friendly to incremental compilation. The new
plan is to compute, for each body, a set of trait imports used in that
body (slightly subtle: for a closure, we assign the trait imports to the
enclosing fn). Then we walk all bodies and union these sets. The reason
we do this is that we can save the individual sets in the incremental
state, and then recompute only those sets that are needed. Before we
were planning to save only the final union, but in that case if some
components are invalidated we have to recompute *all* of them since we
don't have enough information to "partly" invalidate a result.

In truth, this set probably ought to be part of the `TypeckTables`;
however, I opted not to do that because I don't want to have to
save/restore the entire tables in the incremental state yet (since it
contains a lot of `NodeId` references, and removing those is a
significant refactoring).
@nikomatsakis
Copy link
Contributor Author

@eddyb I reworked this branch substantially, so you probably want to take another look. In particular, I did the following

  • tightened up DepTrackingMap (e.g., preventing duplicate writes in many situations)
    • it still permits some dynamic patterns I'd prefer to forbid, but that will take more work
  • change the trait import strategy to something that will actually work for incremental
  • fixed the 'tcx thing you were complaining about

@eddyb
Copy link
Member

eddyb commented Feb 3, 2017

@nikomatsakis r=me but some of the incremental tests are failing (see Travis).

@nikomatsakis
Copy link
Contributor Author

@eddyb ah, strange, I thought they passed locally, but ... I'll figure it out.

/me grumpy.

@nikomatsakis
Copy link
Contributor Author

@bors r=eddyb

@bors
Copy link
Contributor

bors commented Feb 4, 2017

📌 Commit 2fc1586 has been approved by eddyb

@bors
Copy link
Contributor

bors commented Feb 4, 2017

⌛ Testing commit 2fc1586 with merge e4eea73...

bors added a commit that referenced this pull request Feb 4, 2017
Miscellaneous refactors around how lints and typeck interact

This is preparation for making incr. comp. skip typeck. The main gist of is trying to rationalize the outputs from typeck that are not part of tables:

- one bit of output is the `used_trait_imports` set, which becomes something we track for dependencies
- the other big of output are various lints; we used to store these into a table on sess, but this work stores them into the`TypeckTables`, and then makes the lint pass consult that
    - I think it probably makes sense to handle errors similarly, eventually, but that's not necessary now

r? @eddyb

Fixes #39495
@bors
Copy link
Contributor

bors commented Feb 4, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing e4eea73 to master...

@bors bors merged commit 2fc1586 into rust-lang:master Feb 4, 2017
@nikomatsakis nikomatsakis deleted the incr-comp-skip-typeck-2 branch April 14, 2017 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants