Skip to content

Commit

Permalink
Fix {Method,UnboundMethod}#{public?,private?,protected?} for ZSUPER m…
Browse files Browse the repository at this point in the history
…ethods

Add a visibility member to struct METHOD storing the original
method visibility, and use that, instead of taking the visibility
from the stored method entry (which may have different visibility
for ZSUPER methods).

Consider Method/UnboundMethod objects different if they have
different visibilities.

Fixes [Bug #18435]
  • Loading branch information
jeremyevans committed Jan 14, 2022
1 parent a93cc3e commit 58dc8bf
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 4 deletions.
15 changes: 11 additions & 4 deletions proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ struct METHOD {
const VALUE iclass;
const rb_method_entry_t * const me;
/* for bound methods, `me' should be rb_callable_method_entry_t * */
rb_method_visibility_t visibility;
};

VALUE rb_cUnboundMethod;
Expand Down Expand Up @@ -1626,6 +1627,7 @@ mnew_missing(VALUE klass, VALUE obj, ID id, VALUE mclass)
me = rb_method_entry_create(id, klass, METHOD_VISI_UNDEF, def);

RB_OBJ_WRITE(method, &data->me, me);
data->visibility = METHOD_ENTRY_VISI(me);

return method;
}
Expand Down Expand Up @@ -1683,6 +1685,7 @@ mnew_internal(const rb_method_entry_t *me, VALUE klass, VALUE iclass,
RB_OBJ_WRITE(method, &data->klass, klass);
RB_OBJ_WRITE(method, &data->iclass, iclass);
RB_OBJ_WRITE(method, &data->me, me);
data->visibility = visi;

return method;
}
Expand Down Expand Up @@ -1780,6 +1783,7 @@ method_eq(VALUE method, VALUE other)

if (!rb_method_entry_eq(m1->me, m2->me) ||
klass1 != klass2 ||
m1->visibility != m2->visibility ||
m1->klass != m2->klass ||
m1->recv != m2->recv) {
return Qfalse;
Expand Down Expand Up @@ -1833,6 +1837,7 @@ method_unbind(VALUE obj)
RB_OBJ_WRITE(method, &data->klass, orig->klass);
RB_OBJ_WRITE(method, &data->iclass, orig->iclass);
RB_OBJ_WRITE(method, &data->me, rb_method_entry_clone(orig->me));
data->visibility = orig->visibility;

return method;
}
Expand Down Expand Up @@ -2340,6 +2345,7 @@ method_clone(VALUE self)
RB_OBJ_WRITE(clone, &data->klass, orig->klass);
RB_OBJ_WRITE(clone, &data->iclass, orig->iclass);
RB_OBJ_WRITE(clone, &data->me, rb_method_entry_clone(orig->me));
data->visibility = orig->visibility;
return clone;
}

Expand Down Expand Up @@ -2590,6 +2596,7 @@ umethod_bind(VALUE method, VALUE recv)
RB_OBJ_WRITE(method, &bound->klass, klass);
RB_OBJ_WRITE(method, &bound->iclass, iclass);
RB_OBJ_WRITE(method, &bound->me, me);
bound->visibility = data->visibility;

return method;
}
Expand Down Expand Up @@ -2625,7 +2632,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, me, METHOD_ENTRY_VISI(me) };

return call_method_data(ec, &bound, argc, argv, passed_procval, RB_PASS_CALLED_KEYWORDS);
}
Expand Down Expand Up @@ -3314,7 +3321,7 @@ method_public_p(VALUE method)
{
const struct METHOD *data;
TypedData_Get_Struct(method, struct METHOD, &method_data_type, data);
return RBOOL(METHOD_ENTRY_VISI(data->me) == METHOD_VISI_PUBLIC);
return RBOOL(data->visibility == METHOD_VISI_PUBLIC);
}

/*
Expand All @@ -3329,7 +3336,7 @@ method_protected_p(VALUE method)
{
const struct METHOD *data;
TypedData_Get_Struct(method, struct METHOD, &method_data_type, data);
return RBOOL(METHOD_ENTRY_VISI(data->me) == METHOD_VISI_PROTECTED);
return RBOOL(data->visibility == METHOD_VISI_PROTECTED);
}

/*
Expand All @@ -3344,7 +3351,7 @@ method_private_p(VALUE method)
{
const struct METHOD *data;
TypedData_Get_Struct(method, struct METHOD, &method_data_type, data);
return RBOOL(METHOD_ENTRY_VISI(data->me) == METHOD_VISI_PRIVATE);
return RBOOL(data->visibility == METHOD_VISI_PRIVATE);
}

/*
Expand Down
30 changes: 30 additions & 0 deletions test/ruby/test_method.rb
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,11 @@ def m.foo; end
assert_equal(o.method(:foo), o.method(:foo))
assert_equal(o.method(:foo), o.method(:bar))
assert_not_equal(o.method(:foo), o.method(:baz))

class << o
private :bar
end
assert_not_equal(o.method(:foo), o.method(:bar))
end

def test_hash
Expand Down Expand Up @@ -1200,6 +1205,31 @@ def test_unbound_method_visibility_predicates
assert_equal(false, Visibility.instance_method(:mv1).protected?)
end

class VisibilitySub < Visibility
protected :mv1
public :mv2
private :mv3
end

def test_method_visibility_predicates_with_subclass_visbility_change
v = VisibilitySub.new
assert_equal(false, v.method(:mv1).public?)
assert_equal(false, v.method(:mv2).private?)
assert_equal(false, v.method(:mv3).protected?)
assert_equal(true, v.method(:mv2).public?)
assert_equal(true, v.method(:mv3).private?)
assert_equal(true, v.method(:mv1).protected?)
end

def test_unbound_method_visibility_predicates_with_subclass_visbility_change
assert_equal(false, VisibilitySub.instance_method(:mv1).public?)
assert_equal(false, VisibilitySub.instance_method(:mv2).private?)
assert_equal(false, VisibilitySub.instance_method(:mv3).protected?)
assert_equal(true, VisibilitySub.instance_method(:mv2).public?)
assert_equal(true, VisibilitySub.instance_method(:mv3).private?)
assert_equal(true, VisibilitySub.instance_method(:mv1).protected?)
end

def rest_parameter(*rest)
rest
end
Expand Down

0 comments on commit 58dc8bf

Please sign in to comment.