Feature #18552
closedExpose `VALUE rb_singleton_class_get(VALUE)` to extensions
Description
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¶
Updated by alanwu (Alan Wu) over 1 year ago
!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. 12 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.
Updated by byroot (Jean Boussier) over 1 year ago
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.
Updated by Eregon (Benoit Daloze) over 1 year ago
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);
Updated by byroot (Jean Boussier) over 1 year ago
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?
Updated by byroot (Jean Boussier) over 1 year ago
- 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.