Skip to content

Fix conversion of singleton classes in Ruby#9342

Merged
acozzette merged 2 commits intoprotocolbuffers:masterfrom
antstorm:singleton-string
Feb 11, 2022
Merged

Fix conversion of singleton classes in Ruby#9342
acozzette merged 2 commits intoprotocolbuffers:masterfrom
antstorm:singleton-string

Conversation

@antstorm
Copy link
Contributor

@antstorm antstorm commented Dec 24, 2021

I found a nasty issue when passing an object who's singleton class was accessed as a Protobuf field value. In my case I was using a string and got this cryptic message:

Invalid argument for string field 'name' (given String). (Google::Protobuf::TypeError)

After digging deeper I've discovered that Ruby has a weird way of representing a singleton class (also known as eigenclass in Ruby) — basically once it was accessed via the Object#singleton_class method the true class of an object becomes a wrapper (such as #<Class:#<String:0x00000001168372e8>> in case of a string).

This is normally an issue because all the class accessor methods in Ruby take this into account and unwrap the real class, so it's not even noticeable. However when using Ruby C API's CLASS_OF() method, it shows the wrapper class and therefore is not very usable with wrapped classes. Instead a rb_obj_class should be used, which will correctly account for wrapped classes. More details (as well as an example) can be found here — https://bugs.ruby-lang.org/issues/18428.

The error itself is also pretty confusing because it's using rb_class2name() which is wrapper aware and shows the unwrapped class (which is not the same one that was used in the conditional).

Note: I've included a test which should replicate this issue, however I wasn't able to run rake test locally. There seems to be a big circular dependency issue between ruby/lib/google/protobuf/descriptor_dsl.rb and ruby/lib/google/protobuf/descriptor_pb.rb (DSL depends on the generated proto, which itself is built using the DSL). Not sure how it was supposed to work. Hoping on the CI run to make sure everything is good.

@acozzette acozzette merged commit 2ce9604 into protocolbuffers:master Feb 11, 2022
@antstorm antstorm deleted the singleton-string branch February 14, 2022 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants