diff options
| author | nagachika <nagachika@ruby-lang.org> | 2022-10-01 13:39:47 +0900 |
|---|---|---|
| committer | nagachika <nagachika@ruby-lang.org> | 2022-10-01 15:32:44 +0900 |
| commit | 9e739022ded433f189a575017d3936b79541f229 (patch) | |
| tree | cfb987600d99acf60ab1959873e1cbf51028f8f7 | |
| parent | e46532feafadef252682794c941180df72483c19 (diff) | |
merge revision(s) 94cea3e4d0a60326bd95be762819eed8ccd59ca6,aa53d69aa21c4dfa2a78a1cec5cb34e9697b3d30,6b7d32a5e54088b6b4014529bbf2b4b8c1a96029,c6319026caa6c8f0f569f80011e8502349a04b14,aa490f9442c32cd0e1e449ac817f410bd5924c8b: [Backport #18435]
Fix {Method,UnboundMethod}#super_method for zsuper methods
* We need to resolve the zsuper method first, and then look the super
method of that.
---
proc.c | 25 ++++++++++++-----------
spec/ruby/core/method/super_method_spec.rb | 15 +++-----------
spec/ruby/core/unboundmethod/super_method_spec.rb | 16 ++++++---------
3 files changed, 22 insertions(+), 34 deletions(-)
Add specs for {Method,UnboundMethod}#owner of a zsuper method
---
spec/ruby/core/method/owner_spec.rb | 6 ++++++
spec/ruby/core/unboundmethod/owner_spec.rb | 7 +++++++
2 files changed, 13 insertions(+)
Resolve zsuper method during lookup but preserve owner separately
* See https://bugs.ruby-lang.org/issues/18729#note-34
* See [Bug #18729]
---
proc.c | 109 +++++++++++++++++++++++++----------------------
test/ruby/test_method.rb | 66 +++++++++++++++++++++++-----
2 files changed, 114 insertions(+), 61 deletions(-)
Extend tests for a zsuper method of which the method it resolved to
has been removed
---
test/ruby/test_method.rb | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
Reduce diff to proc.c @ b0b9f7201acab05c2a3ad92c3043a1f01df3e17f
* So it's easy to review https://github.com/ruby/ruby/pull/6242 +
https://github.com/ruby/ruby/pull/6467 and there are less changes
overall.
---
proc.c | 76 ++++++++++++++++++------------------------------
test/ruby/test_method.rb | 7 +++--
2 files changed, 34 insertions(+), 49 deletions(-)
| -rw-r--r-- | proc.c | 76 | ||||
| -rw-r--r-- | spec/ruby/core/method/fixtures/classes.rb | 24 | ||||
| -rw-r--r-- | spec/ruby/core/method/owner_spec.rb | 6 | ||||
| -rw-r--r-- | spec/ruby/core/method/super_method_spec.rb | 19 | ||||
| -rw-r--r-- | spec/ruby/core/unboundmethod/owner_spec.rb | 7 | ||||
| -rw-r--r-- | spec/ruby/core/unboundmethod/super_method_spec.rb | 21 | ||||
| -rw-r--r-- | test/ruby/test_method.rb | 112 | ||||
| -rw-r--r-- | version.h | 6 |
8 files changed, 214 insertions, 57 deletions
@@ -37,7 +37,11 @@ const rb_cref_t *rb_vm_cref_in_context(VALUE self, VALUE cbase); struct METHOD { const VALUE recv; const VALUE klass; + /* needed for #super_method */ const VALUE iclass; + /* Different than me->owner only for ZSUPER methods. + This is error-prone but unavoidable unless ZSUPER methods are removed. */ + const VALUE owner; const rb_method_entry_t * const me; /* for bound methods, `me' should be rb_callable_method_entry_t * */ }; @@ -1618,6 +1622,7 @@ mnew_missing(VALUE klass, VALUE obj, ID id, VALUE mclass) RB_OBJ_WRITE(method, &data->recv, obj); RB_OBJ_WRITE(method, &data->klass, klass); + RB_OBJ_WRITE(method, &data->owner, klass); def = ZALLOC(rb_method_definition_t); def->type = VM_METHOD_TYPE_MISSING; @@ -1641,12 +1646,14 @@ mnew_missing_by_name(VALUE klass, VALUE obj, VALUE *name, int scope, VALUE mclas static VALUE mnew_internal(const rb_method_entry_t *me, VALUE klass, VALUE iclass, - VALUE obj, ID id, VALUE mclass, int scope, int error) + VALUE obj, ID id, VALUE mclass, int scope, int error) { struct METHOD *data; VALUE method; + const rb_method_entry_t *original_me = me; 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); @@ -1662,12 +1669,26 @@ 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); RB_OBJ_WRITE(method, &data->recv, obj); RB_OBJ_WRITE(method, &data->klass, klass); RB_OBJ_WRITE(method, &data->iclass, iclass); + RB_OBJ_WRITE(method, &data->owner, original_me->owner); RB_OBJ_WRITE(method, &data->me, me); return method; @@ -1701,27 +1722,6 @@ mnew_unbound(VALUE klass, ID id, VALUE mclass, int scope) 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) { @@ -1782,13 +1782,10 @@ method_eq(VALUE method, VALUE other) m1 = (struct METHOD *)DATA_PTR(method); m2 = (struct METHOD *)DATA_PTR(other); - 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); - klass1 = method_entry_defined_class(m1_me); - klass2 = method_entry_defined_class(m2_me); - - if (!rb_method_entry_eq(m1_me, m2_me) || + if (!rb_method_entry_eq(m1->me, m2->me) || klass1 != klass2 || m1->klass != m2->klass || m1->recv != m2->recv) { @@ -1842,6 +1839,7 @@ method_unbind(VALUE obj) RB_OBJ_WRITE(method, &data->recv, Qundef); RB_OBJ_WRITE(method, &data->klass, orig->klass); RB_OBJ_WRITE(method, &data->iclass, orig->iclass); + RB_OBJ_WRITE(method, &data->owner, orig->owner); RB_OBJ_WRITE(method, &data->me, rb_method_entry_clone(orig->me)); return method; @@ -1926,7 +1924,7 @@ method_owner(VALUE obj) { struct METHOD *data; TypedData_Get_Struct(obj, struct METHOD, &method_data_type, data); - return data->me->owner; + return data->owner; } void @@ -2357,6 +2355,7 @@ method_clone(VALUE self) RB_OBJ_WRITE(clone, &data->recv, orig->recv); RB_OBJ_WRITE(clone, &data->klass, orig->klass); RB_OBJ_WRITE(clone, &data->iclass, orig->iclass); + RB_OBJ_WRITE(clone, &data->owner, orig->owner); RB_OBJ_WRITE(clone, &data->me, rb_method_entry_clone(orig->me)); return clone; } @@ -2518,7 +2517,7 @@ rb_method_call_with_block(int argc, const VALUE *argv, VALUE method, VALUE passe static void convert_umethod_to_method_components(const struct METHOD *data, VALUE recv, VALUE *methclass_out, VALUE *klass_out, VALUE *iclass_out, const rb_method_entry_t **me_out) { - VALUE methclass = data->me->owner; + VALUE methclass = data->owner; VALUE iclass = data->me->defined_class; VALUE klass = CLASS_OF(recv); @@ -2607,6 +2606,7 @@ umethod_bind(VALUE method, VALUE recv) RB_OBJ_WRITE(method, &bound->recv, recv); RB_OBJ_WRITE(method, &bound->klass, klass); RB_OBJ_WRITE(method, &bound->iclass, iclass); + RB_OBJ_WRITE(method, &bound->owner, methclass); RB_OBJ_WRITE(method, &bound->me, me); return method; @@ -2643,7 +2643,7 @@ umethod_bind_call(int argc, VALUE *argv, VALUE method) VALUE methclass, klass, iclass; const rb_method_entry_t *me; convert_umethod_to_method_components(data, recv, &methclass, &klass, &iclass, &me); - struct METHOD bound = { recv, klass, 0, me }; + struct METHOD bound = { recv, klass, 0, methclass, me }; return call_method_data(ec, &bound, argc, argv, passed_procval, RB_PASS_CALLED_KEYWORDS); } @@ -2914,14 +2914,6 @@ rb_method_entry_location(const rb_method_entry_t *me) 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] @@ -2933,7 +2925,7 @@ zsuper_ref_method_def(VALUE method) VALUE rb_method_location(VALUE method) { - return method_def_location(zsuper_ref_method_def(method)); + return method_def_location(rb_method_def(method)); } static const rb_method_definition_t * @@ -3021,7 +3013,7 @@ method_def_parameters(const rb_method_definition_t *def) static VALUE rb_method_parameters(VALUE method) { - return method_def_parameters(zsuper_ref_method_def(method)); + return method_def_parameters(rb_method_def(method)); } /* @@ -3084,7 +3076,7 @@ method_inspect(VALUE method) defined_class = data->me->def->body.alias.original_me->owner; } else { - defined_class = method_entry_defined_class(zsuper_resolve(data->me)); + defined_class = method_entry_defined_class(data->me); } if (RB_TYPE_P(defined_class, T_ICLASS)) { diff --git a/spec/ruby/core/method/fixtures/classes.rb b/spec/ruby/core/method/fixtures/classes.rb index be96f65e25..50daa773e1 100644 --- a/spec/ruby/core/method/fixtures/classes.rb +++ b/spec/ruby/core/method/fixtures/classes.rb @@ -213,4 +213,28 @@ module MethodSpecs n * m end end + + module InheritedMethods + module A + private + def derp(message) + 'A' + end + end + + module B + private + def derp + 'B' + super('superclass') + end + end + + class C + include A + include B + + public :derp + alias_method :meow, :derp + end + end end diff --git a/spec/ruby/core/method/owner_spec.rb b/spec/ruby/core/method/owner_spec.rb index ca5dff7295..05422f1697 100644 --- a/spec/ruby/core/method/owner_spec.rb +++ b/spec/ruby/core/method/owner_spec.rb @@ -23,4 +23,10 @@ describe "Method#owner" do @m.method(:handled_via_method_missing).owner.should == MethodSpecs::Methods end end + + ruby_version_is "3.2" do + it "returns the class on which public was called for a private method in ancestor" do + MethodSpecs::InheritedMethods::C.new.method(:derp).owner.should == MethodSpecs::InheritedMethods::C + end + end end diff --git a/spec/ruby/core/method/super_method_spec.rb b/spec/ruby/core/method/super_method_spec.rb index e5d8b87a06..c63a7aaa0f 100644 --- a/spec/ruby/core/method/super_method_spec.rb +++ b/spec/ruby/core/method/super_method_spec.rb @@ -42,4 +42,23 @@ describe "Method#super_method" do method.super_method.should == nil end + + # https://github.com/jruby/jruby/issues/7240 + context "after changing an inherited methods visibility" do + it "calls the proper super method" do + MethodSpecs::InheritedMethods::C.new.derp.should == 'BA' + end + + it "returns the expected super_method" do + method = MethodSpecs::InheritedMethods::C.new.method(:derp) + method.super_method.owner.should == MethodSpecs::InheritedMethods::A + end + end + + context "after aliasing an inherited method" do + it "returns the expected super_method" do + method = MethodSpecs::InheritedMethods::C.new.method(:meow) + method.super_method.owner.should == MethodSpecs::InheritedMethods::A + end + end end diff --git a/spec/ruby/core/unboundmethod/owner_spec.rb b/spec/ruby/core/unboundmethod/owner_spec.rb index 5f1f4646b3..e8a734dac4 100644 --- a/spec/ruby/core/unboundmethod/owner_spec.rb +++ b/spec/ruby/core/unboundmethod/owner_spec.rb @@ -1,5 +1,6 @@ require_relative '../../spec_helper' require_relative 'fixtures/classes' +require_relative '../method/fixtures/classes' describe "UnboundMethod#owner" do it "returns the owner of the method" do @@ -23,4 +24,10 @@ describe "UnboundMethod#owner" do child_singleton_class.instance_method(:class_method).owner.should == parent_singleton_class child_singleton_class.instance_method(:another_class_method).owner.should == child_singleton_class end + + ruby_version_is "3.2" do + it "returns the class on which public was called for a private method in ancestor" do + MethodSpecs::InheritedMethods::C.instance_method(:derp).owner.should == MethodSpecs::InheritedMethods::C + end + end end diff --git a/spec/ruby/core/unboundmethod/super_method_spec.rb b/spec/ruby/core/unboundmethod/super_method_spec.rb index c9fa1ec533..aa7c129377 100644 --- a/spec/ruby/core/unboundmethod/super_method_spec.rb +++ b/spec/ruby/core/unboundmethod/super_method_spec.rb @@ -1,5 +1,6 @@ require_relative '../../spec_helper' require_relative 'fixtures/classes' +require_relative '../method/fixtures/classes' describe "UnboundMethod#super_method" do it "returns the method that would be called by super in the method" do @@ -25,4 +26,24 @@ describe "UnboundMethod#super_method" do method.super_method.should == nil end + + # https://github.com/jruby/jruby/issues/7240 + context "after changing an inherited methods visibility" do + it "calls the proper super method" do + method = MethodSpecs::InheritedMethods::C.instance_method(:derp) + method.bind(MethodSpecs::InheritedMethods::C.new).call.should == 'BA' + end + + it "returns the expected super_method" do + method = MethodSpecs::InheritedMethods::C.instance_method(:derp) + method.super_method.owner.should == MethodSpecs::InheritedMethods::A + end + end + + context "after aliasing an inherited method" do + it "returns the expected super_method" do + method = MethodSpecs::InheritedMethods::C.instance_method(:meow) + method.super_method.owner.should == MethodSpecs::InheritedMethods::A + end + end end diff --git a/test/ruby/test_method.rb b/test/ruby/test_method.rb index f1522890e5..ac50a9d0b0 100644 --- a/test/ruby/test_method.rb +++ b/test/ruby/test_method.rb @@ -1052,7 +1052,7 @@ class TestMethod < Test::Unit::TestCase c1.class_eval {undef foo} m = c3.instance_method(:foo) m = assert_nothing_raised(NameError, Feature9781) {break m.super_method} - assert_equal c2, m.owner + assert_nil(m, Feature9781) end def test_super_method_removed_regular @@ -1064,19 +1064,26 @@ class TestMethod < Test::Unit::TestCase end def test_prepended_public_zsuper - mod = EnvUtil.labeled_module("Mod") {private def foo; :ok end} + mod = EnvUtil.labeled_module("Mod") {private def foo; [:ok] end} obj = Object.new.extend(mod) - mods = [obj.singleton_class] + class << obj public :foo end - 2.times do |i| - mods.unshift(mod = EnvUtil.labeled_module("Mod#{i}") {def foo; end}) - obj.singleton_class.prepend(mod) - end + + mod1 = EnvUtil.labeled_module("Mod1") {def foo; [:mod1] + super end} + obj.singleton_class.prepend(mod1) + + mod2 = EnvUtil.labeled_module("Mod2") {def foo; [:mod2] + super end} + obj.singleton_class.prepend(mod2) + m = obj.method(:foo) - assert_equal(mods, mods.map {m.owner.tap {m = m.super_method}}) - assert_nil(m.super_method) + assert_equal mod2, m.owner + assert_equal mod1, m.super_method.owner + assert_equal obj.singleton_class, m.super_method.super_method.owner + assert_equal nil, m.super_method.super_method.super_method + + assert_equal [:mod2, :mod1, :ok], obj.foo end def test_super_method_with_prepended_module @@ -1242,11 +1249,92 @@ class TestMethod < Test::Unit::TestCase a.remove_method(:foo) - assert_equal [[:rest]], unbound.parameters - assert_equal "#<UnboundMethod: B#foo(*)>", unbound.inspect + assert_equal "#<UnboundMethod: B(A)#foo(arg=...) #{__FILE__}:#{line}>", unbound.inspect + assert_equal [[:opt, :arg]], unbound.parameters obj = b.new - assert_raise_with_message(NoMethodError, /super: no superclass method `foo'/) { unbound.bind_call(obj) } + assert_equal 1, unbound.bind_call(obj) + + assert_include b.instance_methods(false), :foo + link = 'https://github.com/ruby/ruby/pull/6467#issuecomment-1262159088' + assert_raise(NameError, link) { b.instance_method(:foo) } + # For #test_method_list below, otherwise we get the same error as just above + b.remove_method(:foo) + end + + def test_zsuper_method_removed_higher_method + a0 = EnvUtil.labeled_class('A0') do + def foo(arg1 = nil, arg2 = nil) + 0 + end + end + line0 = __LINE__ - 4 + a0_foo = a0.instance_method(:foo) + + a = EnvUtil.labeled_class('A', a0) 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 a0_foo, unbound.super_method + + a.remove_method(:foo) + + assert_equal "#<UnboundMethod: B(A)#foo(arg=...) #{__FILE__}:#{line}>", unbound.inspect + assert_equal [[:opt, :arg]], unbound.parameters + assert_equal a0_foo, unbound.super_method + + obj = b.new + assert_equal 1, unbound.bind_call(obj) + + assert_include b.instance_methods(false), :foo + assert_equal "#<UnboundMethod: B(A0)#foo(arg1=..., arg2=...) #{__FILE__}:#{line0}>", b.instance_method(:foo).inspect + end + + def test_zsuper_method_redefined_bind_call + c0 = EnvUtil.labeled_class('C0') do + def foo + [:foo] + end + end + + c1 = EnvUtil.labeled_class('C1', c0) do + def foo + super + [:bar] + end + end + m1 = c1.instance_method(:foo) + + c2 = EnvUtil.labeled_class('C2', c1) do + private :foo + end + + assert_equal [:foo], c2.private_instance_methods(false) + m2 = c2.instance_method(:foo) + + c1.class_exec do + def foo + [:bar2] + end + end + + m3 = c2.instance_method(:foo) + c = c2.new + assert_equal [:foo, :bar], m1.bind_call(c) + assert_equal c1, m1.owner + assert_equal [:foo, :bar], m2.bind_call(c) + assert_equal c2, m2.owner + assert_equal [:bar2], m3.bind_call(c) + assert_equal c2, m3.owner end # Bug #18751 @@ -11,11 +11,11 @@ # define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR #define RUBY_VERSION_TEENY 3 #define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR -#define RUBY_PATCHLEVEL 64 +#define RUBY_PATCHLEVEL 65 #define RUBY_RELEASE_YEAR 2022 -#define RUBY_RELEASE_MONTH 9 -#define RUBY_RELEASE_DAY 25 +#define RUBY_RELEASE_MONTH 10 +#define RUBY_RELEASE_DAY 1 #include "ruby/version.h" |
