ruby-changes:73386
From: nagachika <ko1@a...>
Date: Sat, 3 Sep 2022 15:06:12 +0900 (JST)
Subject: [ruby-changes:73386] 5473c1064d (ruby_3_1): merge revision(s) 8212aab81a77a2a91fb7c1681b4968171193b48f,209631a45f9682dedf718f4b4a140efe7d21a6fc: [Backport #18435]
https://git.ruby-lang.org/ruby.git/commit/?id=5473c1064d From 5473c1064d5e82d706b554f2fe5a5b41900f6b14 Mon Sep 17 00:00:00 2001 From: nagachika <nagachika@r...> Date: Sat, 3 Sep 2022 15:05:32 +0900 Subject: merge revision(s) 8212aab81a77a2a91fb7c1681b4968171193b48f,209631a45f9682dedf718f4b4a140efe7d21a6fc: [Backport #18435] 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(-) Consider resolved-through-zsuper methods equal for compatibility * Fixes https://bugs.ruby-lang.org/issues/18751 --- proc.c | 65 +++++++++++------------- spec/ruby/core/unboundmethod/equal_value_spec.rb | 37 ++++++++++++++ test/ruby/test_method.rb | 18 +++++++ 3 files changed, 86 insertions(+), 34 deletions(-) --- proc.c | 82 ++++++++++++++++-------- spec/ruby/core/unboundmethod/equal_value_spec.rb | 37 +++++++++++ test/ruby/test_method.rb | 77 ++++++++++++++++++++-- version.h | 6 +- 4 files changed, 166 insertions(+), 36 deletions(-) diff --git a/proc.c b/proc.c index d075b7382e..83c7fe4410 100644 --- a/proc.c +++ b/proc.c @@ -1647,7 +1647,6 @@ mnew_internal(const rb_method_entry_t *me, VALUE klass, VALUE iclass, https://github.com/ruby/ruby/blob/trunk/proc.c#L1647 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); @@ -1663,19 +1662,6 @@ mnew_internal(const rb_method_entry_t *me, VALUE klass, VALUE iclass, https://github.com/ruby/ruby/blob/trunk/proc.c#L1662 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); @@ -1715,6 +1701,27 @@ mnew_unbound(VALUE klass, ID id, VALUE mclass, int scope) https://github.com/ruby/ruby/blob/trunk/proc.c#L1701 return mnew_from_me(me, klass, iclass, Qundef, id, mclass, scope); } +static const rb_method_entry_t* +zsuper_resolve(const rb_method_entry_t *me) +{ + const rb_method_entry_t *super_me; + while (me->def->type == VM_METHOD_TYPE_ZSUPER) { + VALUE defined_class = me->defined_class ? me->defined_class : me->owner; + VALUE super_class = RCLASS_SUPER(RCLASS_ORIGIN(defined_class)); + if (!super_class) { + break; + } + ID id = me->def->original_id; + VALUE iclass; + super_me = (rb_method_entry_t *)rb_callable_method_entry_with_refinements(super_class, id, &iclass); + if (!super_me) { + break; + } + me = super_me; + } + return me; +} + static inline VALUE method_entry_defined_class(const rb_method_entry_t *me) { @@ -1767,22 +1774,25 @@ method_eq(VALUE method, VALUE other) https://github.com/ruby/ruby/blob/trunk/proc.c#L1774 VALUE klass1, klass2; if (!rb_obj_is_method(other)) - return Qfalse; + return Qfalse; if (CLASS_OF(method) != CLASS_OF(other)) - return Qfalse; + return Qfalse; Check_TypedStruct(method, &method_data_type); m1 = (struct METHOD *)DATA_PTR(method); m2 = (struct METHOD *)DATA_PTR(other); - klass1 = method_entry_defined_class(m1->me); - klass2 = method_entry_defined_class(m2->me); + const rb_method_entry_t *m1_me = zsuper_resolve(m1->me); + const rb_method_entry_t *m2_me = zsuper_resolve(m2->me); + + klass1 = method_entry_defined_class(m1_me); + klass2 = method_entry_defined_class(m2_me); - if (!rb_method_entry_eq(m1->me, m2->me) || - klass1 != klass2 || - m1->klass != m2->klass || - m1->recv != m2->recv) { - return Qfalse; + if (!rb_method_entry_eq(m1_me, m2_me) || + klass1 != klass2 || + m1->klass != m2->klass || + m1->recv != m2->recv) { + return Qfalse; } return Qtrue; @@ -1897,7 +1907,15 @@ method_original_name(VALUE obj) https://github.com/ruby/ruby/blob/trunk/proc.c#L1907 * 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 @@ -2896,6 +2914,14 @@ rb_method_entry_location(const rb_method_entry_t *me) https://github.com/ruby/ruby/blob/trunk/proc.c#L2914 return method_def_location(me->def); } +static const rb_method_definition_t * +zsuper_ref_method_def(VALUE method) +{ + const struct METHOD *data; + TypedData_Get_Struct(method, struct METHOD, &method_data_type, data); + return zsuper_resolve(data->me)->def; +} + /* * call-seq: * meth.source_location -> [String, Integer] @@ -2907,7 +2933,7 @@ rb_method_entry_location(const rb_method_entry_t *me) https://github.com/ruby/ruby/blob/trunk/proc.c#L2933 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 * @@ -2995,7 +3021,7 @@ method_def_parameters(const rb_method_definition_t *def) https://github.com/ruby/ruby/blob/trunk/proc.c#L3021 static VALUE rb_method_parameters(VALUE method) { - return method_def_parameters(rb_method_def(method)); + return method_def_parameters(zsuper_ref_method_def(method)); } /* @@ -3055,10 +3081,10 @@ method_inspect(VALUE method) https://github.com/ruby/ruby/blob/trunk/proc.c#L3081 } if (data->me->def->type == VM_METHOD_TYPE_ALIAS) { - defined_class = data->me->def->body.alias.original_me->owner; + defined_class = data->me->def->body.alias.original_me->owner; } else { - defined_class = method_entry_defined_class(data->me); + defined_class = method_entry_defined_class(zsuper_resolve(data->me)); } if (RB_TYPE_P(defined_class, T_ICLASS)) { diff --git a/spec/ruby/core/unboundmethod/equal_value_spec.rb b/spec/ruby/core/unboundmethod/equal_value_spec.rb index 6242b04884..b21677687e 100644 --- a/spec/ruby/core/unboundmethod/equal_value_spec.rb +++ b/spec/ruby/core/unboundmethod/equal_value_spec.rb @@ -98,4 +98,41 @@ describe "UnboundMethod#==" do https://github.com/ruby/ruby/blob/trunk/spec/ruby/core/unboundmethod/equal_value_spec.rb#L98 (@discard_1 == UnboundMethodSpecs::Methods.instance_method(:discard_1)).should == false end + + it "considers methods through aliasing equal" do + c = Class.new do + class << self + alias_method :n, :new + end + end + + c.method(:new).should == c.method(:n) + c.method(:n).should == Class.instance_method(:new).bind(c) + end + + # On CRuby < 3.2, the 2 specs below pass due to method/instance_method skipping zsuper methods. + # We are interested in the general pattern working, i.e. the combination of method/instance_method + # and #== exposes the wanted behavior. + it "considers methods through visibility change equal" do + c = Class.new do + class << self + private :new + end + end + + c.method(:new).should == Class.instance_method(:new).bind(c) + end + + it "considers methods through aliasing and visibility change equal" do + c = Class.new do + class << self + alias_method :n, :new + private :new + end + end + + c.method(:new).should == c.method(:n) + c.method(:n).should == Class.instance_method(:new).bind(c) + c.method(:new).should == Class.instance_method(:new).bind(c) + end end diff --git a/test/ruby/test_method.rb b/test/ruby/test_method.rb index da68787933..f1522890e5 100644 --- a/test/ruby/test_method.rb +++ b/test/ruby/test_method.rb @@ -1045,20 +1045,28 @@ class TestMethod < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_method.rb#L1045 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.in (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/