summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authornagachika <nagachika@ruby-lang.org>2022-10-01 13:39:47 +0900
committernagachika <nagachika@ruby-lang.org>2022-10-01 15:32:44 +0900
commit9e739022ded433f189a575017d3936b79541f229 (patch)
treecfb987600d99acf60ab1959873e1cbf51028f8f7
parente46532feafadef252682794c941180df72483c19 (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.c76
-rw-r--r--spec/ruby/core/method/fixtures/classes.rb24
-rw-r--r--spec/ruby/core/method/owner_spec.rb6
-rw-r--r--spec/ruby/core/method/super_method_spec.rb19
-rw-r--r--spec/ruby/core/unboundmethod/owner_spec.rb7
-rw-r--r--spec/ruby/core/unboundmethod/super_method_spec.rb21
-rw-r--r--test/ruby/test_method.rb112
-rw-r--r--version.h6
8 files changed, 214 insertions, 57 deletions
diff --git a/proc.c b/proc.c
index 83c7fe4410..84e6645835 100644
--- a/proc.c
+++ b/proc.c
@@ -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
diff --git a/version.h b/version.h
index ea82e737c8..151d2a5178 100644
--- a/version.h
+++ b/version.h
@@ -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"