Skip to content

Commit df2d787

Browse files
committed
Update Message_index to account for oneof's
This is mostly patterned off of `Message_method_missing`, but that function does a bunch of extra stuff (to handle, e.g., `has_` and `clear_` and setter methods), while `Message_index` only needs to handle getter methods. So instead of copying the `Message_method_missing` implementation verbatim, we can "constant fold" away the parts of the method that we know cannot (or should not) happen for `msg[...]` calls.
1 parent 640a776 commit df2d787

File tree

2 files changed

+11
-4
lines changed

2 files changed

+11
-4
lines changed

ruby/ext/google/protobuf_c/message.c

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -890,16 +890,23 @@ VALUE Message_freeze(VALUE _self) {
890890
*/
891891
static VALUE Message_index(VALUE _self, VALUE field_name) {
892892
Message* self = ruby_to_Message(_self);
893+
const upb_OneofDef* o;
893894
const upb_FieldDef* f;
894895

895896
Check_Type(field_name, T_STRING);
896-
f = upb_MessageDef_FindFieldByName(self->msgdef, RSTRING_PTR(field_name));
897+
const char* name = RSTRING_PTR(field_name);
898+
size_t sn = strlen(name);
897899

898-
if (f == NULL) {
900+
if (!upb_MessageDef_FindByNameWithSize(self->msgdef, name, sn, &f, &o)) {
899901
return Qnil;
900902
}
901903

902-
return Message_getfield(_self, f);
904+
// Dispatch accessor.
905+
if (o != NULL) {
906+
return Message_oneof_accessor(_self, o, METHOD_GETTER);
907+
} else {
908+
return Message_getfield(_self, f);
909+
}
903910
}
904911

905912
/*

ruby/tests/basic.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -800,7 +800,7 @@ def test_oneof_fields_index
800800

801801
assert_equal :a, msg.my_oneof
802802
assert_equal :a, msg.method_missing(:my_oneof)
803-
assert_equal nil, msg["my_oneof"]
803+
assert_equal :a, msg["my_oneof"]
804804
end
805805

806806
def test_string_subclass

0 commit comments

Comments
 (0)