Fix stability of Definition entities#1253
Open
lionel- wants to merge 2 commits into
Open
Conversation
f6332d0 to
14bacca
Compare
59a2813 to
0cd9628
Compare
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
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.
Branched from #1252
Progress towards #1212
Like in ty, our
Definitions are a tracked struct with some degree of stability across edits. Unlike in ty, the definitions are not created within the semantic index, they are created outside so that we keep the index salsa-free, which allows it to be easily reused to figure out data flow dependencies in non-salsa contexts.Because the definitions are created outside the index we need to be a bit careful. This is a tracked struct, not an interned one, and that means that different Salsa queries create different instances of the definition that don't compare equal, even if they are the same definition. (See https://hackmd.io/5ddaZLKYSVC_YTTD3SzRDw?view#Tracked-Structs, thanks to @DavisVaughan for finding this doc.)
To ensure definitions have a single identity, we now funnel all definitions lookup through a single
File::definitions()query that creates a map ofDefinitionkeyed byScopeandDefId(bundled in a new structDefinitionSite).New tests ensure that definitions are not invalidated when they shift in a file or when definitions are inserted before (which changes the lookup key but not the definition ID, so downstream queries remain valid even if their key is now outdated).