[ruby-core:107925] [Ruby master Bug#18561] Make singleton def operation and define_singleton_method explicitly use public visibility

Issue #18561 has been updated by headius (Charles Nutter).


Thank you @jeremyevans0 for the analysis and PR. I agree that the one weird edge case would generally just be unexpected by a user and should be considered a bug.

----------------------------------------
Bug #18561: Make singleton def operation and define_singleton_method explicitly use public visibility
https://bugs.ruby-lang.org/issues/18561#change-96872

* Author: headius (Charles Nutter)
* Status: Open
* Priority: Normal
* Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN
----------------------------------------
Currently nearly all uses of `define_singleton_method` or `def obj.foo` will ignore the caller's frame/cref visibility and use the default visibility of public. I propose this be made explicit in the code, documentation, and ruby specs.

```
$ ruby -e 'class Foo; private; define_singleton_method(:foo) {p :ok}; end; Foo.foo'
:ok
$ ruby -e 'class Foo; private; def self.foo; p :ok; end; end; Foo.foo'
:ok
```

This works because the class in which the method is defined is nearly always different from the calling scope, since we are usually calling `define_singleton_method` against some other object. It "accidentally" ends up being public all the time, like `def self.foo`.

However, there is at least one (weird) edge case where the frame/cref visibility is honored:

```
$ ruby -e '$o = Object.new; class << $o; private; $o.define_singleton_method(:foo){}; end; $o.foo'
-e:1:in `<main>': private method `foo' called for #<Object:0x00007fcf0e00dc98> (NoMethodError)
```

This also works for `def $o.foo` but I would argue this is unexpected behavior in both cases. It is difficult to trigger, since you have to already be within the target singleton class body, and the "normal" behavior everywhere else is to ignore the frame/cref visibility.

It would not be difficult to make both forms always use public visibility:

* Split off the actual method-binding logic from `rb_mod_define_method` into a separate function `mod_define_method_internal` that takes a visibility parameter.
* Call that new method from `rb_mod_define_method` (with cref-based visibility calculation) and `rb_obj_define_method` (with explicit public visibility).



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

Unsubscribe: <mailto:[e-mail]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>