Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prepend breaks module "implementer" search #2864

Closed
headius opened this issue Apr 21, 2015 · 2 comments
Closed

Prepend breaks module "implementer" search #2864

headius opened this issue Apr 21, 2015 · 2 comments
Labels
core JRuby 9000 ruby 2.0-2.2
Milestone
JRuby 9.0.0.0.pre2

Comments

@headius
Copy link
Member

headius commented Apr 21, 2015

This code should work and it doesn't. This is the cause of the Rails master "each_with_index" bug when running tests:

module A
  def foo
    p 'ok'
  end
end

class X
  include A
end

class Y < X
end

module B
end

class Y
  prepend B
end

module A
  prepend B
end

Y.new.foo

I believe the problem is that our "findImplementer" logic for module methods ends up jumping above Enumerable in the search, due to the same module appearing in the hierarchy twice (due to lazy prepends).
@headius headius added this to the 9.0.0.0.pre2 milestone Apr 21, 2015
@headius
Copy link
Member Author

headius commented Apr 21, 2015

Ok, I think I'm wrong. In monitoring a method search for the code above, we do walk the hierarchy correctly, but when it gets to the A module it only searches its direct method table. Because we've prepended something into A, that table is empty.

My original fix for this was to make IncludedModuleWrapper keep searching prepended items until it finds a non-prepended, but that didn't seem to fix it.

@headius
Copy link
Member Author

headius commented Apr 21, 2015

The problem here was that I broke my prepend-searching fix.

In 7ac2331, I fixed prepends-in-a-module by searching the module's ancestors. However I searched too deeply, going into includes above the module that normally are not searched (they are copied into target hierarchy at include time).

Then in c725c69 I fixed the search to not go so deep...but I broke it by making it start at this – the IncludedModuleWrapper – rather than at origin – the actual module. That resulted in it going back to old behavior.

The new fix, in b9473af, restores the proper starting point and terminates the search by looking for methodLocation (which indicates the module under which prepends should go). The final search covers the module itself, and all is well.

@aCandidMind aCandidMind mentioned this issue Dec 9, 2015
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
core JRuby 9000 ruby 2.0-2.2
Projects
None yet
Milestone
JRuby 9.0.0.0.pre2
Development

No branches or pull requests
1 participant
@headius