Project

General

Profile

Actions

Feature #18798

open

`UnboundMethod#==` with inherited classes

Added by ko1 (Koichi Sasada) 7 months ago. Updated 5 days ago.

Status:
Open
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:108659]

Description

Now UnboundMethod for a same method from a superclass and an inherited class are not ==.

class C
  def foo = :C
  $mc = instance_method(:foo)
end

class D < C
  $md = instance_method(:foo)
end

p $mc == $md #=> false
p $mc.owner #=> C
p $mc.owner == $md.owner #=> true
p $mc.source_location == $md.source_location #=> true
p $mc.inspect #=> "#<UnboundMethod: C#foo() t.rb:3>"
p $md.inspect #=> "#<UnboundMethod: D(C)#foo() t.rb:3>"

How about to make it UnboundMethod#== return true for this case?
Rule: "return true if the UnboundMethod objects point to a same method definition" seems simple.

FYI: On aliased unbound methods point to a same method are ==.

class C
  def foo = :C
  alias bar foo
  $mfoo = instance_method(:foo)
  $mbar = instance_method(:bar)
end

p $mfoo, $mbar
#=> #<UnboundMethod: C#foo() t.rb:2>
#=> #<UnboundMethod: C#bar(foo)() t.rb:2>

p $mfoo == $mbar #=> true

Related issues 2 (0 open2 closed)

Related to Ruby master - Bug #18751: Regression on master for Method#== when comparing public with private methodClosedEregon (Benoit Daloze)Actions
Has duplicate Ruby master - Feature #18969: Compare only method definitions for Method#== and UnboundMethod#==ClosedActions
Actions #1

Updated by ko1 (Koichi Sasada) 7 months ago

  • Description updated (diff)

Updated by sawa (Tsuyoshi Sawada) 7 months ago

Did you mean:

p $mc.owner == $md.owner #=> true
p $mc.source_location == $md.source_location #=> true

I think the proposal is a good idea.

Updated by jeremyevans0 (Jeremy Evans) 7 months ago

I'm not against this change (for UnboundMethod, I think Method should remain different), but it seems more like a feature request than a bug fix to me.

Updated by Eregon (Benoit Daloze) 7 months ago

+1. @ko1 (Koichi Sasada) I guess you meant this as a feature request?

Updated by Eregon (Benoit Daloze) 7 months ago

Regarding Method#==, I think it should also respects the simple rule "point to a same method definition" + ensure the receiver is the same object for both Method instances.

Updated by ko1 (Koichi Sasada) 7 months ago

  • Tracker changed from Bug to Feature
  • ruby -v deleted (ruby 3.2.0dev (2022-01-14T04:46:12Z gh-4636 c613d79f9b) [x64-mswin64_140])
  • Backport deleted (2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN)

Ah, yes, it is a feature request.

Updated by ko1 (Koichi Sasada) 7 months ago

  • Description updated (diff)

ah, mistake...

Actions #8

Updated by Eregon (Benoit Daloze) 4 months ago

  • Has duplicate Feature #18969: Compare only method definitions for Method#== and UnboundMethod#== added

Updated by Eregon (Benoit Daloze) 4 months ago

I think we should do this, because I think the main purpose of UnboundMethod#== is to find out if two methods have the same definition (or in other words, are equivalent in behavior).

I filed #18969 which is a duplicate.
That issue also suggests that if changing UnboundMethod#== is not acceptable, then a new method could do that e.g. {Method,UnboundMethod}#same_definition?(other).

Updated by joel@drapper.me (Joel Drapper) 3 months ago

This would be really helpful for checking if a class has redefined a method inherited form a superclass.

As an example, I’m working on a compiler that replaces certain method calls with their inlined expected behaviour from an abstract superclass. Because it's possible for the subclass to override these abstract methods, the compiler should check if instance_method(name) == AbstractClass.instance_method(name) before folding it. While this wouldn't cover the method being overridden in the singleton class, that's a reasonable concession for my specific use case.

Updated by matz (Yukihiro Matsumoto) 2 months ago

I don't think UnboundMethod needs the reference to the class that generates the object, so that UnboundMethod#== works.

Matz.

Updated by Eregon (Benoit Daloze) 2 months ago

UnboundMethod#inspect currently shows the class used for lookup:

irb(main):001:0> String.instance_method(:object_id)
=> #<UnboundMethod: String(Kernel)#object_id()>

Without it we can't show String here, it'd have to be #<UnboundMethod: Kernel#object_id()>.

We might also need it for #super_method (when the method entry is defined on a module and not a class), although in CRuby it seems the iclass field is used for that.
The iclass field can already change the result of super_method but does not affect #== or #hash.

I'm not sure this incompatibility is OK, it seems easier to only change #== (UnboundMethod#hash already ignores the class).
On the upside, removing that class from #inspect and as a field means there is only one module/class shown per UnboundMethod (well, except there is a hidden iclass which does matter for #super_method).

Updated by Eregon (Benoit Daloze) 2 months ago

In code terms:

module M
  def foo
  end
end

class A
  prepend M
  def foo
  end
end

class B
  prepend M
  def foo
  end
end

a = A.instance_method(:foo)
b = B.instance_method(:foo)
p a
p b
p a == b
p [a.super_method, b.super_method]

Currently:

#<UnboundMethod: A(M)#foo() umethod_iclass.rb:2>
#<UnboundMethod: B(M)#foo() umethod_iclass.rb:2>
false
[#<UnboundMethod: A#foo() umethod_iclass.rb:8>, #<UnboundMethod: B#foo() umethod_iclass.rb:14>]

Proposed (just changing UnboundMethod#==):

#<UnboundMethod: A(M)#foo() umethod_iclass.rb:2>
#<UnboundMethod: B(M)#foo() umethod_iclass.rb:2>
true
[#<UnboundMethod: A#foo() umethod_iclass.rb:8>, #<UnboundMethod: B#foo() umethod_iclass.rb:14>]

No class field anymore:

#<UnboundMethod: M#foo() umethod_iclass.rb:2>
#<UnboundMethod: M#foo() umethod_iclass.rb:2> (no visual difference)
true
[#<UnboundMethod: A#foo() umethod_iclass.rb:8>, #<UnboundMethod: B#foo() umethod_iclass.rb:14>]

Updated by mame (Yusuke Endoh) 2 months ago

No class field anymore:

#<UnboundMethod: M#foo() umethod_iclass.rb:2>
#<UnboundMethod: M#foo() umethod_iclass.rb:2> (no visual difference)
true
[#<UnboundMethod: A#foo() umethod_iclass.rb:8>, #<UnboundMethod: B#foo() umethod_iclass.rb:14>]

This understanding is correct.

Discussed at the dev meeting. To the best of our knowledge, the receiver of instance_method is now only used in UnboundMethod#inspect (and #to_s). At the dev meeting, no one saw the significance of keeping track of this receiver. So @matz (Yukihiro Matsumoto) wants to discard the information and to change #==, #eql?, #hash (if needed), #inspect, and #to_s as you said.

Updated by Eregon (Benoit Daloze) 2 months ago

Thanks for confirming. It sounds good to me.
Does @ko1 (Koichi Sasada) or anyone else plan to work on this? Otherwise I can give it a try when I have some time.

Actions #17

Updated by matz (Yukihiro Matsumoto) 5 days ago

  • Related to Bug #18751: Regression on master for Method#== when comparing public with private method added
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0