Skip to content

Commit c49d529

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 d30f329 commit c49d529

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
@@ -895,16 +895,23 @@ VALUE Message_freeze(VALUE _self) {
895895
*/
896896
static VALUE Message_index(VALUE _self, VALUE field_name) {
897897
Message* self = ruby_to_Message(_self);
898+
const upb_OneofDef* o;
898899
const upb_FieldDef* f;
899900

900901
Check_Type(field_name, T_STRING);
901-
f = upb_MessageDef_FindFieldByName(self->msgdef, RSTRING_PTR(field_name));
902+
const char* name = RSTRING_PTR(field_name);
903+
size_t sn = strlen(name);
902904

903-
if (f == NULL) {
905+
if (!upb_MessageDef_FindByNameWithSize(self->msgdef, name, sn, &f, &o)) {
904906
return Qnil;
905907
}
906908

907-
return Message_getfield(_self, f);
909+
// Dispatch accessor.
910+
if (o != NULL) {
911+
return Message_oneof_accessor(_self, o, METHOD_GETTER);
912+
} else {
913+
return Message_getfield(_self, f);
914+
}
908915
}
909916

910917
/*

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)