incr.comp.: Verify stability of incr. comp. hashes and clean up various other things. #45867
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ilently wrong results).
… as these make a difference for the RegionScopeTree.
…onsistency check for stored query result fingerprints.
…he existing Span/HIR hashing hack.
kennytm
added
the
S-waiting-on-review
|
@bors r+ p=1 Giving high priority because this is a big ticket item and I want @michaelwoerister unblocked. |
|
📌 Commit d01b89b has been approved by |
bors
added a commit
that referenced
this pull request
Nov 8, 2017
…tsakis incr.comp.: Verify stability of incr. comp. hashes and clean up various other things. The main contribution of this PR is that it adds the `-Z incremental-verify-ich` functionality. Normally, when the red-green tracking system determines that a certain query result has not changed, it does not re-compute the incr. comp. hash (ICH) for that query result because that hash is already known. `-Z incremental-verify-ich` tells the compiler to re-hash the query result and compare the new hash against the cached hash. This is a rather thorough way of - testing hashing implementation stability, - finding missing `[input]` annotations on `DepNodes`, and - finding missing read-edges, since both a missed read and a missing `[input]` annotation can lead to something being marked as green instead of red and thus will have a different hash than it should have. Case in point, implementing this verification logic and activating it for all `src/test/incremental` tests has revealed several such oversights, all of which are fixed in this PR. r? @nikomatsakis
|
☀️ Test successful - status-appveyor, status-travis |
bors
added a commit
that referenced
this pull request
Nov 14, 2017
…en, r=alexcrichton incr.comp.: Don't crash in DepGraph::try_mark_green() when encountering a removed input node. Fixes a small regression that was introduced in #45867. r? @nikomatsakis
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The main contribution of this PR is that it adds the
-Z incremental-verify-ichfunctionality. Normally, when the red-green tracking system determines that a certain query result has not changed, it does not re-compute the incr. comp. hash (ICH) for that query result because that hash is already known.-Z incremental-verify-ichtells the compiler to re-hash the query result and compare the new hash against the cached hash. This is a rather thorough way of[input]annotations onDepNodes, andsince both a missed read and a missing
[input]annotation can lead to something being marked as green instead of red and thus will have a different hash than it should have.Case in point, implementing this verification logic and activating it for all
src/test/incrementaltests has revealed several such oversights, all of which are fixed in this PR.r? @nikomatsakis