-
Notifications
You must be signed in to change notification settings - Fork 13k
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
incr. comp.: Take spans into account for ICH #36025
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
eviction_index: usize, | ||
} | ||
|
||
impl<'tcx> CachedCodemapView<'tcx> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: Rename this to CachingCodemapView
.
☔ The latest upstream changes (presumably #36030) made this pull request unmergeable. Please resolve the merge conflicts. |
4dcf193
to
e063f41
Compare
This is ready for review now. |
pub struct StrictVersionHashVisitor<'a, 'hash: 'a, 'tcx: 'hash> { | ||
pub tcx: TyCtxt<'hash, 'tcx, 'tcx>, | ||
pub st: &'a mut SipHasher, | ||
// collect a deterministic hash of def-ids that we have seen | ||
def_path_hashes: &'a mut DefPathHashes<'hash, 'tcx>, | ||
hash_spans: bool, | ||
codemap: CachedCodemapView<'tcx>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I think it would be better if this lived one level up, next to the DefPathHashes
, so that we can cache across items.
r=me modulo comments about span hashing |
@bors r=nikomatsakis |
📌 Commit 4549b6b has been approved by |
☔ The latest upstream changes (presumably #35718) made this pull request unmergeable. Please resolve the merge conflicts. |
4549b6b
to
7310a8f
Compare
@bors r=nikomatsakis rebased. |
📌 Commit 7310a8f has been approved by |
⌛ Testing commit 7310a8f with merge 4c70c01... |
💔 Test failed - auto-win-gnu-32-opt-rustbuild |
@bors retry |
@bors r- Want to take a look at the failed test on travis first... |
@bors retry Couldn't reproduce the test failure. |
@bors r=nikomatsakis |
📌 Commit 7310a8f has been approved by |
⌛ Testing commit 7310a8f with merge 3af7da5... |
💔 Test failed - auto-win-msvc-64-cargotest |
Can reproduce, will investigate... |
r=me on the last commit |
@bors r=nikomatsakis |
📌 Commit 3057b7b has been approved by |
…atsakis incr. comp.: Take spans into account for ICH This PR makes the ICH (incr. comp. hash) take spans into account when debuginfo is enabled. A side-effect of this is that the SVH (which is based on the ICHs of all items in the crate) becomes sensitive to the tiniest change in a code base if debuginfo is enabled. Since we are not trying to model ABI compatibility via the SVH anymore (this is done via the crate disambiguator now), this should be not be a problem. Fixes #33888. Fixes #32753.
This PR makes the ICH (incr. comp. hash) take spans into account when debuginfo is enabled.
A side-effect of this is that the SVH (which is based on the ICHs of all items in the crate) becomes sensitive to the tiniest change in a code base if debuginfo is enabled. Since we are not trying to model ABI compatibility via the SVH anymore (this is done via the crate disambiguator now), this should be not be a problem.
Fixes #33888.
Fixes #32753.