diff --git a/class.c b/class.c index 3cc9c59742..78667addae 100644 --- a/class.c +++ b/class.c @@ -316,26 +316,9 @@ class_init_copy_check(VALUE clone, VALUE orig) } } -/* :nodoc: */ -VALUE -rb_mod_init_copy(VALUE clone, VALUE orig) +static void +copy_tables(VALUE clone, VALUE orig) { - /* cloned flag is refer at constant inline cache - * see vm_get_const_key_cref() in vm_insnhelper.c - */ - FL_SET(clone, RCLASS_CLONED); - FL_SET(orig , RCLASS_CLONED); - - if (RB_TYPE_P(clone, T_CLASS)) { - class_init_copy_check(clone, orig); - } - if (!OBJ_INIT_COPY(clone, orig)) return clone; - if (!FL_TEST(CLASS_OF(clone), FL_SINGLETON)) { - RBASIC_SET_CLASS(clone, rb_singleton_class_clone(orig)); - rb_singleton_class_attached(RBASIC(clone)->klass, (VALUE)clone); - } - RCLASS_SET_SUPER(clone, RCLASS_SUPER(orig)); - RCLASS_EXT(clone)->allocator = RCLASS_EXT(orig)->allocator; if (RCLASS_IV_TBL(clone)) { st_free_table(RCLASS_IV_TBL(clone)); RCLASS_IV_TBL(clone) = 0; @@ -363,6 +346,28 @@ rb_mod_init_copy(VALUE clone, VALUE orig) arg.klass = clone; rb_id_table_foreach(RCLASS_CONST_TBL(orig), clone_const_i, &arg); } +} + +/* :nodoc: */ +VALUE +rb_mod_init_copy(VALUE clone, VALUE orig) +{ + /* cloned flag is refer at constant inline cache + * see vm_get_const_key_cref() in vm_insnhelper.c + */ + FL_SET(clone, RCLASS_CLONED); + FL_SET(orig , RCLASS_CLONED); + + if (RB_TYPE_P(clone, T_CLASS)) { + class_init_copy_check(clone, orig); + } + if (!OBJ_INIT_COPY(clone, orig)) return clone; + if (!FL_TEST(CLASS_OF(clone), FL_SINGLETON)) { + RBASIC_SET_CLASS(clone, rb_singleton_class_clone(orig)); + rb_singleton_class_attached(RBASIC(clone)->klass, (VALUE)clone); + } + RCLASS_EXT(clone)->allocator = RCLASS_EXT(orig)->allocator; + copy_tables(clone, orig); if (RCLASS_M_TBL(orig)) { struct clone_method_arg arg; arg.old_klass = orig; @@ -371,6 +376,75 @@ rb_mod_init_copy(VALUE clone, VALUE orig) rb_id_table_foreach(RCLASS_M_TBL(orig), clone_method_i, &arg); } + if (RCLASS_ORIGIN(orig) == orig) { + RCLASS_SET_SUPER(clone, RCLASS_SUPER(orig)); + } + else { + VALUE p = RCLASS_SUPER(orig); + VALUE orig_origin = RCLASS_ORIGIN(orig); + VALUE prev_clone_p = clone; + VALUE origin_stack = rb_ary_tmp_new(2); + VALUE origin[2]; + VALUE clone_p = 0; + long origin_len; + int add_subclass; + VALUE clone_origin; + + rb_ensure_origin(clone); + clone_origin = RCLASS_ORIGIN(clone); + + while (p && p != orig_origin) { + if (BUILTIN_TYPE(p) != T_ICLASS) { + rb_bug("non iclass between module/class and origin"); + } + clone_p = class_alloc(RBASIC(p)->flags, RBASIC(p)->klass); + RCLASS_SET_SUPER(prev_clone_p, clone_p); + prev_clone_p = clone_p; + RCLASS_M_TBL(clone_p) = RCLASS_M_TBL(p); + RCLASS_CONST_TBL(clone_p) = RCLASS_CONST_TBL(p); + RCLASS_IV_TBL(clone_p) = RCLASS_IV_TBL(p); + RCLASS_EXT(clone_p)->allocator = RCLASS_EXT(p)->allocator; + if (RB_TYPE_P(clone, T_CLASS)) { + RCLASS_SET_INCLUDER(clone_p, clone); + } + add_subclass = TRUE; + if (p != RCLASS_ORIGIN(p)) { + origin[0] = clone_p; + origin[1] = RCLASS_ORIGIN(p); + rb_ary_cat(origin_stack, origin, 2); + } + else if ((origin_len = RARRAY_LEN(origin_stack)) > 1 && + RARRAY_AREF(origin_stack, origin_len - 1) == p) { + RCLASS_SET_ORIGIN(RARRAY_AREF(origin_stack, (origin_len -= 2)), clone_p); + RICLASS_SET_ORIGIN_SHARED_MTBL(clone_p); + rb_ary_resize(origin_stack, origin_len); + add_subclass = FALSE; + } + if (add_subclass) { + rb_module_add_to_subclasses_list(RBASIC(p)->klass, clone_p); + } + p = RCLASS_SUPER(p); + } + + if (p == orig_origin) { + if (clone_p) { + RCLASS_SET_SUPER(clone_p, clone_origin); + RCLASS_SET_SUPER(clone_origin, RCLASS_SUPER(orig_origin)); + } + copy_tables(clone_origin, orig_origin); + if (RCLASS_M_TBL(orig_origin)) { + struct clone_method_arg arg; + arg.old_klass = orig; + arg.new_klass = clone; + RCLASS_M_TBL_INIT(clone_origin); + rb_id_table_foreach(RCLASS_M_TBL(orig_origin), clone_method_i, &arg); + } + } + else { + rb_bug("no origin for class that has origin"); + } + } + return clone; } @@ -912,8 +986,6 @@ add_refined_method_entry_i(ID key, VALUE value, void *data) return ID_TABLE_CONTINUE; } -static void ensure_origin(VALUE klass); - static enum rb_id_table_iterator_result clear_module_cache_i(ID id, VALUE val, void *data) { @@ -931,9 +1003,7 @@ include_modules_at(const VALUE klass, VALUE c, VALUE module, int search_super) struct rb_id_table *const klass_m_tbl = RCLASS_M_TBL(RCLASS_ORIGIN(klass)); VALUE original_klass = klass; - if (FL_TEST(module, RCLASS_REFINED_BY_ANY)) { - ensure_origin(module); - } + rb_ensure_origin(module); while (module) { int superclass_seen = FALSE; @@ -1046,8 +1116,8 @@ move_refined_method(ID key, VALUE value, void *data) } } -static void -ensure_origin(VALUE klass) +void +rb_ensure_origin(VALUE klass) { VALUE origin = RCLASS_ORIGIN(klass); if (origin == klass) { @@ -1068,7 +1138,7 @@ rb_prepend_module(VALUE klass, VALUE module) int changed = 0; ensure_includable(klass, module); - ensure_origin(klass); + rb_ensure_origin(klass); changed = include_modules_at(klass, klass, module, FALSE); if (changed < 0) rb_raise(rb_eArgError, "cyclic prepend detected"); @@ -1142,7 +1212,7 @@ rb_mod_include_p(VALUE mod, VALUE mod2) Check_Type(mod2, T_MODULE); for (p = RCLASS_SUPER(mod); p; p = RCLASS_SUPER(p)) { - if (BUILTIN_TYPE(p) == T_ICLASS) { + if (BUILTIN_TYPE(p) == T_ICLASS && !FL_TEST(p, RICLASS_IS_ORIGIN)) { if (RBASIC(p)->klass == mod2) return Qtrue; } } diff --git a/complex.c b/complex.c index 94cd505ad1..3e118677a3 100644 --- a/complex.c +++ b/complex.c @@ -2356,7 +2356,7 @@ Init_Complex(void) rb_define_global_function("Complex", nucomp_f_complex, -1); - rb_undef_methods_from(rb_cComplex, rb_mComparable); + rb_undef_methods_from(rb_cComplex, RCLASS_ORIGIN(rb_mComparable)); rb_undef_method(rb_cComplex, "%"); rb_undef_method(rb_cComplex, "div"); rb_undef_method(rb_cComplex, "divmod"); diff --git a/eval.c b/eval.c index b95ca1a49a..c44bf13ed8 100644 --- a/eval.c +++ b/eval.c @@ -1382,7 +1382,7 @@ refinement_superclass(VALUE superclass) { if (RB_TYPE_P(superclass, T_MODULE)) { /* FIXME: Should ancestors of superclass be used here? */ - return rb_include_class_new(superclass, rb_cBasicObject); + return rb_include_class_new(RCLASS_ORIGIN(superclass), rb_cBasicObject); } else { return superclass; @@ -1556,7 +1556,7 @@ rb_mod_refine(VALUE module, VALUE klass) ensure_class_or_module(klass); if (RB_TYPE_P(klass, T_MODULE)) { - FL_SET(klass, RCLASS_REFINED_BY_ANY); + rb_ensure_origin(klass); } CONST_ID(id_refinements, "__refinements__"); refinements = rb_attr_get(module, id_refinements); diff --git a/internal/class.h b/internal/class.h index 7724fe43a6..eb4e7883f6 100644 --- a/internal/class.h +++ b/internal/class.h @@ -97,7 +97,6 @@ typedef struct rb_classext_struct rb_classext_t; #define RICLASS_IS_ORIGIN FL_USER5 #define RCLASS_CLONED FL_USER6 -#define RCLASS_REFINED_BY_ANY FL_USER7 #define RICLASS_ORIGIN_SHARED_MTBL FL_USER8 /* class.c */ @@ -120,6 +119,8 @@ VALUE rb_singleton_class_clone_and_attach(VALUE obj, VALUE attach); VALUE rb_singleton_class_get(VALUE obj); int rb_class_has_methods(VALUE c); void rb_undef_methods_from(VALUE klass, VALUE super); +void rb_ensure_origin(VALUE klass); + static inline void RCLASS_SET_ORIGIN(VALUE klass, VALUE origin); static inline void RICLASS_SET_ORIGIN_SHARED_MTBL(VALUE iclass); static inline VALUE RCLASS_SUPER(VALUE klass); diff --git a/marshal.c b/marshal.c index db064c735f..88ea2916c2 100644 --- a/marshal.c +++ b/marshal.c @@ -531,10 +531,13 @@ w_extended(VALUE klass, struct dump_arg *arg, int check) klass = RCLASS_SUPER(klass); } while (BUILTIN_TYPE(klass) == T_ICLASS) { - VALUE path = rb_class_name(RBASIC(klass)->klass); - w_byte(TYPE_EXTENDED, arg); - w_unique(path, arg); - klass = RCLASS_SUPER(klass); + if (!FL_TEST(klass, RICLASS_IS_ORIGIN) || + BUILTIN_TYPE(RBASIC(klass)->klass) != T_MODULE) { + VALUE path = rb_class_name(RBASIC(klass)->klass); + w_byte(TYPE_EXTENDED, arg); + w_unique(path, arg); + } + klass = RCLASS_SUPER(klass); } } diff --git a/spec/ruby/core/module/prepend_spec.rb b/spec/ruby/core/module/prepend_spec.rb index e143f51673..1905021cf7 100644 --- a/spec/ruby/core/module/prepend_spec.rb +++ b/spec/ruby/core/module/prepend_spec.rb @@ -128,17 +128,34 @@ describe "Module#prepend" do c.dup.new.should be_kind_of(m) end - it "keeps the module in the chain when dupping an intermediate module" do - m1 = Module.new { def calc(x) x end } - m2 = Module.new { prepend(m1) } - c1 = Class.new { prepend(m2) } - m2dup = m2.dup - m2dup.ancestors.should == [m2dup,m1,m2] - c2 = Class.new { prepend(m2dup) } - c1.ancestors[0,3].should == [m1,m2,c1] - c1.new.should be_kind_of(m1) - c2.ancestors[0,4].should == [m2dup,m1,m2,c2] - c2.new.should be_kind_of(m1) + ruby_version_is '0'...'2.8' do + it "keeps the module in the chain when dupping an intermediate module" do + m1 = Module.new { def calc(x) x end } + m2 = Module.new { prepend(m1) } + c1 = Class.new { prepend(m2) } + m2dup = m2.dup + m2dup.ancestors.should == [m2dup,m1,m2] + c2 = Class.new { prepend(m2dup) } + c1.ancestors[0,3].should == [m1,m2,c1] + c1.new.should be_kind_of(m1) + c2.ancestors[0,4].should == [m2dup,m1,m2,c2] + c2.new.should be_kind_of(m1) + end + end + + ruby_version_is '2.8' do + it "uses only new module when dupping the module" do + m1 = Module.new { def calc(x) x end } + m2 = Module.new { prepend(m1) } + c1 = Class.new { prepend(m2) } + m2dup = m2.dup + m2dup.ancestors.should == [m1,m2dup] + c2 = Class.new { prepend(m2dup) } + c1.ancestors[0,3].should == [m1,m2,c1] + c1.new.should be_kind_of(m1) + c2.ancestors[0,3].should == [m1,m2dup,c2] + c2.new.should be_kind_of(m1) + end end it "depends on prepend_features to add the module" do diff --git a/test/ruby/test_enum.rb b/test/ruby/test_enum.rb index 7b647231c8..809db31c2c 100644 --- a/test/ruby/test_enum.rb +++ b/test/ruby/test_enum.rb @@ -279,6 +279,25 @@ class TestEnumerable < Test::Unit::TestCase end; end + def test_refine_Enumerable_then_include + assert_separately([], "#{<<~"end;"}\n") + module RefinementBug + refine Enumerable do + def refined_method + :rm + end + end + end + using RefinementBug + + class A + include Enumerable + end + + assert_equal(:rm, [].refined_method) + end; + end + def test_partition assert_equal([[1, 3, 1], [2, 2]], @obj.partition {|x| x % 2 == 1 }) cond = ->(x, i) { x % 2 == 1 } diff --git a/test/ruby/test_module.rb b/test/ruby/test_module.rb index 561f99fb22..d3012e59ae 100644 --- a/test/ruby/test_module.rb +++ b/test/ruby/test_module.rb @@ -479,6 +479,19 @@ class TestModule < Test::Unit::TestCase assert_raise(ArgumentError) { Module.new { include } } end + def test_prepend_works_with_duped_classes + m = Module.new + a = Class.new do + def b; 2 end + prepend m + end + a2 = a.dup.new + a.class_eval do + def b; 1 end + end + assert_equal(2, a2.b) + end + def test_gc_prepend_chain assert_separately([], <<-EOS) 10000.times { |i| @@ -501,6 +514,30 @@ class TestModule < Test::Unit::TestCase EOS end + def test_refine_module_then_include + assert_separately([], "#{<<~"end;"}\n") + module M + end + class C + include M + end + module RefinementBug + refine M do + def refined_method + :rm + end + end + end + using RefinementBug + + class A + include M + end + + assert_equal(:rm, C.new.refined_method) + end; + end + def test_include_into_module_already_included c = Class.new{def foo; [:c] end} modules = lambda do || diff --git a/vm_method.c b/vm_method.c index 150fc9d55f..0c4c26a193 100644 --- a/vm_method.c +++ b/vm_method.c @@ -694,10 +694,13 @@ rb_method_entry_make(VALUE klass, ID mid, VALUE defined_class, rb_method_visibil struct rb_id_table *mtbl; st_data_t data; int make_refined = 0; + VALUE orig_klass; if (NIL_P(klass)) { klass = rb_cObject; } + orig_klass = klass; + if (!FL_TEST(klass, FL_SINGLETON) && type != VM_METHOD_TYPE_NOTIMPLEMENTED && type != VM_METHOD_TYPE_ZSUPER) { @@ -723,6 +726,9 @@ rb_method_entry_make(VALUE klass, ID mid, VALUE defined_class, rb_method_visibil } else { klass = RCLASS_ORIGIN(klass); + if (klass != orig_klass) { + rb_clear_method_cache(orig_klass, mid); + } } mtbl = RCLASS_M_TBL(klass); @@ -799,7 +805,7 @@ rb_method_entry_make(VALUE klass, ID mid, VALUE defined_class, rb_method_visibil VM_ASSERT(me->def != NULL); /* check optimized method override by a prepended module */ - if (RB_TYPE_P(klass, T_MODULE)) { + if (RB_TYPE_P(orig_klass, T_MODULE)) { check_override_opt_method(klass, (VALUE)mid); } @@ -1191,6 +1197,9 @@ remove_method(VALUE klass, ID mid) klass, ID2SYM(mid)); } + if (klass != self) { + rb_clear_method_cache(self, mid); + } rb_clear_method_cache(klass, mid); rb_id_table_delete(RCLASS_M_TBL(klass), mid);