Skip to content

Fix stability of Definition entities#1253

Open
lionel- wants to merge 2 commits into
oak/salsa-16-document-wire-urlfrom
oak/salsa-17-interning
Open

Fix stability of Definition entities#1253
lionel- wants to merge 2 commits into
oak/salsa-16-document-wire-urlfrom
oak/salsa-17-interning

Conversation

@lionel-
Copy link
Copy Markdown
Contributor

@lionel- lionel- commented Jun 3, 2026

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 of Definition keyed by Scope and DefId (bundled in a new struct DefinitionSite).

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).

@lionel- lionel- force-pushed the oak/salsa-16-document-wire-url branch from f6332d0 to 14bacca Compare June 4, 2026 14:19
@lionel- lionel- force-pushed the oak/salsa-17-interning branch from 59a2813 to 0cd9628 Compare June 4, 2026 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant