Skip to content

Commit

Permalink
Honor refinements for modules that prepend other modules
Browse files Browse the repository at this point in the history
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]
  • Loading branch information
jeremyevans authored and nobu committed Nov 28, 2019
1 parent 4325f08 commit 5069c5f
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 6 deletions.
18 changes: 12 additions & 6 deletions class.c
Expand Up @@ -825,10 +825,20 @@ VALUE
rb_include_class_new(VALUE module, VALUE super)
{
VALUE klass = class_alloc(T_ICLASS, rb_cClass);
RCLASS_SET_ORIGIN(klass, klass);

RCLASS_M_TBL(OBJ_WB_UNPROTECT(klass)) =
RCLASS_M_TBL(OBJ_WB_UNPROTECT(module)); /* TODO: unprotected? */

if (BUILTIN_TYPE(module) == T_ICLASS) {
if (module != RCLASS_ORIGIN(module)) {
RCLASS_SET_ORIGIN(klass, RCLASS_ORIGIN(module));
}
module = RBASIC(module)->klass;
}
else if (module != RCLASS_ORIGIN(module)) {
RCLASS_SET_ORIGIN(klass, RCLASS_ORIGIN(module));
}
if (!RCLASS_IV_TBL(module)) {
RCLASS_IV_TBL(module) = st_init_numtable();
}
Expand All @@ -838,9 +848,6 @@ rb_include_class_new(VALUE module, VALUE super)
RCLASS_IV_TBL(klass) = RCLASS_IV_TBL(module);
RCLASS_CONST_TBL(klass) = RCLASS_CONST_TBL(module);

RCLASS_M_TBL(OBJ_WB_UNPROTECT(klass)) =
RCLASS_M_TBL(OBJ_WB_UNPROTECT(RCLASS_ORIGIN(module))); /* TODO: unprotected? */

RCLASS_SET_SUPER(klass, super);
if (RB_TYPE_P(module, T_ICLASS)) {
RBASIC_SET_CLASS(klass, RBASIC(module)->klass);
Expand Down Expand Up @@ -894,8 +901,6 @@ include_modules_at(const VALUE klass, VALUE c, VALUE module, int search_super)
int superclass_seen = FALSE;
struct rb_id_table *tbl;

if (RCLASS_ORIGIN(module) != module)
goto skip;
if (klass_m_tbl && klass_m_tbl == RCLASS_M_TBL(module))
return -1;
/* ignore if the module included already in superclasses */
Expand Down Expand Up @@ -1091,10 +1096,11 @@ rb_mod_ancestors(VALUE mod)
VALUE p, ary = rb_ary_new();

for (p = mod; p; p = RCLASS_SUPER(p)) {
if (p != RCLASS_ORIGIN(p)) continue;
if (BUILTIN_TYPE(p) == T_ICLASS) {
rb_ary_push(ary, RBASIC(p)->klass);
}
else if (p == RCLASS_ORIGIN(p)) {
else {
rb_ary_push(ary, p);
}
}
Expand Down
28 changes: 28 additions & 0 deletions test/ruby/test_refinement.rb
Expand Up @@ -2322,6 +2322,34 @@ def test_refine_in_using
assert_equal(:ok, RefineInUsing.test)
end

class Bug16242
module OtherM
end

module M
prepend OtherM

refine M do
def refine_method
"refine_method"
end
end
using M

def hoge
refine_method
end
end

class X
include M
end
end

def test_refine_prepended_module
assert_equal("refine_method", Bug16242::X.new.hoge)
end

private

def eval_using(mod, s)
Expand Down

0 comments on commit 5069c5f

Please sign in to comment.