Bug #17887
closedMissed constant lookup after prepend
Description
Description
The following shows that the constant lookup from B does not find the constant in the prepended M module. I would expect this lookup to behave like "B.include M" which does print the constant from module M.
Example
module M
FOO = 'm'
end
class A
FOO = 'a'
end
class B < A
def foo
FOO
end
end
b = B.new
p b.foo
A.prepend M
p b.foo
Expected Result
"a"
"m"
Actual Result
"a"
"a"
Updated by Eregon (Benoit Daloze) over 3 years ago
In the ancestors, M
is before A
, so this seems like a bug:
B.ancestors # => [B, M, A, Object, Kernel, BasicObject]
Also note that with B.include M
, which reports the same ancestors, it works as expected:
module M
FOO = 'm'
end
class A
FOO = 'a'
end
class B < A
def foo
FOO
end
end
b = B.new
p b.foo # => "a"
B.include M # instead of A.prepend M
p b.foo # => "m"
Updated by alanwu (Alan Wu) over 3 years ago
- ruby -v changed from 3.0.0 to 2.6.x-3.0.x
I also think that the same .ancestors
should yield the same constant lookup result.
It would be harder for users to predict constant lookup otherwise.
By the way, the repro script has behaved the same since Ruby 2.2.6.
Updated by jeremyevans0 (Jeremy Evans) over 3 years ago
This is because unlike method tables, constant tables are not moved to origin classes. This behavior has been present since origin classes were added in Ruby 2.0. I submitted a pull request to work around this: https://github.com/ruby/ruby/pull/4538
Updated by Eregon (Benoit Daloze) over 3 years ago
jeremyevans0 (Jeremy Evans) wrote in #note-3:
This is because unlike method tables, constant tables are not moved to origin classes.
Do we know if this was somehow intentional or more like an oversight/incorrect optimization? I would think the latter.
This behavior has been present since origin classes were added in Ruby 2.0.
Weren't origin classes introduced at the same time than prepend
, specifically to implement prepend
?
I submitted a pull request to work around this: https://github.com/ruby/ruby/pull/4538
Thanks!
Updated by jeremyevans0 (Jeremy Evans) over 3 years ago
Eregon (Benoit Daloze) wrote in #note-4:
jeremyevans0 (Jeremy Evans) wrote in #note-3:
This is because unlike method tables, constant tables are not moved to origin classes.
Do we know if this was somehow intentional or more like an oversight/incorrect optimization? I would think the latter.
That I do not know. Most of the talk when prepend was added was about method lookup, but the same issues apply to constant lookup.
This behavior has been present since origin classes were added in Ruby 2.0.
Weren't origin classes introduced at the same time than
prepend
, specifically to implementprepend
?
Yes.
Updated by matz (Yukihiro Matsumoto) over 3 years ago
I agree it should be a bug. @nobu (Nobuyoshi Nakada), could you review @jeremyevans's patch? I'd like to see if the fix causes any compatibility issue.
Matz.
Updated by jeremyevans0 (Jeremy Evans) over 3 years ago
- Status changed from Open to Assigned
- Assignee set to nobu (Nobuyoshi Nakada)
@alanwu (Alan Wu) came up with a much simpler fix for this issue, so I closed my pull request in favor of his. @nobu (Nobuyoshi Nakada), could you please review his pull request: https://github.com/ruby/ruby/pull/4585 ?
Updated by alanwu (Alan Wu) over 3 years ago
- Status changed from Assigned to Closed
Applied in changeset git|3dd3ea092acead6179033f2c95525ffc5b8bb6ff.
Use Module#ancestors order in recursive constant lookup
Before this commit, const_get with inherit=true and constant lookup
expressions searched the ancestors of the starting point in an order
different from starting_point.ancestors
.
Items in the ancestry list introduced through prepend were searched
after searching the module they were prepended into. This oddity allowed
for situations where constant lookups gave different results even though
starting_point.ancestors
is the same.
Do the lookup in the same order as starting_point.ancestors
by
skipping classes and modules that have an origin iclass. The origin
iclass is in the super chain after the prepended modules.
Note that just like before this commit, the starting point of the
constant lookup is always the first item that we search, regardless of
the presence of any prepended modules.
[Bug #17887]