Feature #18552: Expose `VALUE rb_singleton_class_get(VALUE)` to extensions

• Status: Closed
• Priority: Normal
• Assignee: -
• Target version: -

[byroot (Jean Boussier)]

Right now the only way to check wether an object has a singleton class is to do something akin to:

!SPECIAL_CONST(obj) && FL_TEST(RBASIC(obj)->klass, FL_SINGLETON);

Which doesn't seem very clean.

Use case

This came up in msgpack. The library have a registry of serializers on a per class basis, and wish to support singleton classes too.
So it is calling rb_singleton_class() which cause lots of useless singleton classes to be created: https://github.com/msgpack/msgpack-ruby/pull/245

Proposed patch

https://github.com/ruby/ruby/pull/5499

[alanwu (Alan Wu)]

!SPECIAL_CONST(obj) && FL_TEST(RBASIC(obj)->klass, FL_SINGLETON);
Which does seem very clean.

Beyond cleanliness, it uses FL_SINGLETON, which is marked @internal. I
agree with @shyouhei (Shyouhei Urabe) that this flag should not be visible in the first place.
Also, it's tempting to think the predicate being true implies
RBASIC(obj)->klass gives the singleton class of obj while that is not
necessarily true and has lead to bugs in the past. ¹² These subtleties
are why it's generally a bad idea for extensions to take advantage of
implementation details.

I can emphasize with trying to avoid allocations, but I want to be cautious
about expanding the C extension API surface. In this case, it's exposing
yet another operation unavailable to plain Ruby code. It might make some
optimizations in JITs more complex to implement or worse, introduce bugs down
the line in the interpreter because C APIs are generally less frequently
exercised and their extra capabilities easy to be forgotten.

We should review how much rb_singleton_class_get() is exposing and not exposing, and
whether it's feasible to implement in other runtimes such as TruffleRuby.

1. https://bugs.ruby-lang.org/issues/17321 ↩

2. https://github.com/ruby/ruby/commit/0cd3b97e027332236625835578329580be12023c ↩

[byroot (Jean Boussier)]

• Description updated (diff)

[byroot (Jean Boussier)]

Which does seem very clean.

I of course meant "doesn't". Oops.

In this case, it's exposing yet another operation unavailable to plain Ruby code.

To be fair, I was entertaining the idea to also open a feature request to get a Ruby side API for it, e.g. Object#has_singletong_class?.

We should review how much rb_singleton_class_get() is exposing and not exposing

It really doesn't expose much, it's nearly the same code than rb_singleton_class except it returns Qnil instead of creating the singleton class on the fly.

whether it's feasible to implement in other runtimes such as TruffleRuby.

Not sure about TruffleRuby, but since msgpack has a JRuby extension, I also had to do this same there, and their API already allows to check for the singleton class existence without creating it.

[Eregon (Benoit Daloze)]

A new function seems better than that flag check (which doesn't work on TruffleRuby currently, will always return false).
The name rb_singleton_class_get() doesn't seem great though.

However I think there is already a way to check this, RBASIC_CLASS(obj)/rb_class_of(obj) already gets the singleton_class-if-there-is-one, or the same as .class, and there is Module#singleton_class? to find out.
The first is only available to C exts, the second doesn't seem to have a C API function.
But, this is an easy way in C:

bool has_singleton_class = rb_class_of(obj) != rb_obj_class(obj);

[byroot (Jean Boussier)]

The name rb_singleton_class_get() doesn't seem great though.

Yeah, it could definitely be expose under another name.

bool has_singleton_class = rb_class_of(obj) != rb_obj_class(obj);

I don't follow, how could this expression ever be false?

[byroot (Jean Boussier)]

• Status changed from Open to Closed

Nevermind, on first read I didn't notice that these were two distinct function. That's good enough I think, no need to expose a new function. Closing.