Bug #22462: Method#== can return false for methods that should be considered equal

• Status: Open
• Assignee: -
• Target version: -
• Backport: 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN

[jeremyevans0 (Jeremy Evans)]

When a Method is created using UnboundMethod#bind and #super_method, the order in which the #bind and #super_method were applied could result in Method objects that were considered unequal.

I believe we should have the invariant: for any UnboundMethod um where super_method is not nil and o is a valid object to bind to um, the following code should be true:

um.bind(o).super_method == um.super_method.bind(o)

However, that didn't work correctly in some cases:

module M; def m = [:M, *super]; end
module N; include M; end
class C; def m = :C; end
class SC < C
  include N
  prepend M
  def m = [:SC, *super]
end
o = SC.new
um = SC.instance_method(:m).super_method.super_method  # M#m below SC (included via N)
um.bind(o).super_method == um.super_method.bind(o)
# => false # should be true

There is a similar invariant for Method: for any Method m where super_method is not nil, the following code should be true:

m.unbind.super_method == m.super_method.unbind

This appears to already hold, as the existing refinement tests already checked for this behavior.

I've submitted a PR to fix this issue: https://github.com/ruby/ruby/pull/19474

[shugo (Shugo Maeda)]

As I commented in https://github.com/ruby/ruby/pull/19474#pullrequestreview-5609342827, with the PR change, == UnboundMethods can bind to non-== Methods:

module M; def m = [:M, *super]; end
class Base; def m = [:Base]; end
class A < Base; include M; end
class B < A; prepend M; def m = [:B, *super]; end
b = B.new
ua = A.instance_method(:m)
ub = B.instance_method(:m)
p ua == ub                 # => true
p ua.bind(b) == ub.bind(b) # => true before, false after

I suggested there normalizing an iclass to its module in method_eq(), but Method objects with different iclasses may behave differently when super is called.

Another option would be to reconsider [Feature #18798].

If UnboundMethod#== compared iclasses, == would imply the same behavior, but ua == M.instance_method(:m) would also be false, as would Object.instance_method(:to_s) == Kernel.instance_method(:to_s). Treating an UnboundMethod obtained directly from the module as equal to one with any iclass of it would avoid this, but == would then not be transitive: ua == M.instance_method(:m) and M.instance_method(:m) == ub would both be true, while ua == ub would be false.

[jeremyevans0 (Jeremy Evans)]

shugo (Shugo Maeda) wrote in #note-1:

As I commented in https://github.com/ruby/ruby/pull/19474#pullrequestreview-5609342827, with the PR change, == UnboundMethods can bind to non-== Methods:

module M; def m = [:M, *super]; end
class Base; def m = [:Base]; end
class A < Base; include M; end
class B < A; prepend M; def m = [:B, *super]; end
b = B.new
ua = A.instance_method(:m)
ub = B.instance_method(:m)
p ua == ub                 # => true
p ua.bind(b) == ub.bind(b) # => true before, false after

I think the previous behavior was incorrect. Consider the following code using your example:

p ua.bind_call(b)
# Before: [:M, :B, :M, :Base]
#  After: [:M, :Base]

p ub.bind_call(b)
# Before and after: [:M, :B, :M, :Base]

I think the ua.bind_call(b) result was incorrect previously and is fixed by the PR. There is no reason that A.instance_method(:m).bind_call(b) should include the initial :M, :B, as the code for that is only present in B, not in A. Since the behavior for the two UnboundMethods is different, I think it makes sense that they are considered unequal. I didn't realize at the time I submitted it that my PR fixed this behavior, since my goal was to fix the equality issue. However, since I think it fixes behavior that is currently incorrect, I think that is more of a reason to merge it.