From 8212aab81a77a2a91fb7c1681b4968171193b48f Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Mon, 27 Dec 2021 09:39:15 -0800 Subject: [PATCH] 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 --- 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, 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, 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) * 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) 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) 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) 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) 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 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 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 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 "#", unbound.inspect + assert_equal [[:opt, :arg]], unbound.parameters + + a.remove_method(:foo) + + assert_equal [[:rest]], unbound.parameters + assert_equal "#", 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 ::Object.prepend(M2) m = Object.instance_method(:x) - assert_equal M, m.owner + assert_equal M2, m.owner end; end