Repository navigation
Conversation
CRuby's constant search (rb_const_search_from) reads the starting namespace's own constant table on its first iteration, and only skips a prepended class in favour of its origin on later iterations. So for `class C; prepend P; X = 2; end`, `C::X` is C's own X even though `C.ancestors` starts with P. The same applies to every search that starts from Object: top-level references, `::X` and the module fallback. search_ancestors walked the linearization directly, so it found P::X first. It now checks the namespace's own members before walking its ancestors. Co-authored-by: Ufuk Kayserilioglu <ufuk.kayserilioglu@shopify.com>
In Ruby, `Foo::TOP` raises NameError even though Object is one of Foo's ancestors. A qualified lookup (rb_public_const_get_from) passes `exclude` to rb_const_search_from: when the first owner it finds is Object itself and the search did not start at Object, the constant is not found. Owners that come after Object, like Kernel or BasicObject, are still found, so `Foo::KERNEL_CONST` resolves. Rubydex searched the whole ancestor chain for `Foo::Bar`, so it resolved references that raise at runtime, and everything chained after them (`Foo::Bar::Baz`). Co-authored-by: Ufuk Kayserilioglu <ufuk.kayserilioglu@shopify.com>
`private_constant` is only enforced by the `::` operator: `Foo::PRIV` raises NameError, even inside `class Foo` itself, and so does `::PRIV` for a private top-level constant. Unqualified references from the lexical scope or through inheritance are allowed. Rubydex records constant visibility but resolution ignored it. Constant visibility is only known after the convergence loop, so the resolver now remembers the constant paths it resolves and, once visibilities are applied, reports each one that names a private constant with a new PrivateConstantReference warning. The references still resolve, so go-to-definition keeps working. Co-authored-by: Ufuk Kayserilioglu <ufuk.kayserilioglu@shopify.com>
A top level `include M` is `main.include`, which includes M into Object, so M's constants become reachable from every class, from modules through the Object fallback, and as `Foo::MX`. Both indexers dropped mixins without an enclosing namespace, so those references stayed unresolved. A top level `include` statement is now recorded as a reopening of Object at the call site, carrying the mixins. Only statements directly in the program body count: inside a block (`Struct.new do ... end`) `self` is unknown, top level `prepend` is a NoMethodError and top level `extend` only affects `main`. Co-authored-by: Ufuk Kayserilioglu <ufuk.kayserilioglu@shopify.com>
`X ||= value` is `defined?(X) ? X : (X = value)`: it only defines X when the full lookup for X fails. A common case is `VERSION ||= ...` in a subclass or a reopened namespace, where the existing constant is inherited or lexically visible and nothing new is defined. Both indexers now mark `||=` constant and alias definitions as CONDITIONAL. When the resolver reaches one, it first looks the name up the way `defined?` does (lexically for a bare name, through a qualified search for a constant path). If that finds a constant owned by another namespace, the definition joins that declaration and its name resolves there, instead of declaring a new constant in the current scope. If the lookup is blocked by a partial ancestor chain, the definition waits once, then falls back to the old behaviour. Known limitation: like other cross-file effects, this is decided once. Deleting the existing constant later re-declares the conditional one in its own scope, but adding it back afterwards does not undo that until the file with the `||=` is re-indexed. Co-authored-by: Ufuk Kayserilioglu <ufuk.kayserilioglu@shopify.com>
Reassigning a constant replaces its value, so after `X = A; X = B`, `X::C` is `B::C` and `module X` reopens B. A `||=` assignment only takes effect when nothing was assigned before it, so `Z = A; Z ||= B` leaves Z as A. alias_targets returned targets in definition order and resolution treats the first one as primary, so Rubydex used the first assignment. Targets are now ordered with the last plain assignment first, then earlier plain assignments, then `||=` assignments in order. All targets are still returned, so constant paths through an alias with several possible values keep trying each of them. Co-authored-by: Ufuk Kayserilioglu <ufuk.kayserilioglu@shopify.com>
Directly in a class, module or singleton class body, `self` is that namespace, so `self::X` is the same qualified lookup as `Foo::X`. Both indexers reported every `self::X` as a dynamic constant reference. `self::X` is now indexed with the enclosing namespace as its parent scope when `self` is known statically: the innermost nesting is a class, module or singleton class body, and no block encloses the reference within it. Inside a method `self` depends on the receiver, and a block can run with another `self` (instance_eval, class_eval), so those stay dynamic. Co-authored-by: Ufuk Kayserilioglu <ufuk.kayserilioglu@shopify.com>
Ruby runs the `defined?` check of `X ||= value` when the assignment executes. In
module Early
LATER ||= 1
end
LATER = 2
the top level LATER doesn't exist yet when the `||=` runs, so Early::LATER is defined. A conditional definition now only joins an existing constant if that constant has a definition in another file, or earlier in the same file. Definitions in other files may have been loaded first, so they still count.
Co-authored-by: Ufuk Kayserilioglu <ufuk.kayserilioglu@shopify.com>
The VM includes Kernel into Object before any code runs, so a module included into Object later lands in front of Kernel: `class Object; include M; end` gives `[Object, M, Kernel, BasicObject]`. Definitions are processed in URI order, and `rubydex:built-in` sorts after `file://` URIs, so the built-in `include Kernel` was applied last and Kernel ended up in front of every user module included into Object (including top level includes). Linearization now applies the mixins of built-in definitions first, without changing the order definitions are listed in. Co-authored-by: Ufuk Kayserilioglu <ufuk.kayserilioglu@shopify.com>
In `class ApplicationRecord < ApplicationRecord` inside `module GSD`, Ruby evaluates the superclass before `class` creates GSD::ApplicationRecord, so the lookup continues past it and finds ::ApplicationRecord. A class can never be its own superclass: if GSD::ApplicationRecord already existed, the definition would raise a superclass mismatch. The superclass name was the same name as the class's own name (same string, scope and nesting), so it resolved to the class being defined. Both indexers now mark a bare superclass with the class's own bare name as evaluated before the definition. That gives it a separate name, and its lookup skips the declaration the class's own name resolves to. The rest of the lookup is unchanged, so a namesake inherited by the enclosing namespace is still found. Fixes #1073 Co-authored-by: Ufuk Kayserilioglu <ufuk.kayserilioglu@shopify.com>
This branch has not been deployed
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.
This aligns Rubydex's constant resolution with CRuby in nine places. Fixes #1073. Each commit fixes one behaviour and adds tests that fail without it, for both indexer backends, plus a note in
docs/ruby-behaviors.md.The gaps came from comparing Rubydex with a formal model of CRuby's constant lookup (ported from
vm_get_ev_const,rb_const_searchandrb_const_search_from). Every case was then checked against real Ruby (4.0.9). On that differential suite, the number of probes where Rubydex and CRuby agree goes from 78 of 103 to 96 of 103. The remaining 7 are intentional; see below.Changes
rb_const_search_fromreads the starting namespace's own table first, so withclass C; prepend P; X = 2; end,C::XisC::XP::XFoo::TOPraises NameError: a qualified search stops when the first owner is Object itself, unless it started at Object. Kernel/BasicObject constants past Object are still found::TOP::enforcesprivate_constant, even inside the class (Foo::PRIVinclass Foo) and for::PRIV. NewPrivateConstantReferencewarning; the reference still resolves, so navigation keeps workinginclude Mismain.include, which includes M into Object||=constant writes as conditional definitionsVERSION ||= "2"in a subclass of a class that definesVERSIONdefines nothingChild::VERSIONX = A; X = B; X::CisB::CA::Cself::Xdirectly in namespace bodiesself::XisFoo::Xmodule GSD; class ApplicationRecord < ApplicationRecord, the superclass is evaluated before the class exists, so it is::ApplicationRecord(#1073)GSD::ApplicationRecorditselfclass Object; include M; endgives[Object, M, Kernel, BasicObject]: the VM includes Kernel before any code runs[Object, Kernel, M, BasicObject]A follow-up commit refines the
||=change: when the existing constant is only defined later in the same file, the||=still defines a new constant, because Ruby runsdefined?at that point.Design choices worth a look
includeis recorded as a synthetic reopening ofObject: aClassDefinitionat theincludecall site, carrying the mixins. This reuses the existing linearization and incremental invalidation. The trade-off is thatObjectgains a definition at that location. Only statements directly in the program body count: inside a block (Struct.new do ... end),selfis unknown. Top levelprependis a NoMethodError, and top levelextendonly affectsmain.||=: both indexers set a newDefinitionFlags::CONDITIONALon||=constant and alias definitions. Before declaring one, the resolver looks the name up the waydefined?does: lexically for a bare name, with a qualified search for a path. If that finds a constant owned by another namespace and defined earlier (or in another file), the definition joins that declaration instead of creating a new one. If the lookup is blocked by a partial ancestor chain, it waits once and then falls back to the old behaviour.PrivateConstantReferenceis aWarning, like the other resolution rules. Visibility is only known after the convergence loop, so the resolver remembers the constant paths it resolved in this run and checks them once visibilities are applied.Nameflag (before_definition) gives the superclass inclass Foo < Fooits own name instead of sharing the class's name. Its lookup skips the declaration the class's own name resolves to. The flag only changes theNameIdwhen it is set, so all existing ids are unchanged.self::Xis only resolved when no block encloses it within the namespace body. A block may beinstance_eval'd orclass_eval'd, and inside a methodselfdepends on the receiver.Known limitations and remaining differences
||=deferred to is deleted, the conditional constant is re-declared in its own scope. Adding the original back doesn't undo that until the file with the||=is re-indexed.self::Xinside a block, whereselfcannot be known statically.Testing
cargo test(1187 lib tests, including the new ones for both backends),cargo fmt --checkandcargo clippy --all-targetsare clean.bundle exec rake compileandbundle exec rake ruby_test: 411 runs, 0 failures.ruby4.0.9.Requested by Ufuk Kayserilioglu ufuk.kayserilioglu@shopify.com
AI Confidence Score: 4/5. All behaviours were verified against CRuby and covered by tests. The
||=and top levelincluderepresentations are design choices the maintainers may want to change.