Skip to content

Re-queue reference dependents when removing a declaration#715

Merged
st0012 merged 1 commit intomainfrom
fix-requeue-reference-dependents
Apr 2, 2026
Merged

Re-queue reference dependents when removing a declaration#715
st0012 merged 1 commit intomainfrom
fix-requeue-reference-dependents

Conversation

@st0012
Copy link
Copy Markdown
Member

@st0012 st0012 commented Apr 1, 2026

Summary

Found by the incremental consistency testing tool.

When invalidate_declaration removes a declaration, it unresolves the declaration's names and cascades structurally to ChildName/NestedName dependents. But Reference dependents of those names were not re-queued — constant references from surviving files that depended on the removed name would stay permanently unresolved. Additionally, references that did get re-queued but resolved to Unresolved(None) during delete-phase resolution were terminally dropped instead of retried.

This caused singleton classes to be lost after file deletion when a .new call was nested inside a compact-notation class sharing the same parent namespace (e.g. Parent::Target.new inside class Parent::Caller).

Reproduction

# parent.rb — defines the parent module
module Parent; end

# target.rb — defines a class under Parent (compact notation)
class Parent::Target; end

# caller.rb — a sibling class that calls .new on Target
# The .new call creates an Attached singleton name <Target>,
# which is how rubydex tracks that Target has a singleton class.
class Parent::Caller
  Parent::Target.new
end

When parent.rb and target.rb are both deleted and re-added:

  1. Deleting parent.rb removes the Parent declaration
  2. The removal cascades: Parent::Target and Parent::Caller names are unresolved (ChildName dependents)
  3. Bug (fix 1): The constant reference for Parent::Target in caller.rb is a Reference dependent of the Parent::Target name — but the removal path only cascaded structural dependents (ChildName/NestedName), not Reference dependents. The reference was never re-queued.
  4. Bug (fix 2): Even when a reference IS re-queued, during the delete-phase resolve() it resolves to Unresolved(None) (parent doesn't exist yet) and gets terminally dropped instead of retried.
  5. After re-add, Parent::Target is recreated but the .new reference from caller.rb is gone — the singleton <Target> is never recreated.

Two fixes for singleton classes being lost after file deletion:

1. When invalidate_declaration removes a declaration, it unresolves the
   declaration's names and cascades structurally to ChildName/NestedName
   dependents. Reference dependents of those names were not re-queued,
   so constant references from surviving files would stay permanently
   unresolved.

2. Constant references that resolve to Unresolved(None) during
   delete-phase resolution were terminally dropped. With incremental
   invalidation this is temporary — the parent namespace may be re-added.
   Treat Unresolved(None) as Retry so references survive to the next
   resolve() call.
@st0012 st0012 force-pushed the fix-requeue-reference-dependents branch from f7c987f to 77efbe6 Compare April 1, 2026 20:58
@st0012 st0012 self-assigned this Apr 1, 2026
@st0012 st0012 added the bugfix A change that fixes an existing bug label Apr 1, 2026
@st0012 st0012 marked this pull request as ready for review April 1, 2026 21:19
@st0012 st0012 requested a review from a team as a code owner April 1, 2026 21:19
Comment thread rust/rubydex/src/resolution.rs
match self.resolve_constant_internal(*constant_ref.name_id()) {
Outcome::Retry(None) => {
// There might be dependencies we haven't figured out yet, so we need to retry
Outcome::Retry(None) | Outcome::Unresolved(None) => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is fine for now, but I think we need to review if Retry is still applicable or if we should just be re-enqueueing unresolved items.

Maybe that will simplify a lot.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The distinction is still needed in definition cases AFAICT. Just that for constant references they now should be processed the same way.

@st0012 st0012 merged commit f627ccc into main Apr 2, 2026
37 checks passed
@st0012 st0012 deleted the fix-requeue-reference-dependents branch April 2, 2026 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix A change that fixes an existing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants