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

ruby-changes:64894

From: Jeremy <ko1@a...>
Date: Fri, 15 Jan 2021 13:43:59 +0900 (JST)
Subject: [ruby-changes:64894] e09094546a (master): Make Module#prepend affect ancestor chain even if argument already included in receiver

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

From e09094546a19d6b62b3e21d0b061b103cf21f760 Mon Sep 17 00:00:00 2001
From: Jeremy Evans <code@j...>
Date: Thu, 14 Jan 2021 11:59:25 -0800
Subject: Make Module#prepend affect ancestor chain even if argument already
 included in receiver

Previously, if a class included a module and then prepended the
same module, the prepend had no effect.  This changes the behavior
so that the prepend has an effect unless the module is already
prepended the receiver.

While here, rename the origin_seen variable in include_modules_at,
since it is misleading. The variable tracks whether c has been seen,
not whether the origin of klass has been.

Fixes [Bug #17423]

diff --git a/NEWS.md b/NEWS.md
index 1e3202c..962a777 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -21,6 +21,13 @@ Outstanding ones only. https://github.com/ruby/ruby/blob/trunk/NEWS.md#L21
 
     * Enumerator::Lazy#compact is added. [[Feature #17312]]
 
+* Module
+
+    * Module#prepend now modifies the ancestor chain if the receiver
+      already includes the argument. Module#prepend still does not
+      modify the ancestor chain if the receiver has already prepended
+      the argument. [[Bug #17423]]
+
 ## Stdlib updates
 
 Outstanding ones only.
@@ -49,3 +56,4 @@ Excluding feature bug fixes. https://github.com/ruby/ruby/blob/trunk/NEWS.md#L56
 
 
 [Feature #17312]: https://bugs.ruby-lang.org/issues/17312
+[Bug #17423]: https://bugs.ruby-lang.org/issues/17423
diff --git a/class.c b/class.c
index 47f35b1..a62ae66 100644
--- a/class.c
+++ b/class.c
@@ -1010,36 +1010,43 @@ include_modules_at(const VALUE klass, VALUE c, VALUE module, int search_super) https://github.com/ruby/ruby/blob/trunk/class.c#L1010
     VALUE p, iclass, origin_stack = 0;
     int method_changed = 0, constant_changed = 0, add_subclass;
     long origin_len;
-    struct rb_id_table *const klass_m_tbl = RCLASS_M_TBL(RCLASS_ORIGIN(klass));
+    VALUE klass_origin = RCLASS_ORIGIN(klass);
+    struct rb_id_table *const klass_m_tbl = RCLASS_M_TBL(klass_origin);
     VALUE original_klass = klass;
 
     while (module) {
-        int origin_seen = FALSE;
+        int c_seen = FALSE;
 	int superclass_seen = FALSE;
 	struct rb_id_table *tbl;
 
-        if (klass == c)
-            origin_seen = TRUE;
+        if (klass == c) {
+            c_seen = TRUE;
+        }
 	if (klass_m_tbl && klass_m_tbl == RCLASS_M_TBL(module))
 	    return -1;
-	/* ignore if the module included already in superclasses */
-        for (p = RCLASS_SUPER(klass); p; p = RCLASS_SUPER(p)) {
-	    int type = BUILTIN_TYPE(p);
-            if (c == p)
-                origin_seen = TRUE;
-	    if (type == T_ICLASS) {
-		if (RCLASS_M_TBL(p) == RCLASS_M_TBL(module)) {
-                    if (!superclass_seen && origin_seen) {
-			c = p;  /* move insertion point */
-		    }
-		    goto skip;
-		}
-	    }
-	    else if (type == T_CLASS) {
-		if (!search_super) break;
-                superclass_seen = TRUE;
-	    }
-	}
+        if (klass_origin != c || search_super) {
+            /* ignore if the module included already in superclasses for include,
+             * ignore if the module included before origin class for prepend
+             */
+            for (p = RCLASS_SUPER(klass); p; p = RCLASS_SUPER(p)) {
+                int type = BUILTIN_TYPE(p);
+                if (klass_origin == p && !search_super)
+                    break;
+                if (c == p)
+                    c_seen = TRUE;
+                if (type == T_ICLASS) {
+                    if (RCLASS_M_TBL(p) == RCLASS_M_TBL(module)) {
+                        if (!superclass_seen && c_seen) {
+                            c = p;  /* move insertion point */
+                        }
+                        goto skip;
+                    }
+                }
+                else if (type == T_CLASS) {
+                    superclass_seen = TRUE;
+                }
+            }
+        }
 
         VALUE super_class = RCLASS_SUPER(c);
 
diff --git a/test/ruby/test_module.rb b/test/ruby/test_module.rb
index fac3131..6b53b7d 100644
--- a/test/ruby/test_module.rb
+++ b/test/ruby/test_module.rb
@@ -641,6 +641,41 @@ class TestModule < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_module.rb#L641
     assert_equal([:p, :a, :s, :q, :r, :c], a.new.m)
   end
 
+  def test_prepend_after_include
+    c = Class.new{def m; [:c] end}
+    sc = Class.new(c){def m; [:sc] + super end}
+    m = Module.new{def m; [:m] + super end}
+    sc.include m
+    sc.prepend m
+    sc.prepend m
+    assert_equal([:m, :sc, :m, :c], sc.new.m)
+
+    c = Class.new{def m; [:c] end}
+    sc = Class.new(c){def m; [:sc] + super end}
+    m0 = Module.new{def m; [:m0] + super end}
+    m1 = Module.new{def m; [:m1] + super end}
+    m1.prepend m0
+    sc.include m1
+    sc.prepend m1
+    assert_equal([:m0, :m1, :sc, :m0, :m1, :c], sc.new.m)
+    sc.prepend m
+    assert_equal([:m, :m0, :m1, :sc, :m0, :m1, :c], sc.new.m)
+    sc.prepend m1
+    assert_equal([:m, :m0, :m1, :sc, :m0, :m1, :c], sc.new.m)
+
+
+    c = Class.new{def m; [:c] end}
+    sc = Class.new(c){def m; [:sc] + super end}
+    m0 = Module.new{def m; [:m0] + super end}
+    m1 = Module.new{def m; [:m1] + super end}
+    m1.include m0
+    sc.include m1
+    sc.prepend m
+    sc.prepend m1
+    sc.prepend m1
+    assert_equal([:m1, :m0, :m, :sc, :m1, :m0, :c], sc.new.m)
+  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)
@@ -2158,7 +2193,7 @@ class TestModule < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_module.rb#L2193
     assert_equal([:c2, :m0, :m1, :m2, :c0], c2.new.x)
 
     m3 = labeled_module("m3") {include m1; prepend m1}
-    assert_equal([m3, m0, m1], m3.ancestors)
+    assert_equal([m0, m1, m3, m0, m1], m3.ancestors)
     m3 = labeled_module("m3") {prepend m1; include m1}
     assert_equal([m0, m1, m3], m3.ancestors)
     m3 = labeled_module("m3") {prepend m1; prepend m1}
-- 
cgit v0.10.2


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

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