[前][次][番号順一覧][スレッド一覧]

ruby-changes:58937

From: Jeremy <ko1@a...>
Date: Thu, 28 Nov 2019 19:57:33 +0900 (JST)
Subject: [ruby-changes:58937] a0579f3606 (master): Make prepending a refined module after inclusion not break refinements

https://git.ruby-lang.org/ruby.git/commit/?id=a0579f3606

From a0579f3606561a74e323f6193b9504c06845236c Mon Sep 17 00:00:00 2001
From: Jeremy Evans <code@j...>
Date: Sat, 12 Oct 2019 01:02:51 -0700
Subject: 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]

diff --git a/class.c b/class.c
index e03ba22..f9e2322 100644
--- a/class.c
+++ b/class.c
@@ -890,6 +890,8 @@ add_refined_method_entry_i(ID key, VALUE value, void *data) https://github.com/ruby/ruby/blob/trunk/class.c#L890
     return ID_TABLE_CONTINUE;
 }
 
+static void ensure_origin(VALUE klass);
+
 static int
 include_modules_at(const VALUE klass, VALUE c, VALUE module, int search_super)
 {
@@ -897,6 +899,10 @@ include_modules_at(const VALUE klass, VALUE c, VALUE module, int search_super) https://github.com/ruby/ruby/blob/trunk/class.c#L899
     int method_changed = 0, constant_changed = 0;
     struct rb_id_table *const klass_m_tbl = RCLASS_M_TBL(RCLASS_ORIGIN(klass));
 
+    if (FL_TEST(module, RCLASS_REFINED_BY_ANY)) {
+        ensure_origin(module);
+    }
+
     while (module) {
 	int superclass_seen = FALSE;
 	struct rb_id_table *tbl;
@@ -978,15 +984,10 @@ move_refined_method(ID key, VALUE value, void *data) https://github.com/ruby/ruby/blob/trunk/class.c#L984
     }
 }
 
-void
-rb_prepend_module(VALUE klass, VALUE module)
+static void
+ensure_origin(VALUE klass)
 {
-    VALUE origin;
-    int changed = 0;
-
-    ensure_includable(klass, module);
-
-    origin = RCLASS_ORIGIN(klass);
+    VALUE origin = RCLASS_ORIGIN(klass);
     if (origin == klass) {
 	origin = class_alloc(T_ICLASS, klass);
 	OBJ_WB_UNPROTECT(origin); /* TODO: conservative shading. Need more survey. */
@@ -997,6 +998,16 @@ rb_prepend_module(VALUE klass, VALUE module) https://github.com/ruby/ruby/blob/trunk/class.c#L998
 	RCLASS_M_TBL_INIT(klass);
 	rb_id_table_foreach(RCLASS_M_TBL(origin), move_refined_method, (void *)klass);
     }
+}
+
+void
+rb_prepend_module(VALUE klass, VALUE module)
+{
+    VALUE origin;
+    int changed = 0;
+
+    ensure_includable(klass, module);
+    ensure_origin(klass);
     changed = include_modules_at(klass, klass, module, FALSE);
     if (changed < 0)
 	rb_raise(rb_eArgError, "cyclic prepend detected");
diff --git a/eval.c b/eval.c
index 77b0efa..7bcb544 100644
--- a/eval.c
+++ b/eval.c
@@ -1542,6 +1542,9 @@ rb_mod_refine(VALUE module, VALUE klass) https://github.com/ruby/ruby/blob/trunk/eval.c#L1542
     }
 
     ensure_class_or_module(klass);
+    if (RB_TYPE_P(klass, T_MODULE)) {
+        FL_SET(klass, RCLASS_REFINED_BY_ANY);
+    }
     CONST_ID(id_refinements, "__refinements__");
     refinements = rb_attr_get(module, id_refinements);
     if (NIL_P(refinements)) {
diff --git a/internal.h b/internal.h
index 50c129e..ca635c9 100644
--- a/internal.h
+++ b/internal.h
@@ -1081,6 +1081,7 @@ int rb_singleton_class_internal_p(VALUE sklass); https://github.com/ruby/ruby/blob/trunk/internal.h#L1081
 
 #define RCLASS_CLONED     FL_USER6
 #define RICLASS_IS_ORIGIN FL_USER5
+#define RCLASS_REFINED_BY_ANY FL_USER7
 
 static inline void
 RCLASS_SET_ORIGIN(VALUE klass, VALUE origin)
diff --git a/test/ruby/test_refinement.rb b/test/ruby/test_refinement.rb
index 618175f..6fb04de 100644
--- a/test/ruby/test_refinement.rb
+++ b/test/ruby/test_refinement.rb
@@ -2350,6 +2350,38 @@ class TestRefinement < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_refinement.rb#L2350
     assert_equal("refine_method", Bug16242::X.new.hoge)
   end
 
+  module Bug13446
+    module Enumerable
+      def sum(*args)
+        i = 0
+        args.each { |arg| i += a }
+        i
+      end
+    end
+
+    using Module.new {
+      refine Enumerable do
+        alias :orig_sum :sum
+      end
+    }
+
+    module Enumerable
+      def sum(*args)
+        orig_sum(*args)
+      end
+    end
+
+    class GenericEnumerable
+      include Enumerable
+    end
+
+    Enumerable.prepend(Module.new)
+  end
+
+  def test_prepend_refined_module
+    assert_equal(0, Bug13446::GenericEnumerable.new.sum)
+  end
+
   private
 
   def eval_using(mod, s)
-- 
cgit v0.10.2


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/

[前][次][番号順一覧][スレッド一覧]