Fix dep graph edges around the fulfillment cache in the tcx #31087
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
size of `DepNode` smaller and because we are not that fine-grained yet anyhow
was the major use-case, and to update the dep-graph. Other kinds of predicates are now excluded from the cache because there is no easy way to make a good dep-graph node for them, and because they are not believed to be that useful. :) Fixes rust-lang#30741. (However, the test still gives wrong result for trans, for an independent reason which is fixed in the next commit.)
|
Nice way to introduce me to depgraph! |
|
@bors r+ |
|
📌 Commit 56c73e5 has been approved by |
bors
added a commit
that referenced
this pull request
Jan 22, 2016
…ielb1 This is a fix for #30741. It simplifies dep-graph tracking for trait matching. I was experimenting with having a greater resolution here, but decided to pare back to just have one dep node for "trait resolutions on trait `Foo`", which means that adding an impl to the trait `Foo` will invalidate all fns that had to do any trait matching at all on `Foo`. This seems like a reasonable starting place. Independently, I realized I had neglected to record a dependency from trans on typeck -- this is obviously needed, since trans consumes a bunch of data structures that typeck produces (but which are not currently individually tracked) -- and because trans assumes that typeck has been done. Eventually those are going to go away and be replaced with MIR, which will be tracked, so this edge would presumably be derived automatically then, but it's an obvious enough thing to want for now. r? @arielb1 cc @michaelwoerister -- this might indirectly fix the problem you observed with the trans cache, though it'd be nice to try and craft an independent test case for that.
|
Could this be used to lint on unused trait impls? |
Not with much precision, at least not right now. We're just tracking what traits you looked at, not which impls. |
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.
This is a fix for #30741. It simplifies dep-graph tracking for trait matching. I was experimenting with having a greater resolution here, but decided to pare back to just have one dep node for "trait resolutions on trait
Foo", which means that adding an impl to the traitFoowill invalidate all fns that had to do any trait matching at all onFoo. This seems like a reasonable starting place.Independently, I realized I had neglected to record a dependency from trans on typeck -- this is obviously needed, since trans consumes a bunch of data structures that typeck produces (but which are not currently individually tracked) -- and because trans assumes that typeck has been done. Eventually those are going to go away and be replaced with MIR, which will be tracked, so this edge would presumably be derived automatically then, but it's an obvious enough thing to want for now.
r? @arielb1
cc @michaelwoerister -- this might indirectly fix the problem you observed with the trans cache, though it'd be nice to try and craft an independent test case for that.