From: "jeremyevans0 (Jeremy Evans)" <noreply@...>
Date: 2022-05-17T15:41:18+00:00
Subject: [ruby-core:108596] [Ruby master Bug#18751] Regression on master for Method#== when comparing public with private method

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/

Unsubscribe: <mailto:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>