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.

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

Fix modules that are use prepend and are refined #2550

Merged
merged 2 commits into from Nov 28, 2019

Conversation

jeremyevans
Copy link
Contributor

@jeremyevans jeremyevans commented Oct 12, 2019

Honor refinements for modules that prepend other modules

This previously did not work, and the reason it did not work is
that:

  1. Refining a module or class that prepends other modules places
    the refinements in the class itself and not the origin iclass.

  2. Inclusion of a module that prepends other modules skips the
    module itself, including only iclasses for the prepended modules
    and the origin iclass.

Those two behaviors combined meant that the method table for the
refined methods for the included module never ends up in the
method lookup chain for the class including the module.

Fix this by not skipping the module itself when the module is
included. This requires some code rearranging in
rb_include_class_new to make sure the correct method tables and
origin settings are used for the created iclass.

As origin iclasses shouldn't be exposed to Ruby, this also
requires skipping modules that have origin iclasses in
Module#ancestors (classes that have origin iclasses were already
skipped).

Make prepending a refined module after inclusion not break refinements

After the previous commit, this was still broken. The reason it
was broken is that a refined module that hasn't been prepended to
yet keeps the refined methods in the module's method table. When
prepending, the module's method table is moved to the origin
iclass, and then the refined methods are moved from the method
table to a new method table in the module itself.

Unfortunately, that means that if a class has included the module,
prepending breaks the refinements, because when the methods are
moved from the origin iclass method table to the module method
table, they are removed from the method table from the iclass
created when the module was included earlier.

Fix this by always creating an origin class when including a
module that has any refinements, even if the refinements are
not currently used. I wasn't sure the best way to do that.
The approach I choose was to use an object flag. The flag is
set on the module when Module#refine is called, and if the
flag is present when the module is included in another module
or class, an origin iclass is created for the module.

Fixes Ruby bugs 16242 and 13446.

jeremyevans added 2 commits Oct 12, 2019
This previously did not work, and the reason it did not work is
that:

1) Refining a module or class that prepends other modules places
   the refinements in the class itself and not the origin iclass.

2) Inclusion of a module that prepends other modules skips the
   module itself, including only iclasses for the prepended modules
   and the origin iclass.

Those two behaviors combined meant that the method table for the
refined methods for the included module never ends up in the
method lookup chain for the class including the module.

Fix this by not skipping the module itself when the module is
included.  This requires some code rearranging in
rb_include_class_new to make sure the correct method tables and
origin settings are used for the created iclass.

As origin iclasses shouldn't be exposed to Ruby, this also
requires skipping modules that have origin iclasses in
Module#ancestors (classes that have origin iclasses were already
skipped).

Fixes [Bug #16242]
After the previous commit, this was still broken. The reason it
was broken is that a refined module that hasn't been prepended to
yet keeps the refined methods in the module's method table. When
prepending, the module's method table is moved to the origin
iclass, and then the refined methods are moved from the method
table to a new method table in the module itself.

Unfortunately, that means that if a class has included the module,
prepending breaks the refinements, because when the methods are
moved from the origin iclass method table to the module method
table, they are removed from the method table from the iclass
created when the module was included earlier.

Fix this by always creating an origin class when including a
module that has any refinements, even if the refinements are
not currently used.  I wasn't sure the best way to do that.
The approach I choose was to use an object flag. The flag is
set on the module when Module#refine is called, and if the
flag is present when the module is included in another module
or class, an origin iclass is created for the module.

Fixes [Bug #13446]
@jeremyevans jeremyevans force-pushed the fix-prepend-refine-modules-16242 branch from f97c71b to 48c7a14 Compare Oct 12, 2019
@nobu nobu merged commit a0579f3 into ruby:master Nov 28, 2019
11 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants