Skip to content

Commit a0579f3

Browse files
jeremyevansnobu
authored andcommitted
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 [Bug #13446]
1 parent 5069c5f commit a0579f3

File tree

4 files changed

+55
-8
lines changed

4 files changed

+55
-8
lines changed

class.c

+19-8
Original file line numberDiff line numberDiff line change
@@ -890,13 +890,19 @@ add_refined_method_entry_i(ID key, VALUE value, void *data)
890890
return ID_TABLE_CONTINUE;
891891
}
892892

893+
static void ensure_origin(VALUE klass);
894+
893895
static int
894896
include_modules_at(const VALUE klass, VALUE c, VALUE module, int search_super)
895897
{
896898
VALUE p, iclass;
897899
int method_changed = 0, constant_changed = 0;
898900
struct rb_id_table *const klass_m_tbl = RCLASS_M_TBL(RCLASS_ORIGIN(klass));
899901

902+
if (FL_TEST(module, RCLASS_REFINED_BY_ANY)) {
903+
ensure_origin(module);
904+
}
905+
900906
while (module) {
901907
int superclass_seen = FALSE;
902908
struct rb_id_table *tbl;
@@ -978,15 +984,10 @@ move_refined_method(ID key, VALUE value, void *data)
978984
}
979985
}
980986

981-
void
982-
rb_prepend_module(VALUE klass, VALUE module)
987+
static void
988+
ensure_origin(VALUE klass)
983989
{
984-
VALUE origin;
985-
int changed = 0;
986-
987-
ensure_includable(klass, module);
988-
989-
origin = RCLASS_ORIGIN(klass);
990+
VALUE origin = RCLASS_ORIGIN(klass);
990991
if (origin == klass) {
991992
origin = class_alloc(T_ICLASS, klass);
992993
OBJ_WB_UNPROTECT(origin); /* TODO: conservative shading. Need more survey. */
@@ -997,6 +998,16 @@ rb_prepend_module(VALUE klass, VALUE module)
997998
RCLASS_M_TBL_INIT(klass);
998999
rb_id_table_foreach(RCLASS_M_TBL(origin), move_refined_method, (void *)klass);
9991000
}
1001+
}
1002+
1003+
void
1004+
rb_prepend_module(VALUE klass, VALUE module)
1005+
{
1006+
VALUE origin;
1007+
int changed = 0;
1008+
1009+
ensure_includable(klass, module);
1010+
ensure_origin(klass);
10001011
changed = include_modules_at(klass, klass, module, FALSE);
10011012
if (changed < 0)
10021013
rb_raise(rb_eArgError, "cyclic prepend detected");

eval.c

+3
Original file line numberDiff line numberDiff line change
@@ -1542,6 +1542,9 @@ rb_mod_refine(VALUE module, VALUE klass)
15421542
}
15431543

15441544
ensure_class_or_module(klass);
1545+
if (RB_TYPE_P(klass, T_MODULE)) {
1546+
FL_SET(klass, RCLASS_REFINED_BY_ANY);
1547+
}
15451548
CONST_ID(id_refinements, "__refinements__");
15461549
refinements = rb_attr_get(module, id_refinements);
15471550
if (NIL_P(refinements)) {

internal.h

+1
Original file line numberDiff line numberDiff line change
@@ -1081,6 +1081,7 @@ int rb_singleton_class_internal_p(VALUE sklass);
10811081

10821082
#define RCLASS_CLONED FL_USER6
10831083
#define RICLASS_IS_ORIGIN FL_USER5
1084+
#define RCLASS_REFINED_BY_ANY FL_USER7
10841085

10851086
static inline void
10861087
RCLASS_SET_ORIGIN(VALUE klass, VALUE origin)

test/ruby/test_refinement.rb

+32
Original file line numberDiff line numberDiff line change
@@ -2350,6 +2350,38 @@ def test_refine_prepended_module
23502350
assert_equal("refine_method", Bug16242::X.new.hoge)
23512351
end
23522352

2353+
module Bug13446
2354+
module Enumerable
2355+
def sum(*args)
2356+
i = 0
2357+
args.each { |arg| i += a }
2358+
i
2359+
end
2360+
end
2361+
2362+
using Module.new {
2363+
refine Enumerable do
2364+
alias :orig_sum :sum
2365+
end
2366+
}
2367+
2368+
module Enumerable
2369+
def sum(*args)
2370+
orig_sum(*args)
2371+
end
2372+
end
2373+
2374+
class GenericEnumerable
2375+
include Enumerable
2376+
end
2377+
2378+
Enumerable.prepend(Module.new)
2379+
end
2380+
2381+
def test_prepend_refined_module
2382+
assert_equal(0, Bug13446::GenericEnumerable.new.sum)
2383+
end
2384+
23532385
private
23542386

23552387
def eval_using(mod, s)

0 commit comments

Comments
 (0)