[ruby-core:108378] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

[ruby-core:108378] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

[Eregon (Benoit Daloze)]

Issue #18751 has been reported by Eregon (Benoit Daloze).

----------------------------------------
Bug #18751: Regression on master for Method#== when comparing public with private method
https://bugs.ruby-lang.org/issues/18751

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.2.0dev (2022-04-23T02:59:20Z master e142bea799) [x86_64-linux]
* Backport: 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN
----------------------------------------
This script repros:
```ruby
class C
  class << self
    alias_method :n, :new
    private :new
  end
end

p C.method(:n) == C.method(:new) # => true

puts
p C.method(:n) == Class.method(:new) # => false
p C.method(:n) == Class.method(:new).unbind.bind(C) # => true

puts
p C.method(:new) == Class.method(:new) # => false
p C.method(:new) == Class.method(:new).unbind.bind(C) # => true, BUT false on master
p C.method(:new) == Class.instance_method(:new).bind(C) # => true, BUT false on master
p [C.method(:new), Class.instance_method(:new).bind(C)]
```

So this prints the expected results on 2.7.5, 3.0.3, 3.1.1 but not on master, which seems a regression.
Notably this breaks the pattern discussed in https://bugs.ruby-lang.org/issues/18729#note-5, and it means there is no way to find out if two methods share the same "definition/logic/def", which is a big limitation.



-- 
https://bugs.ruby-lang.org/

[ruby-core:108395] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

[jeremyevans0 (Jeremy Evans)]

Issue #18751 has been updated by jeremyevans0 (Jeremy Evans).


This isn't a regression, this was a change I made deliberately in the fix to #18435.  It's listed in the commit message:

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

I neglected to update the documentation for `Method#==` to state that visibility is now also considered.

I don't have strong feelings about reverting the `Method#==` change.  It seems odd that two methods with different visibilities would be considered equal, but if doing so introduces a backwards compatibility issue in production code, maybe we shouldn't do that.  It would be good to add this as a developer meeting topic.

----------------------------------------
Bug #18751: Regression on master for Method#== when comparing public with private method
https://bugs.ruby-lang.org/issues/18751#change-97428

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.2.0dev (2022-04-23T02:59:20Z master e142bea799) [x86_64-linux]
* Backport: 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED
----------------------------------------
This script repros:
```ruby
class C
  class << self
    alias_method :n, :new
    private :new
  end
end

p C.method(:n) == C.method(:new) # => true

puts
p C.method(:n) == Class.method(:new) # => false
p C.method(:n) == Class.method(:new).unbind.bind(C) # => true

puts
p C.method(:new) == Class.method(:new) # => false
p C.method(:new) == Class.method(:new).unbind.bind(C) # => true, BUT false on master
p C.method(:new) == Class.instance_method(:new).bind(C) # => true, BUT false on master
p [C.method(:new), Class.instance_method(:new).bind(C)] # => [#<Method: #<Class:C>(Class)#new(*)>, #<Method: #<Class:C>(Class)#new(*)>]
```

So this prints the expected results on 2.7.5, 3.0.3, 3.1.1 but not on master, which seems a regression.
Notably this breaks the pattern discussed in https://bugs.ruby-lang.org/issues/18729#note-5, and it means there is no way to find out if two methods share the same "definition/logic/def", which is a big limitation.



-- 
https://bugs.ruby-lang.org/

[ruby-core:108396] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

[Eregon (Benoit Daloze)]

Issue #18751 has been updated by Eregon (Benoit Daloze).


jeremyevans0 (Jeremy Evans) wrote in #note-4:
> This isn't a regression

In my POV (as a user of Method/UnboundMethod#==) it's a regression, but I understand you have a different POV.

> It seems odd that two methods with different visibilities would be considered equal,

I understand, but then if we change this for 3.2 we also need a new method on Method/UnboundMethod to know if they refer to the same "method definition" i.e., the same code/the same piece of bytecode/the same C function/etc.
Without that, I consider that a regression because there is no way to find that out AFAIK.

I think the primary usage of Method/UnboundMethod#== is to compare "are they the same method definition?".

And not so much for whether two methods should be considered the same key in a Hash.
`eql?` could potentially be more strict (and consider visibility too), and that would be somewhat similar to the situation for Numeric, but not sure it's a good idea to have #== and #eql? differ for Method/UnboundMethod.

> but if doing so introduces a backwards compatibility issue in production code, maybe we shouldn't do that.

It kind of does, it breaks latest RSpec tests when run on ruby-head: https://github.com/rspec/rspec-mocks/runs/6128536621?check_suite_focus=true#step:8:1561
Mocking of new/initialize with RSpec likely doesn't work in real code due to this issue, if anyone has a private new, which doesn't seem unlikely (e.g., that makes sense for a singleton pattern class, or a class which want to cache instances, etc).
Since #18729 is accepted (and that was clearly inconsistent so a needed fix), the old `owner` check is not usable anymore, and with this issue the better (because it actually checks it's the same definition) Method#== check also does not work.

>  It would be good to add this as a developer meeting topic.

OK, I will add, but IMHO this is clear we need to fix it, the earlier the better.

----------------------------------------
Bug #18751: Regression on master for Method#== when comparing public with private method
https://bugs.ruby-lang.org/issues/18751#change-97429

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.2.0dev (2022-04-23T02:59:20Z master e142bea799) [x86_64-linux]
* Backport: 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED
----------------------------------------
This script repros:
```ruby
class C
  class << self
    alias_method :n, :new
    private :new
  end
end

p C.method(:n) == C.method(:new) # => true

puts
p C.method(:n) == Class.method(:new) # => false
p C.method(:n) == Class.method(:new).unbind.bind(C) # => true

puts
p C.method(:new) == Class.method(:new) # => false
p C.method(:new) == Class.method(:new).unbind.bind(C) # => true, BUT false on master
p C.method(:new) == Class.instance_method(:new).bind(C) # => true, BUT false on master
p [C.method(:new), Class.instance_method(:new).bind(C)] # => [#<Method: #<Class:C>(Class)#new(*)>, #<Method: #<Class:C>(Class)#new(*)>]
```

So this prints the expected results on 2.7.5, 3.0.3, 3.1.1 but not on master, which seems a regression.
Notably this breaks the pattern discussed in https://bugs.ruby-lang.org/issues/18729#note-5, and it means there is no way to find out if two methods share the same "definition/logic/def", which is a big limitation.



-- 
https://bugs.ruby-lang.org/

[ruby-core:108398] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

[Eregon (Benoit Daloze)]

Issue #18751 has been updated by Eregon (Benoit Daloze).


An advantage of a new method would be it could also ignore the receiver for Method and the source module for UnboundMethod (e.g., the module on which `instance_method` was called), so it would really only compare the actual definition of the method and nothing else, without needing extra bind/unbind.

----------------------------------------
Bug #18751: Regression on master for Method#== when comparing public with private method
https://bugs.ruby-lang.org/issues/18751#change-97431

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.2.0dev (2022-04-23T02:59:20Z master e142bea799) [x86_64-linux]
* Backport: 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED
----------------------------------------
This script repros:
```ruby
class C
  class << self
    alias_method :n, :new
    private :new
  end
end

p C.method(:n) == C.method(:new) # => true

puts
p C.method(:n) == Class.method(:new) # => false
p C.method(:n) == Class.method(:new).unbind.bind(C) # => true

puts
p C.method(:new) == Class.method(:new) # => false
p C.method(:new) == Class.method(:new).unbind.bind(C) # => true, BUT false on master
p C.method(:new) == Class.instance_method(:new).bind(C) # => true, BUT false on master
p [C.method(:new), Class.instance_method(:new).bind(C)] # => [#<Method: #<Class:C>(Class)#new(*)>, #<Method: #<Class:C>(Class)#new(*)>]
```

So this prints the expected results on 2.7.5, 3.0.3, 3.1.1 but not on master, which seems a regression.
Notably this breaks the pattern discussed in https://bugs.ruby-lang.org/issues/18729#note-5, and it means there is no way to find out if two methods share the same "definition/logic/def", which is a big limitation.



-- 
https://bugs.ruby-lang.org/

[ruby-core:108577] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

[mame (Yusuke Endoh)]

Issue #18751 has been updated by mame (Yusuke Endoh).


Let me confirm the current situation:

* Both #18435 and #18729 focus on the same issue (an inconsistency due to the fact that a Method object skips ZSUPER method entry)
* In #18435, the visibility information is now stored in a Method object to hide the inconsistency
* In #18729, we determined to allow a Method object for ZSUPER method entry to fix the inconsistency fundamentally
* In this ticket, Method#== has an incompatibility isue because it respects method visibility information stored in a Method object.

Right?

Now, I wonder if it is really needed to store the visibility information in a Method object. Will just reverting 58dc8bf8f15df9a33d191074e8a5d4946a3d59d5 solve this issue?

----------------------------------------
Bug #18751: Regression on master for Method#== when comparing public with private method
https://bugs.ruby-lang.org/issues/18751#change-97614

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.2.0dev (2022-04-23T02:59:20Z master e142bea799) [x86_64-linux]
* Backport: 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED
----------------------------------------
This script repros:
```ruby
class C
  class << self
    alias_method :n, :new
    private :new
  end
end

p C.method(:n) == C.method(:new) # => true

puts
p C.method(:n) == Class.method(:new) # => false
p C.method(:n) == Class.method(:new).unbind.bind(C) # => true

puts
p C.method(:new) == Class.method(:new) # => false
p C.method(:new) == Class.method(:new).unbind.bind(C) # => true, BUT false on master
p C.method(:new) == Class.instance_method(:new).bind(C) # => true, BUT false on master
p [C.method(:new), Class.instance_method(:new).bind(C)] # => [#<Method: #<Class:C>(Class)#new(*)>, #<Method: #<Class:C>(Class)#new(*)>]
```

So this prints the expected results on 2.7.5, 3.0.3, 3.1.1 but not on master, which seems a regression.
Notably this breaks the pattern discussed in https://bugs.ruby-lang.org/issues/18729#note-5, and it means there is no way to find out if two methods share the same "definition/logic/def", which is a big limitation.



-- 
https://bugs.ruby-lang.org/

[ruby-core:108581] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

[ioquatix (Samuel Williams)]

Issue #18751 has been updated by ioquatix (Samuel Williams).


It's okay for `==` to not be strong equality and be closer to equivalence. That's why we have different methods for "This is the identical thing" or "This is comparably equivalent". I always felt like `eql?` was better for "These are identical" vs `==` which is "These are equivalent or represent the same thing".

----------------------------------------
Bug #18751: Regression on master for Method#== when comparing public with private method
https://bugs.ruby-lang.org/issues/18751#change-97618

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.2.0dev (2022-04-23T02:59:20Z master e142bea799) [x86_64-linux]
* Backport: 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED
----------------------------------------
This script repros:
```ruby
class C
  class << self
    alias_method :n, :new
    private :new
  end
end

p C.method(:n) == C.method(:new) # => true

puts
p C.method(:n) == Class.method(:new) # => false
p C.method(:n) == Class.method(:new).unbind.bind(C) # => true

puts
p C.method(:new) == Class.method(:new) # => false
p C.method(:new) == Class.method(:new).unbind.bind(C) # => true, BUT false on master
p C.method(:new) == Class.instance_method(:new).bind(C) # => true, BUT false on master
p [C.method(:new), Class.instance_method(:new).bind(C)] # => [#<Method: #<Class:C>(Class)#new(*)>, #<Method: #<Class:C>(Class)#new(*)>]
```

So this prints the expected results on 2.7.5, 3.0.3, 3.1.1 but not on master, which seems a regression.
Notably this breaks the pattern discussed in https://bugs.ruby-lang.org/issues/18729#note-5, and it means there is no way to find out if two methods share the same "definition/logic/def", which is a big limitation.



-- 
https://bugs.ruby-lang.org/

[ruby-core:108583] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

[Eregon (Benoit Daloze)]

Issue #18751 has been updated by Eregon (Benoit Daloze).


@mame I'll let @jeremyevans0 reply since he knows better about the implementation details.
Your summary seems overall accurate to me.

----------------------------------------
Bug #18751: Regression on master for Method#== when comparing public with private method
https://bugs.ruby-lang.org/issues/18751#change-97620

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.2.0dev (2022-04-23T02:59:20Z master e142bea799) [x86_64-linux]
* Backport: 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED
----------------------------------------
This script repros:
```ruby
class C
  class << self
    alias_method :n, :new
    private :new
  end
end

p C.method(:n) == C.method(:new) # => true

puts
p C.method(:n) == Class.method(:new) # => false
p C.method(:n) == Class.method(:new).unbind.bind(C) # => true

puts
p C.method(:new) == Class.method(:new) # => false
p C.method(:new) == Class.method(:new).unbind.bind(C) # => true, BUT false on master
p C.method(:new) == Class.instance_method(:new).bind(C) # => true, BUT false on master
p [C.method(:new), Class.instance_method(:new).bind(C)] # => [#<Method: #<Class:C>(Class)#new(*)>, #<Method: #<Class:C>(Class)#new(*)>]
```

So this prints the expected results on 2.7.5, 3.0.3, 3.1.1 but not on master, which seems a regression.
Notably this breaks the pattern discussed in https://bugs.ruby-lang.org/issues/18729#note-5, and it means there is no way to find out if two methods share the same "definition/logic/def", which is a big limitation.



-- 
https://bugs.ruby-lang.org/

[ruby-core:108596] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

[jeremyevans0 (Jeremy Evans)]

Issue #18751 has been updated by jeremyevans0 (Jeremy Evans).


mame (Yusuke Endoh) wrote in #note-7:
> Let me confirm the current situation:
> 
> * Both #18435 and #18729 focus on the same issue (an inconsistency due to the fact that a Method object skips ZSUPER method entry)
> * In #18435, the visibility information is now stored in a Method object to hide the inconsistency
> * In #18729, we determined to allow a Method object for ZSUPER method entry to fix the inconsistency fundamentally
> * In this ticket, Method#== has an incompatibility isue because it respects method visibility information stored in a Method object.
> 
> Right?

I think that is a good summary.

> Now, I wonder if it is really needed to store the visibility information in a Method object. Will just reverting 58dc8bf8f15df9a33d191074e8a5d4946a3d59d5 solve this issue?

I don't think reverting that commit will fix the issue.  The `rb_method_entry_t*` in the Method object still points to the original method, not the ZSUPER method.  So if you reverted the commit, the visibility information would be wrong.

If you want to revert commit:58dc8bf8f15df9a33d191074e8a5d4946a3d59d5, you would have to make it so the `rb_method_entry_t*` points to the ZSUPER method.  I'm not against that approach, but I don't understand the code well enough to know whether it will cause problems.  Lacking a detailed understanding of why the code is the way it is, I made the assumption that there is a reason the `rb_method_entry_t*` points to the original method, and I took the conservative approach of just adding visibility information without changing other internals.

----------------------------------------
Bug #18751: Regression on master for Method#== when comparing public with private method
https://bugs.ruby-lang.org/issues/18751#change-97632

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.2.0dev (2022-04-23T02:59:20Z master e142bea799) [x86_64-linux]
* Backport: 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED
----------------------------------------
This script repros:
```ruby
class C
  class << self
    alias_method :n, :new
    private :new
  end
end

p C.method(:n) == C.method(:new) # => true

puts
p C.method(:n) == Class.method(:new) # => false
p C.method(:n) == Class.method(:new).unbind.bind(C) # => true

puts
p C.method(:new) == Class.method(:new) # => false
p C.method(:new) == Class.method(:new).unbind.bind(C) # => true, BUT false on master
p C.method(:new) == Class.instance_method(:new).bind(C) # => true, BUT false on master
p [C.method(:new), Class.instance_method(:new).bind(C)] # => [#<Method: #<Class:C>(Class)#new(*)>, #<Method: #<Class:C>(Class)#new(*)>]
```

So this prints the expected results on 2.7.5, 3.0.3, 3.1.1 but not on master, which seems a regression.
Notably this breaks the pattern discussed in https://bugs.ruby-lang.org/issues/18729#note-5, and it means there is no way to find out if two methods share the same "definition/logic/def", which is a big limitation.



-- 
https://bugs.ruby-lang.org/

[ruby-core:108605] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

[mame (Yusuke Endoh)]

Issue #18751 has been updated by mame (Yusuke Endoh).


jeremyevans0 (Jeremy Evans) wrote in #note-10:
> you would have to make it so the `rb_method_entry_t*` points to the ZSUPER method.

Yes. I meant it by "we determined to allow a Method object for ZSUPER method entry".

> Lacking a detailed understanding of why the code is the way it is, I made the assumption that there is a reason the `rb_method_entry_t*` points to the original method,

I think the original author (matz? nobu? Or ko1? I don't know) wanted to hide ZSUPER method entry from users because it is an implementation detail. However, #18729 showed that the hiding was incomplete. At the previous dev meeting, we discussed whether the hiding is really needed, and agreed to stop hiding it (https://bugs.ruby-lang.org/issues/18729#note-6).

> and I took the conservative approach of just adding visibility information without changing other internals.

You are very thoughtful and wonderful! Thank you always.

----------------------------------------
Bug #18751: Regression on master for Method#== when comparing public with private method
https://bugs.ruby-lang.org/issues/18751#change-97642

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.2.0dev (2022-04-23T02:59:20Z master e142bea799) [x86_64-linux]
* Backport: 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED
----------------------------------------
This script repros:
```ruby
class C
  class << self
    alias_method :n, :new
    private :new
  end
end

p C.method(:n) == C.method(:new) # => true

puts
p C.method(:n) == Class.method(:new) # => false
p C.method(:n) == Class.method(:new).unbind.bind(C) # => true

puts
p C.method(:new) == Class.method(:new) # => false
p C.method(:new) == Class.method(:new).unbind.bind(C) # => true, BUT false on master
p C.method(:new) == Class.instance_method(:new).bind(C) # => true, BUT false on master
p [C.method(:new), Class.instance_method(:new).bind(C)] # => [#<Method: #<Class:C>(Class)#new(*)>, #<Method: #<Class:C>(Class)#new(*)>]
```

So this prints the expected results on 2.7.5, 3.0.3, 3.1.1 but not on master, which seems a regression.
Notably this breaks the pattern discussed in https://bugs.ruby-lang.org/issues/18729#note-5, and it means there is no way to find out if two methods share the same "definition/logic/def", which is a big limitation.



-- 
https://bugs.ruby-lang.org/

[ruby-core:109460] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

[Eregon (Benoit Daloze)]

Issue #18751 has been updated by Eregon (Benoit Daloze).


I still think we should stop hiding ZSUPER method entries, it causes way more confusion than it helps and this bug, and I believe it would cause very little incompatibility.
@jeremyevans0 or @mame Would you be interested to work on that change? Otherwise I'll give it a try when I have some time.

----------------------------------------
Bug #18751: Regression on master for Method#== when comparing public with private method
https://bugs.ruby-lang.org/issues/18751#change-98622

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.2.0dev (2022-04-23T02:59:20Z master e142bea799) [x86_64-linux]
* Backport: 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED
----------------------------------------
This script repros:
```ruby
class C
  class << self
    alias_method :n, :new
    private :new
  end
end

p C.method(:n) == C.method(:new) # => true

puts
p C.method(:n) == Class.method(:new) # => false
p C.method(:n) == Class.method(:new).unbind.bind(C) # => true

puts
p C.method(:new) == Class.method(:new) # => false
p C.method(:new) == Class.method(:new).unbind.bind(C) # => true, BUT false on master
p C.method(:new) == Class.instance_method(:new).bind(C) # => true, BUT false on master
p [C.method(:new), Class.instance_method(:new).bind(C)] # => [#<Method: #<Class:C>(Class)#new(*)>, #<Method: #<Class:C>(Class)#new(*)>]
```

So this prints the expected results on 2.7.5, 3.0.3, 3.1.1 but not on master, which seems a regression.
Notably this breaks the pattern discussed in https://bugs.ruby-lang.org/issues/18729#note-5, and it means there is no way to find out if two methods share the same "definition/logic/def", which is a big limitation.



-- 
https://bugs.ruby-lang.org/

[ruby-core:109486] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

[Eregon (Benoit Daloze)]

Issue #18751 has been updated by Eregon (Benoit Daloze).


Here is a simplified script which does not depend on Class.new == Class#new:
```ruby
class C
  class << self
    alias_method :n, :new
    private :new
  end
end

p C.method(:n) == C.method(:new) # => true

puts
p C.method(:n) == Class.method(:new) # => false
p C.method(:n) == Class.instance_method(:new).bind(C) # => true

puts
p C.method(:new) == Class.method(:new) # => false
p C.method(:new) == Class.instance_method(:new).bind(C) # => true, BUT false on master
p [C.method(:new), Class.instance_method(:new).bind(C)] # => [#<Method: #<Class:C>(Class)#new(*)>, #<Method: #<Class:C>(Class)#new(*)>]
```

Based on this PR to fix this by no longer hiding ZSUPER methods: https://github.com/ruby/ruby/pull/6242,
I think we should add a new method on Method and UnboundMethod to check if they have the same definition.
Because otherwise naturally a method with a different visibility is a separate method, and making them the same with == seems wrong.

Maybe simply `{Method,UnboundMethod}#same_definition?(other)`.
I would also allow either Method or UnboundMethod as the argument, so there is no need to bind/unbind to compare.

----------------------------------------
Bug #18751: Regression on master for Method#== when comparing public with private method
https://bugs.ruby-lang.org/issues/18751#change-98654

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.2.0dev (2022-04-23T02:59:20Z master e142bea799) [x86_64-linux]
* Backport: 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED
----------------------------------------
This script repros:
```ruby
class C
  class << self
    alias_method :n, :new
    private :new
  end
end

p C.method(:n) == C.method(:new) # => true

puts
p C.method(:n) == Class.method(:new) # => false
p C.method(:n) == Class.method(:new).unbind.bind(C) # => true

puts
p C.method(:new) == Class.method(:new) # => false
p C.method(:new) == Class.method(:new).unbind.bind(C) # => true, BUT false on master
p C.method(:new) == Class.instance_method(:new).bind(C) # => true, BUT false on master
p [C.method(:new), Class.instance_method(:new).bind(C)] # => [#<Method: #<Class:C>(Class)#new(*)>, #<Method: #<Class:C>(Class)#new(*)>]
```

So this prints the expected results on 2.7.5, 3.0.3, 3.1.1 but not on master, which seems a regression.
Notably this breaks the pattern discussed in https://bugs.ruby-lang.org/issues/18729#note-5, and it means there is no way to find out if two methods share the same "definition/logic/def", which is a big limitation.



-- 
https://bugs.ruby-lang.org/

[ruby-core:109487] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

[Eregon (Benoit Daloze)]

Issue #18751 has been updated by Eregon (Benoit Daloze).


https://github.com/ruby/ruby/pull/6242 now makes resolved-through-zsuper methods equal for compatibility.
That means this issue is fixed by that PR too.

It's maybe a little bit strange that they are equal, but until we have a new method such as `{Method,UnboundMethod}#same_definition?(other)` this seems best for compatibility and generally honors the same behavior as previous versions which resolved ZSUPER methods in `Kernel#method`/`Module#instance_method` and now in `==`.

In fact the docs of Method#== do hint at definition equality:
```
Two method objects are equal if they are bound to the same object and
refer to the same method definition and the classes defining the methods
are the same class or module.
```

If ZSUPER methods are ever removed, this can all be simplified quite significantly.

----------------------------------------
Bug #18751: Regression on master for Method#== when comparing public with private method
https://bugs.ruby-lang.org/issues/18751#change-98655

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.2.0dev (2022-04-23T02:59:20Z master e142bea799) [x86_64-linux]
* Backport: 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED
----------------------------------------
This script repros:
```ruby
class C
  class << self
    alias_method :n, :new
    private :new
  end
end

p C.method(:n) == C.method(:new) # => true

puts
p C.method(:n) == Class.method(:new) # => false
p C.method(:n) == Class.method(:new).unbind.bind(C) # => true

puts
p C.method(:new) == Class.method(:new) # => false
p C.method(:new) == Class.method(:new).unbind.bind(C) # => true, BUT false on master
p C.method(:new) == Class.instance_method(:new).bind(C) # => true, BUT false on master
p [C.method(:new), Class.instance_method(:new).bind(C)] # => [#<Method: #<Class:C>(Class)#new(*)>, #<Method: #<Class:C>(Class)#new(*)>]
```

So this prints the expected results on 2.7.5, 3.0.3, 3.1.1 but not on master, which seems a regression.
Notably this breaks the pattern discussed in https://bugs.ruby-lang.org/issues/18729#note-5, and it means there is no way to find out if two methods share the same "definition/logic/def", which is a big limitation.



-- 
https://bugs.ruby-lang.org/

[ruby-core:109540] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

[Eregon (Benoit Daloze)]

Issue #18751 has been updated by Eregon (Benoit Daloze).


We discussed this at the dev meeting.
@ko1 said Method#== should be "is it the same definition?".
I agree, I'll take a look at this with #18729 and #18435.

----------------------------------------
Bug #18751: Regression on master for Method#== when comparing public with private method
https://bugs.ruby-lang.org/issues/18751#change-98711

* Author: Eregon (Benoit Daloze)
* Status: Open
* Priority: Normal
* ruby -v: ruby 3.2.0dev (2022-04-23T02:59:20Z master e142bea799) [x86_64-linux]
* Backport: 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED
----------------------------------------
This script repros:
```ruby
class C
  class << self
    alias_method :n, :new
    private :new
  end
end

p C.method(:n) == C.method(:new) # => true

puts
p C.method(:n) == Class.method(:new) # => false
p C.method(:n) == Class.method(:new).unbind.bind(C) # => true

puts
p C.method(:new) == Class.method(:new) # => false
p C.method(:new) == Class.method(:new).unbind.bind(C) # => true, BUT false on master
p C.method(:new) == Class.instance_method(:new).bind(C) # => true, BUT false on master
p [C.method(:new), Class.instance_method(:new).bind(C)] # => [#<Method: #<Class:C>(Class)#new(*)>, #<Method: #<Class:C>(Class)#new(*)>]
```

So this prints the expected results on 2.7.5, 3.0.3, 3.1.1 but not on master, which seems a regression.
Notably this breaks the pattern discussed in https://bugs.ruby-lang.org/issues/18729#note-5, and it means there is no way to find out if two methods share the same "definition/logic/def", which is a big limitation.



-- 
https://bugs.ruby-lang.org/

[ruby-core:109589] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

[Eregon (Benoit Daloze)]

Issue #18751 has been updated by Eregon (Benoit Daloze).

Assignee set to Eregon (Benoit Daloze)
Status changed from Open to Closed

Fixed in https://github.com/ruby/ruby/pull/6242

----------------------------------------
Bug #18751: Regression on master for Method#== when comparing public with private method
https://bugs.ruby-lang.org/issues/18751#change-98766

* Author: Eregon (Benoit Daloze)
* Status: Closed
* Priority: Normal
* Assignee: Eregon (Benoit Daloze)
* ruby -v: ruby 3.2.0dev (2022-04-23T02:59:20Z master e142bea799) [x86_64-linux]
* Backport: 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED
----------------------------------------
This script repros:
```ruby
class C
  class << self
    alias_method :n, :new
    private :new
  end
end

p C.method(:n) == C.method(:new) # => true

puts
p C.method(:n) == Class.method(:new) # => false
p C.method(:n) == Class.method(:new).unbind.bind(C) # => true

puts
p C.method(:new) == Class.method(:new) # => false
p C.method(:new) == Class.method(:new).unbind.bind(C) # => true, BUT false on master
p C.method(:new) == Class.instance_method(:new).bind(C) # => true, BUT false on master
p [C.method(:new), Class.instance_method(:new).bind(C)] # => [#<Method: #<Class:C>(Class)#new(*)>, #<Method: #<Class:C>(Class)#new(*)>]
```

So this prints the expected results on 2.7.5, 3.0.3, 3.1.1 but not on master, which seems a regression.
Notably this breaks the pattern discussed in https://bugs.ruby-lang.org/issues/18729#note-5, and it means there is no way to find out if two methods share the same "definition/logic/def", which is a big limitation.



-- 
https://bugs.ruby-lang.org/

[ruby-core:109591] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

[Eregon (Benoit Daloze)]

Issue #18751 has been updated by Eregon (Benoit Daloze).


Eregon (Benoit Daloze) wrote in #note-16:
> @ko1 said Method#== should be "is it the same definition?".
> I agree, I'll take a look at this with #18729 and #18435.

I did this for zsuper methods (resolve before comparing them for compatibility).
But not for other cases, I'll opened a separate issue for that: #18969

----------------------------------------
Bug #18751: Regression on master for Method#== when comparing public with private method
https://bugs.ruby-lang.org/issues/18751#change-98767

* Author: Eregon (Benoit Daloze)
* Status: Closed
* Priority: Normal
* Assignee: Eregon (Benoit Daloze)
* ruby -v: ruby 3.2.0dev (2022-04-23T02:59:20Z master e142bea799) [x86_64-linux]
* Backport: 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED
----------------------------------------
This script repros:
```ruby
class C
  class << self
    alias_method :n, :new
    private :new
  end
end

p C.method(:n) == C.method(:new) # => true

puts
p C.method(:n) == Class.method(:new) # => false
p C.method(:n) == Class.method(:new).unbind.bind(C) # => true

puts
p C.method(:new) == Class.method(:new) # => false
p C.method(:new) == Class.method(:new).unbind.bind(C) # => true, BUT false on master
p C.method(:new) == Class.instance_method(:new).bind(C) # => true, BUT false on master
p [C.method(:new), Class.instance_method(:new).bind(C)] # => [#<Method: #<Class:C>(Class)#new(*)>, #<Method: #<Class:C>(Class)#new(*)>]
```

So this prints the expected results on 2.7.5, 3.0.3, 3.1.1 but not on master, which seems a regression.
Notably this breaks the pattern discussed in https://bugs.ruby-lang.org/issues/18729#note-5, and it means there is no way to find out if two methods share the same "definition/logic/def", which is a big limitation.



-- 
https://bugs.ruby-lang.org/

[ruby-core:111107] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

[matz (Yukihiro Matsumoto)]

Issue #18751 has been updated by matz (Yukihiro Matsumoto).


This should be fixed by #18798 which is accepted.

Matz.


----------------------------------------
Bug #18751: Regression on master for Method#== when comparing public with private method
https://bugs.ruby-lang.org/issues/18751#change-100377

* Author: Eregon (Benoit Daloze)
* Status: Closed
* Priority: Normal
* Assignee: Eregon (Benoit Daloze)
* ruby -v: ruby 3.2.0dev (2022-04-23T02:59:20Z master e142bea799) [x86_64-linux]
* Backport: 2.7: DONTNEED, 3.0: DONTNEED, 3.1: DONTNEED
----------------------------------------
This script repros:
```ruby
class C
  class << self
    alias_method :n, :new
    private :new
  end
end

p C.method(:n) == C.method(:new) # => true

puts
p C.method(:n) == Class.method(:new) # => false
p C.method(:n) == Class.method(:new).unbind.bind(C) # => true

puts
p C.method(:new) == Class.method(:new) # => false
p C.method(:new) == Class.method(:new).unbind.bind(C) # => true, BUT false on master
p C.method(:new) == Class.instance_method(:new).bind(C) # => true, BUT false on master
p [C.method(:new), Class.instance_method(:new).bind(C)] # => [#<Method: #<Class:C>(Class)#new(*)>, #<Method: #<Class:C>(Class)#new(*)>]
```

So this prints the expected results on 2.7.5, 3.0.3, 3.1.1 but not on master, which seems a regression.
Notably this breaks the pattern discussed in https://bugs.ruby-lang.org/issues/18729#note-5, and it means there is no way to find out if two methods share the same "definition/logic/def", which is a big limitation.



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/