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

ruby-changes:72996

From: Jeremy <ko1@a...>
Date: Sat, 20 Aug 2022 20:44:28 +0900 (JST)
Subject: [ruby-changes:72996] 8212aab81a (master): Make Object#method and Module#instance_method not skip ZSUPER methods

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

From 8212aab81a77a2a91fb7c1681b4968171193b48f Mon Sep 17 00:00:00 2001
From: Jeremy Evans <code@j...>
Date: Mon, 27 Dec 2021 09:39:15 -0800
Subject: Make Object#method and Module#instance_method not skip ZSUPER methods

Based on https://github.com/jeremyevans/ruby/commit/c95e7e5329140f640b6497905485761f3336d967

Among other things, this fixes calling visibility methods (public?,
protected?, and private?) on them.  It also fixes #owner to show the
class the zsuper method entry is defined in, instead of the original
class it references.

For some backwards compatibility, adjust #parameters and #source_location,
to show the parameters and source location of the method originally
defined. Also have the parameters and source location still be shown
by #inspect.

Clarify documentation of {Method,UnboundMethod}#owner.

Add tests based on the description of https://bugs.ruby-lang.org/issues/18435
and based on https://github.com/ruby/ruby/pull/5356#issuecomment-1005298809

Fixes [Bug #18435] [Bug #18729]

Co-authored-by: Benoit Daloze <eregontp@g...>
---
 proc.c                   | 63 +++++++++++++++++++++++++++++++++++-------------
 test/ruby/test_method.rb | 59 +++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 100 insertions(+), 22 deletions(-)

diff --git a/proc.c b/proc.c
index 3c52fb06a7..dbf28aa55e 100644
--- a/proc.c
+++ b/proc.c
@@ -1684,7 +1684,6 @@ mnew_internal(const rb_method_entry_t *me, VALUE klass, VALUE iclass, https://github.com/ruby/ruby/blob/trunk/proc.c#L1684
     VALUE method;
     rb_method_visibility_t visi = METHOD_VISI_UNDEF;
 
-  again:
     if (UNDEFINED_METHOD_ENTRY_P(me)) {
         if (respond_to_missing_p(klass, obj, ID2SYM(id), scope)) {
             return mnew_missing(klass, obj, id, mclass);
@@ -1700,19 +1699,6 @@ mnew_internal(const rb_method_entry_t *me, VALUE klass, VALUE iclass, https://github.com/ruby/ruby/blob/trunk/proc.c#L1699
             rb_print_inaccessible(klass, id, visi);
         }
     }
-    if (me->def->type == VM_METHOD_TYPE_ZSUPER) {
-        if (me->defined_class) {
-            VALUE klass = RCLASS_SUPER(RCLASS_ORIGIN(me->defined_class));
-            id = me->def->original_id;
-            me = (rb_method_entry_t *)rb_callable_method_entry_with_refinements(klass, id, &iclass);
-        }
-        else {
-            VALUE klass = RCLASS_SUPER(RCLASS_ORIGIN(me->owner));
-            id = me->def->original_id;
-            me = rb_method_entry_without_refinements(klass, id, &iclass);
-        }
-        goto again;
-    }
 
     method = TypedData_Make_Struct(mclass, struct METHOD, &method_data_type, data);
 
@@ -1934,7 +1920,15 @@ method_original_name(VALUE obj) https://github.com/ruby/ruby/blob/trunk/proc.c#L1920
  *  call-seq:
  *     meth.owner    -> class_or_module
  *
- *  Returns the class or module that defines the method.
+ *  Returns the class or module on which this method is defined.
+ *  In other words,
+ *
+ *    meth.owner.instance_methods(false).include?(meth.name) # => true
+ *
+ *  holds as long as the method is not removed/undefined/replaced,
+ *  (with private_instance_methods instead of instance_methods if the method
+ *  is private).
+ *
  *  See also Method#receiver.
  *
  *    (1..3).method(:map).owner #=> Enumerable
@@ -2951,6 +2945,24 @@ rb_method_entry_location(const rb_method_entry_t *me) https://github.com/ruby/ruby/blob/trunk/proc.c#L2945
     return method_def_location(me->def);
 }
 
+static VALUE method_super_method(VALUE method);
+
+static const rb_method_definition_t *
+zsuper_ref_method_def(VALUE method)
+{
+    const rb_method_definition_t *def = rb_method_def(method);
+    VALUE super_method;
+    while (def->type == VM_METHOD_TYPE_ZSUPER) {
+        super_method = method_super_method(method);
+        if (NIL_P(super_method)) {
+            break;
+        }
+        method = super_method;
+        def = rb_method_def(method);
+    }
+    return def;
+}
+
 /*
  * call-seq:
  *    meth.source_location  -> [String, Integer]
@@ -2962,7 +2974,7 @@ rb_method_entry_location(const rb_method_entry_t *me) https://github.com/ruby/ruby/blob/trunk/proc.c#L2974
 VALUE
 rb_method_location(VALUE method)
 {
-    return method_def_location(rb_method_def(method));
+    return method_def_location(zsuper_ref_method_def(method));
 }
 
 static const rb_method_definition_t *
@@ -3050,7 +3062,7 @@ method_def_parameters(const rb_method_definition_t *def) https://github.com/ruby/ruby/blob/trunk/proc.c#L3062
 static VALUE
 rb_method_parameters(VALUE method)
 {
-    return method_def_parameters(rb_method_def(method));
+    return method_def_parameters(zsuper_ref_method_def(method));
 }
 
 /*
@@ -3112,6 +3124,23 @@ method_inspect(VALUE method) https://github.com/ruby/ruby/blob/trunk/proc.c#L3124
     if (data->me->def->type == VM_METHOD_TYPE_ALIAS) {
         defined_class = data->me->def->body.alias.original_me->owner;
     }
+    else if (data->me->def->type == VM_METHOD_TYPE_ZSUPER) {
+        const rb_method_definition_t *zsuper_ref_def = data->me->def;
+        struct METHOD *zsuper_ref_data;
+        VALUE super_method;
+
+        do {
+            super_method = method_super_method(method);
+            if (NIL_P(super_method)) {
+                break;
+            }
+            method = super_method;
+            zsuper_ref_def = rb_method_def(method);
+        } while (zsuper_ref_def->type == VM_METHOD_TYPE_ZSUPER);
+
+        TypedData_Get_Struct(method, struct METHOD, &method_data_type, zsuper_ref_data);
+        defined_class = method_entry_defined_class(zsuper_ref_data->me);
+    }
     else {
         defined_class = method_entry_defined_class(data->me);
     }
diff --git a/test/ruby/test_method.rb b/test/ruby/test_method.rb
index 56e94493d9..5f689c3d4f 100644
--- a/test/ruby/test_method.rb
+++ b/test/ruby/test_method.rb
@@ -1056,20 +1056,28 @@ class TestMethod < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_method.rb#L1056
     assert_equal(sm, im.clone.bind(o).super_method)
   end
 
-  def test_super_method_removed
+  def test_super_method_removed_public
     c1 = Class.new {private def foo; end}
     c2 = Class.new(c1) {public :foo}
     c3 = Class.new(c2) {def foo; end}
     c1.class_eval {undef foo}
     m = c3.instance_method(:foo)
     m = assert_nothing_raised(NameError, Feature9781) {break m.super_method}
-    assert_nil(m, Feature9781)
+    assert_equal c2, m.owner
+  end
+
+  def test_super_method_removed_regular
+    c1 = Class.new { def foo; end }
+    c2 = Class.new(c1) { def foo; end }
+    assert_equal c1.instance_method(:foo), c2.instance_method(:foo).super_method
+    c1.remove_method :foo
+    assert_equal nil, c2.instance_method(:foo).super_method
   end
 
   def test_prepended_public_zsuper
     mod = EnvUtil.labeled_module("Mod") {private def foo; :ok end}
-    mods = [mod]
     obj = Object.new.extend(mod)
+    mods = [obj.singleton_class]
     class << obj
       public :foo
     end
@@ -1079,7 +1087,7 @@ class TestMethod < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_method.rb#L1087
     end
     m = obj.method(:foo)
     assert_equal(mods, mods.map {m.owner.tap {m = m.super_method}})
-    assert_nil(m)
+    assert_nil(m.super_method)
   end
 
   def test_super_method_with_prepended_module
@@ -1192,6 +1200,47 @@ class TestMethod < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_method.rb#L1200
     assert_nil(super_method)
   end
 
+  # Bug 18435
+  def test_instance_methods_owner_consistency
+    a = Module.new { def method1; end }
+
+    b = Class.new do
+      include a
+      protected :method1
+    end
+
+    assert_equal [:method1], b.instance_methods(false)
+    assert_equal b, b.instance_method(:method1).owner
+  end
+
+  def test_zsuper_method_removed
+    a = EnvUtil.labeled_class('A') do
+      private
+      def foo(arg = nil)
+        1
+      end
+    end
+    line = __LINE__ - 4
+
+    b = EnvUtil.labeled_class('B', a) do
+      public :foo
+    end
+
+    unbound = b.instance_method(:foo)
+
+    assert_equal unbound, b.public_instance_method(:foo)
+    assert_equal "#<UnboundMethod: B(A)#foo(arg=...) #{__FILE__}:#{line}>", unbound.inspect
+    assert_equal [[:opt, :arg]], unbound.parameters
+
+    a.remove_method(:foo)
+
+    assert_equal [[:rest]], unbound.parameters
+    assert_equal "#<UnboundMethod: B#foo(*)>", unbound.inspect
+
+    obj = b.new
+    assert_raise_with_message(NoMethodError, /super: no superclass method `foo'/) { unbound.bind_call(obj) }
+  end
+
   def rest_parameter(*rest)
     rest
   end
@@ -1310,7 +1359,7 @@ class TestMethod < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_method.rb#L1359
       ::Object.prepend(M2)
 
       m = Object.instance_method(:x)
-      assert_equal M, m.owner
+      assert_equal M2, m.owner
     end;
   end
 
-- 
cgit v1.2.1


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

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