Skip to content

incr.comp.: Implement compiler diagnostic persistence. #45472

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

Merged
merged 4 commits into from
Nov 1, 2017

Conversation

michaelwoerister
Copy link
Member

@michaelwoerister michaelwoerister commented Oct 23, 2017

This PR implements storing and loading diagnostics that the compiler generates and thus allows for emitting warnings during incremental compilation without actually re-evaluating the thing the warning originally came from. It also lays some groundwork for storing and loading type information and MIR in the incr. comp. cache.

It is still work in progress:

  • There's still some documentation to be added.
  • The way anonymous queries are handled might lead to duplicated emissions of warnings. Not sure if there is a better way or how frequent such duplication would be in practice.

Diagnostic message duplication is addressed separately in #45519.

r? @nikomatsakis

@carols10cents carols10cents added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 23, 2017
@michaelwoerister michaelwoerister changed the title [WIP] incr.comp.: Implement compiler diagnostic persistence. incr.comp.: Implement compiler diagnostic persistence. Oct 25, 2017
@michaelwoerister michaelwoerister added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 25, 2017
/// This provides access to the incr. comp. on-disk cache for query results.
/// Do not access this directly. It is only meant to be used by
/// `DepGraph::try_mark_green()` and the query infrastructure in `ty::maps`.
pub(crate) on_disk_query_result_cache: maps::OnDiskCache<'tcx>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as an aside, it feels to me like "pub(crate) visibility with scary comment" is a sign that this code is not "well aligned" in terms of module structure. We should review at some point.


// revisions: cfail1 cfail2 cfail3
// compile-flags: -Coverflow-checks=on
// must-compile-successfully
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test seems good, but I wish we had some way to guarantee that the warning was coming from the red-green code. e.g., some way to assert that the relevant query that emits the warning normally was not running. But... good enough I guess.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will only start to be really exercised once we cache pre-trans data.

@nikomatsakis
Copy link
Contributor

@bors r+

@michaelwoerister
Copy link
Member Author

@bors r=nikomatsakis

@kennytm
Copy link
Member

kennytm commented Oct 30, 2017

@bors r=nikomatsakis

Err this PR is not in bors's queue 😕

@kennytm kennytm closed this Oct 30, 2017
@kennytm kennytm reopened this Oct 30, 2017
@kennytm
Copy link
Member

kennytm commented Oct 30, 2017

@bors r=nikomatsakis

@bors
Copy link
Collaborator

bors commented Oct 30, 2017

📌 Commit 6faba5b has been approved by nikomatsakis

@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 Oct 30, 2017
@bors
Copy link
Collaborator

bors commented Oct 30, 2017

⌛ Testing commit 6faba5b with merge 3a74b1bd23da0d0fffa8f7e2a9b11b636d01450d...

@bors
Copy link
Collaborator

bors commented Oct 30, 2017

💔 Test failed - status-travis

@kennytm
Copy link
Member

kennytm commented Oct 31, 2017

MIPS TLS problem same as that of #45187 (comment) and #45529 (comment). Legit.

@kennytm kennytm 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 31, 2017
@alexcrichton
Copy link
Member

The linkage error here I hope will be unblocked with #45655

@alexcrichton
Copy link
Member

@bors: retry

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 31, 2017
@bors
Copy link
Collaborator

bors commented Nov 1, 2017

⌛ Testing commit 6faba5b with merge a3f990d...

bors added a commit that referenced this pull request Nov 1, 2017
…omatsakis

incr.comp.: Implement compiler diagnostic persistence.

This PR implements storing and loading diagnostics that the compiler generates and thus allows for emitting warnings during incremental compilation without actually re-evaluating the thing the warning originally came from. It also lays some groundwork for storing and loading type information and MIR in the incr. comp. cache.

~~It is still work in progress:~~
- ~~There's still some documentation to be added.~~
- ~~The way anonymous queries are handled might lead to duplicated emissions of warnings. Not sure if there is a better way or how frequent such duplication would be in practice.~~

Diagnostic message duplication is addressed separately in #45519.

r? @nikomatsakis
@nikomatsakis
Copy link
Contributor

@bors p=1

@bors
Copy link
Collaborator

bors commented Nov 1, 2017

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

@bors bors merged commit 6faba5b into rust-lang:master Nov 1, 2017
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