ruby-changes:61329
From: Jeremy <ko1@a...>
Date: Fri, 22 May 2020 23:56:06 +0900 (JST)
Subject: [ruby-changes:61329] 8d798e7c53 (master): Revert "Fix origin iclass pointer for modules"
https://git.ruby-lang.org/ruby.git/commit/?id=8d798e7c53 From 8d798e7c531c19756f38aadd03cb801a48cbd97d Mon Sep 17 00:00:00 2001 From: Jeremy Evans <code@j...> Date: Fri, 22 May 2020 07:54:34 -0700 Subject: Revert "Fix origin iclass pointer for modules" This reverts commit c745a60634260ba2080d35af6fdeaaae86fe5193. This triggers a VM assertion. Reverting until the issue can be debugged. diff --git a/class.c b/class.c index 3cc9c59..5270f41 100644 --- a/class.c +++ b/class.c @@ -837,7 +837,7 @@ rb_include_class_new(VALUE module, VALUE super) https://github.com/ruby/ruby/blob/trunk/class.c#L837 RCLASS_M_TBL(OBJ_WB_UNPROTECT(klass)) = RCLASS_M_TBL(OBJ_WB_UNPROTECT(module)); /* TODO: unprotected? */ - RCLASS_SET_ORIGIN(klass, klass); + RCLASS_SET_ORIGIN(klass, module == RCLASS_ORIGIN(module) ? klass : RCLASS_ORIGIN(module)); if (BUILTIN_TYPE(module) == T_ICLASS) { module = RBASIC(module)->klass; } @@ -925,9 +925,8 @@ clear_module_cache_i(ID id, VALUE val, void *data) https://github.com/ruby/ruby/blob/trunk/class.c#L925 static int include_modules_at(const VALUE klass, VALUE c, VALUE module, int search_super) { - VALUE p, iclass, origin_stack = 0; - int method_changed = 0, constant_changed = 0, add_subclass; - long origin_len; + VALUE p, iclass; + int method_changed = 0, constant_changed = 0; struct rb_id_table *const klass_m_tbl = RCLASS_M_TBL(RCLASS_ORIGIN(klass)); VALUE original_klass = klass; @@ -980,24 +979,11 @@ include_modules_at(const VALUE klass, VALUE c, VALUE module, int search_super) https://github.com/ruby/ruby/blob/trunk/class.c#L979 iclass = rb_include_class_new(module, super_class); c = RCLASS_SET_SUPER(c, iclass); RCLASS_SET_INCLUDER(iclass, klass); - add_subclass = TRUE; - if (module != RCLASS_ORIGIN(module)) { - if (!origin_stack) origin_stack = rb_ary_tmp_new(2); - VALUE origin[2] = {iclass, RCLASS_ORIGIN(module)}; - rb_ary_cat(origin_stack, origin, 2); - } - else if (origin_stack && (origin_len = RARRAY_LEN(origin_stack)) > 1 && - RARRAY_AREF(origin_stack, origin_len - 1) == module) { - RCLASS_SET_ORIGIN(RARRAY_AREF(origin_stack, (origin_len -= 2)), iclass); - RICLASS_SET_ORIGIN_SHARED_MTBL(iclass); - rb_ary_resize(origin_stack, origin_len); - add_subclass = FALSE; - } { VALUE m = module; if (BUILTIN_TYPE(m) == T_ICLASS) m = RBASIC(m)->klass; - if (add_subclass) rb_module_add_to_subclasses_list(m, iclass); + rb_module_add_to_subclasses_list(m, iclass); } if (FL_TEST(klass, RMODULE_IS_REFINEMENT)) { @@ -1107,7 +1093,7 @@ rb_mod_included_modules(VALUE mod) https://github.com/ruby/ruby/blob/trunk/class.c#L1093 VALUE origin = RCLASS_ORIGIN(mod); for (p = RCLASS_SUPER(mod); p; p = RCLASS_SUPER(p)) { - if (p != origin && RCLASS_ORIGIN(p) == p && BUILTIN_TYPE(p) == T_ICLASS) { + if (p != origin && BUILTIN_TYPE(p) == T_ICLASS) { VALUE m = RBASIC(p)->klass; if (RB_TYPE_P(m, T_MODULE)) rb_ary_push(ary, m); diff --git a/gc.c b/gc.c index 9674d67..b0fa670 100644 --- a/gc.c +++ b/gc.c @@ -2815,11 +2815,9 @@ obj_free(rb_objspace_t *objspace, VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L2815 break; case T_ICLASS: /* Basically , T_ICLASS shares table with the module */ - if (FL_TEST(obj, RICLASS_IS_ORIGIN) && - !FL_TEST(obj, RICLASS_ORIGIN_SHARED_MTBL)) { - /* Method table is not shared for origin iclasses of classes */ - rb_id_table_free(RCLASS_M_TBL(obj)); - } + if (FL_TEST(obj, RICLASS_IS_ORIGIN)) { + rb_id_table_free(RCLASS_M_TBL(obj)); + } if (RCLASS_CALLABLE_M_TBL(obj) != NULL) { rb_id_table_free(RCLASS_CALLABLE_M_TBL(obj)); } @@ -3913,8 +3911,7 @@ obj_memsize_of(VALUE obj, int use_all_types) https://github.com/ruby/ruby/blob/trunk/gc.c#L3911 } break; case T_ICLASS: - if (FL_TEST(obj, RICLASS_IS_ORIGIN) && - !FL_TEST(obj, RICLASS_ORIGIN_SHARED_MTBL)) { + if (FL_TEST(obj, RICLASS_IS_ORIGIN)) { if (RCLASS_M_TBL(obj)) { size += rb_id_table_memsize(RCLASS_M_TBL(obj)); } @@ -5435,8 +5432,7 @@ gc_mark_children(rb_objspace_t *objspace, VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L5432 break; case T_ICLASS: - if (FL_TEST(obj, RICLASS_IS_ORIGIN) && - !FL_TEST(obj, RICLASS_ORIGIN_SHARED_MTBL)) { + if (FL_TEST(obj, RICLASS_IS_ORIGIN)) { mark_m_tbl(objspace, RCLASS_M_TBL(obj)); } if (RCLASS_SUPER(obj)) { @@ -8300,8 +8296,7 @@ gc_update_object_references(rb_objspace_t *objspace, VALUE obj) https://github.com/ruby/ruby/blob/trunk/gc.c#L8296 break; case T_ICLASS: - if (FL_TEST(obj, RICLASS_IS_ORIGIN) && - !FL_TEST(obj, RICLASS_ORIGIN_SHARED_MTBL)) { + if (FL_TEST(obj, RICLASS_IS_ORIGIN)) { update_m_tbl(objspace, RCLASS_M_TBL(obj)); } if (RCLASS_SUPER((VALUE)obj)) { diff --git a/internal/class.h b/internal/class.h index 7724fe4..4bebb22 100644 --- a/internal/class.h +++ b/internal/class.h @@ -95,10 +95,9 @@ typedef struct rb_classext_struct rb_classext_t; https://github.com/ruby/ruby/blob/trunk/internal/class.h#L95 #endif #define RCLASS_INCLUDER(c) (RCLASS_EXT(c)->includer) -#define RICLASS_IS_ORIGIN FL_USER5 #define RCLASS_CLONED FL_USER6 +#define RICLASS_IS_ORIGIN FL_USER5 #define RCLASS_REFINED_BY_ANY FL_USER7 -#define RICLASS_ORIGIN_SHARED_MTBL FL_USER8 /* class.c */ void rb_class_subclass_add(VALUE super, VALUE klass); @@ -121,7 +120,6 @@ VALUE rb_singleton_class_get(VALUE obj); https://github.com/ruby/ruby/blob/trunk/internal/class.h#L120 int rb_class_has_methods(VALUE c); void rb_undef_methods_from(VALUE klass, VALUE super); 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); static inline VALUE RCLASS_SET_SUPER(VALUE klass, VALUE super); static inline void RCLASS_SET_INCLUDER(VALUE iclass, VALUE klass); @@ -139,12 +137,6 @@ RCLASS_SET_ORIGIN(VALUE klass, VALUE origin) https://github.com/ruby/ruby/blob/trunk/internal/class.h#L137 } static inline void -RICLASS_SET_ORIGIN_SHARED_MTBL(VALUE iclass) -{ - FL_SET(iclass, RICLASS_ORIGIN_SHARED_MTBL); -} - -static inline void RCLASS_SET_INCLUDER(VALUE iclass, VALUE klass) { RB_OBJ_WRITE(iclass, &RCLASS_INCLUDER(iclass), klass); diff --git a/test/ruby/test_module.rb b/test/ruby/test_module.rb index 561f99f..e3cd627 100644 --- a/test/ruby/test_module.rb +++ b/test/ruby/test_module.rb @@ -479,28 +479,6 @@ class TestModule < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_module.rb#L479 assert_raise(ArgumentError) { Module.new { include } } end - def test_gc_prepend_chain - assert_separately([], <<-EOS) - 10000.times { |i| - m1 = Module.new do - def foo; end - end - m2 = Module.new do - prepend m1 - def bar; end - end - m3 = Module.new do - def baz; end - prepend m2 - end - Class.new do - prepend m3 - end - } - GC.start - EOS - end - def test_include_into_module_already_included c = Class.new{def foo; [:c] end} modules = lambda do || @@ -562,16 +540,6 @@ class TestModule < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_module.rb#L540 assert_equal([Comparable, Kernel], String.included_modules - mixins) end - def test_included_modules_with_prepend - m1 = Module.new - m2 = Module.new - m3 = Module.new - - m2.prepend m1 - m3.include m2 - assert_equal([m1, m2], m3.included_modules) - end - def test_instance_methods assert_equal([:user, :user2], User.instance_methods(false).sort) assert_equal([:user, :user2, :mixin].sort, User.instance_methods(true).sort) @@ -2075,33 +2043,6 @@ class TestModule < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_module.rb#L2043 assert_include(im, mixin, bug8025) end - def test_prepended_module_with_super_and_alias - bug16736 = '[Bug #16736]' - - a = labeled_class("A") do - def m; "A"; end - end - m = labeled_module("M") do - prepend Module.new - - def self.included(base) - base.alias_method :base_m, :m - end - - def m - super + "M" - end - - def m2 - base_m - end - end - b = labeled_class("B", a) do - include m - end - assert_equal("AM", b.new.m2, bug16736) - end - def test_prepend_super_in_alias bug7842 = '[Bug #7842]' -- cgit v0.10.2 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/