Skip to content

Add unresolve functions and declaration_to_names reverse index#627

Merged
st0012 merged 1 commit intomainfrom
unresolve-primitives
Mar 3, 2026
Merged

Add unresolve functions and declaration_to_names reverse index#627
st0012 merged 1 commit intomainfrom
unresolve-primitives

Conversation

@st0012
Copy link
Member

@st0012 st0012 commented Mar 2, 2026

Extracted from #589

Add methods for unwinding name resolution during incremental updates:

  • unresolve_name: converts a Resolved name back to Unresolved
  • unresolve_reference: detaches a reference from its declaration and unresolves the underlying name
  • remove_name: removes a name and cleans up reverse indices

Add declaration_to_names reverse index that tracks which names resolve to each declaration. Populated during record_resolved_name, cleaned up by the unresolve/remove primitives.

@st0012 st0012 requested a review from a team as a code owner March 2, 2026 21:51
Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good, just trying to understand if we got the right data structure

@st0012 st0012 force-pushed the unresolve-primitives branch from 3bc96fb to 4980e3b Compare March 3, 2026 21:29
}

/// Removes a name from the graph entirely.
fn remove_name(&mut self, name_id: NameId) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating this method as we'll also update name_dependents here in follow up PRs.

Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let's start without any auxiliary maps and implement the simplest version.

We can iterate on performance once we have something that works and that we fully understand

}

for ref_id in document.constant_references() {
self.unresolve_reference(*ref_id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is okay for now, but be careful with mixing the removal and the invalidation of the data.

The fact that a constant reference was removed doesn't necessarily mean that the NameRef needs to become unresolved. For example:

# file1.rb
class Foo
  # NameRef(CONST, parent_scope: None, nesting: Foo)
  CONST
end

# Delete
# file2.rb
class Foo
  # NameRef(CONST, parent_scope: None, nesting: Foo)
  CONST
end

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My plan is for this and #589 to ALWAYS unresolve CONST when a Foo definition is removed to make sure this will be correct:

class Foo                                                                                                                                                               
  include Bar  # Bar defines CONST
  CONST        # resolves to Bar::CONST via ancestor lookup                                                                                                             
end           

And then we create a follow up PR to make this and other invalidation more efficient.

Introduce `unresolve_name`, `unresolve_reference`, and `remove_name` as
building blocks for incremental invalidation. Use `unresolve_reference`
in `remove_definitions_for_document` to properly detach references from
declarations when a file is updated.
@st0012 st0012 force-pushed the unresolve-primitives branch from 4980e3b to 0ac1ae6 Compare March 3, 2026 22:47
@st0012 st0012 self-assigned this Mar 3, 2026
@st0012 st0012 merged commit 961ea96 into main Mar 3, 2026
33 checks passed
@st0012 st0012 deleted the unresolve-primitives branch March 3, 2026 23:02
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.

2 participants