From: "Eregon (Benoit Daloze)" <noreply@...>
Date: 2022-09-28T16:26:43+00:00
Subject: [ruby-core:110132] [Ruby master Bug#18729] Method#owner and UnboundMethod#owner are incorrect after using Module#public/protected/private

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


@matz
`(a)` should be `p C2.private_instance_methods` (.instance_methods doesn't show private methods).
Then on all versions `foo` is included.
And of course that means it should be possible to retrieve that method with `Kernel#method`/`Module#instance_method`, which became possible since my change.

> As a general principle, retrieving a method object should keep the current behavior

We agree on this.
However, the only way to do that reliably is to remove ZSUPER methods altogether (https://github.com/ruby/ruby/pull/6251).
That removes a lot of complexity and confusion, for a feature nobody needs.
Then it's clear a Method/UnboundMethod points to a specific method definition, and we never have all these subtle edge cases to reason about. Finally we'll have a clear and understandable meta view for methods in Ruby.

The previous logic of skipping ZSUPER methods in `Kernel#method` and `Module#instance_method` just works around the problem and confuses everybody, in addition to revealing inconsistent `owner`, visibility, etc.

Also skipping ZSUPER methods would again introduce issues with `Method#public?`, etc (#11689).
And in 3.1 those methods exist, and ZSUPER methods are now revealed on the 3.1 branch (https://bugs.ruby-lang.org/issues/18435#note-25), which means those methods behave correctly, as expected and intuitively.
So it seems unacceptable to skip ZSUPER methods entirely in `Kernel#method`/`Module#instance_method`, it's just ignoring the fundamental issue.

To compare, the behavior of removing ZSUPER methods (https://github.com/ruby/ruby/pull/6251) and TruffleRuby is:
```
[:foo, :bar]
C1
[:foo, :bar]
C2
[:foo, :bar]
C2
```
That is, a Method/UnboundMethod always points to a specific method entry, and a Method/UnboundMethod actually captures the state when it was created.

In any case, I think the behavior for such a corner case that nobody uses in practice does not matter much, the general semantics and understandability of the model seem far more important.

So I think we have 3 choices:
* Trying to give the correct owner and in `Kernel#method`/`Module#instance_method` already lookup the method entry the ZSUPER method would resolve to. I will give it a try, but it feels brittle and as already tried for the visibility by Jeremy will likely result in some weird inconsistencies and more complexity in the implementation where some of the Method state is overridden by extra fields and some not. But maybe it's good enough.
* Keep as it is on current master, the overall semantics are good, it's only surprising for edge cases nobody uses (redefining a method after changing visibility in subclass and using Method/UnboundMethod objects created before that and wanting the old behavior for that case)
* Remove ZSUPER methods (https://github.com/ruby/ruby/pull/6251). That simplifies everything tremendously and results in better performance.

The last one is IMHO by far the better and safest option, everything is consistent and simple.

----------------------------------------
Bug #18729: Method#owner and UnboundMethod#owner are incorrect after using Module#public/protected/private
https://bugs.ruby-lang.org/issues/18729#change-99390

* Author: Eregon (Benoit Daloze)
* Status: Closed
* Priority: Normal
* Assignee: Eregon (Benoit Daloze)
* ruby -v: ruby 3.1.1p18 (2022-02-18 revision 53f5fc4236) [x86_64-linux]
* Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN
----------------------------------------
The #owner should be "the class or module that defines the method".
Or in other words, the owner is the module which has the method table containing that method.

This generally holds, and it seems very likely this assumption is relied upon (e.g., when decorating a method, undefining it, etc).
But the returned value on CRuby is incorrect for this case:
```ruby
class A
  protected def foo
    :A
  end
end

class B < A
  p [instance_method(:foo), instance_method(:foo).owner, instance_methods(false), A.instance_methods(false)]
  public :foo
  p [instance_method(:foo), instance_method(:foo).owner, instance_methods(false), A.instance_methods(false)]
end
```

It gives:
```
[#<UnboundMethod: B(A)#foo() owner.rb:2>, A, [], [:foo]]
[#<UnboundMethod: B(A)#foo() owner.rb:2>, A, [:foo], [:foo]]
```

So `UnboundMethod#owner` says `A`, but clearly there is a :foo method entry in B created by `public :foo`, and that is shown through `B.instance_methods(false)`.

The expected output is:

```
[#<UnboundMethod: B(A)#foo() owner.rb:2>, A, [], [:foo]]
[#<UnboundMethod: B#foo() owner.rb:2>, B, [:foo], [:foo]]
```



-- 
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>