Skip to content

Keep track of original nesting on top level references#523

Merged
vinistock merged 1 commit intomainfrom
01-26-keep_track_of_original_nesting_on_top_level_references
Jan 27, 2026
Merged

Keep track of original nesting on top level references#523
vinistock merged 1 commit intomainfrom
01-26-keep_track_of_original_nesting_on_top_level_references

Conversation

@vinistock
Copy link
Member

@vinistock vinistock commented Jan 26, 2026

We originally took a shortcut for representing top level constant references. We were simply ignoring (and effectively erasing) its nesting information. This works for almost everything, but it unfortunately falls short for the case of completion.

For example:

module Foo
  CONST = 1
  
  class ::Bar
    # Completion triggered here without a character (CTRL + space)
  end
end

class Bar
  # Completion triggered here without a character (CTRL + space)
end

In this example, both cases of Bar refers to the same class, but the lexical scope in which they are inserted in is different. The first ::Bar has access to Foo::CONST through lexical scope and the second Bar does not.

We can't support this correctly without remembering the original nesting for constant references while at the same time recording that we found a top level reference.

Implementation

I tested two approaches:

  1. Using a new enum for ParentScope. This gives us type safety as the compiler forces us to handle each case.
  2. I originally thought the new enum would consume more memory, so I also experimented with using a special NameId for a fake name <!top-level!>. To my surprise, based on mem::size_of and our benchmarks on Core, there's no memory difference with the extra enum

Because of this, I went with the new enum, so that we can have extra type safety.

@vinistock vinistock self-assigned this Jan 26, 2026
@vinistock vinistock added the bugfix A change that fixes an existing bug label Jan 26, 2026 — with Graphite App
Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@vinistock vinistock marked this pull request as ready for review January 26, 2026 20:46
@vinistock vinistock requested a review from a team as a code owner January 26, 2026 20:46
assert_eq!(
vec![
"Top", "Foo", "Qux", "AfterTop", "Bar", "Baz", "Zip", "Zap", "Zop", "Boop"
"Top", "Foo", "Qux", "Bar", "AfterTop", "Baz", "Zip", "Zap", "Zop", "Boop"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did this flip?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm honestly not sure, but both Bar and AfterTop have the exact same score, so I think it's okay.

@vinistock vinistock merged commit 62b9d75 into main Jan 27, 2026
27 checks passed
@vinistock vinistock deleted the 01-26-keep_track_of_original_nesting_on_top_level_references branch January 27, 2026 17:17
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.

2 participants