Bug #18561: Make singleton def operation and define_singleton_method explicitly use public visibility

• Status: Closed
• Assignee: -
• Target version: -
• Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN, 3.1: UNKNOWN

[headius (Charles Nutter)]

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

[headius (Charles Nutter)]

@marcandre (Marc-Andre Lafortune) provided a possibly more common case, but I still highly doubt that anyone would expect this to behave differently than the unmatched scope version:

$ ruby -e 'class Foo; class << Foo; private; Foo.define_singleton_method(:foo){}; end; end; Foo.foo'
-e:1:in `<main>': private method `foo' called for Foo:Class (NoMethodError)

https://github.com/ruby/ostruct/issues/40#issuecomment-1026111591

[headius (Charles Nutter)]

Related JRuby pull request that makes singleton method definition always public (in response to the ostruct issue linked above):

https://github.com/jruby/jruby/pull/7055

[jeremyevans0 (Jeremy Evans)]

I couldn't get def $o.foo to have non-public visibility in any version of Ruby. I definitely don't think it is possible in the current code. Singleton method definitions use the definesmethod VM instruction, which calls vm_define_method with TRUE as the is_singleton argument, and vm_define_method always uses public visibility in this case.

From testing, the define_singleton_method visibility issue was introduced in Ruby 2.1:

$ ruby19 -ve 'class Foo; class << Foo; private; Foo.define_singleton_method(:foo){}; end; end; Foo.foo'
ruby 1.9.3p551 (2014-11-13 revision 48407) [x86_64-openbsd]

$ ruby20 -ve 'class Foo; class << Foo; private; Foo.define_singleton_method(:foo){}; end; end; Foo.foo'
ruby 2.0.0p648 (2015-12-16 revision 53162) [x86_64-openbsd]

$ ruby21 -ve 'class Foo; class << Foo; private; Foo.define_singleton_method(:foo){}; end; end; Foo.foo'
ruby 2.1.9p490 (2016-03-30 revision 54437) [x86_64-openbsd]
-e:1:in `<main>': private method `foo' called for Foo:Class (NoMethodError)

I submitted a pull request to fix the define_singleton_method visibility issue: https://github.com/ruby/ruby/pull/5636

[headius (Charles Nutter)]

Thank you @jeremyevans0 (Jeremy Evans) 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.

• Status changed from Open to Closed

Applied in changeset git|173a6b6a802d80b8cf200308fd3653832b700b1c.

---

Make define_singleton_method always define a public method

In very unlikely cases, it could previously define a non-public method
starting in Ruby 2.1.

Fixes [Bug #18561]