From 74f0d96a2c0b29cbe0876de773d2ee588db6fc1b Mon Sep 17 00:00:00 2001 From: Masaki Hara Date: Wed, 15 Apr 2020 16:08:14 +0900 Subject: [PATCH] Ruby: encode repeated field in packed format if desired --- ruby/ext/google/protobuf_c/defs.c | 32 +++++++++++++------ ruby/tests/basic.rb | 10 ++++++ ruby/tests/basic_proto2.rb | 10 ++++++ ruby/tests/basic_test.proto | 20 ++++++++++++ ruby/tests/basic_test_proto2.proto | 20 ++++++++++++ .../protobuf/compiler/ruby/ruby_generator.cc | 5 +++ 6 files changed, 88 insertions(+), 9 deletions(-) diff --git a/ruby/ext/google/protobuf_c/defs.c b/ruby/ext/google/protobuf_c/defs.c index babdfa97115fd..abf1912351387 100644 --- a/ruby/ext/google/protobuf_c/defs.c +++ b/ruby/ext/google/protobuf_c/defs.c @@ -1513,6 +1513,17 @@ static void msgdef_add_field(VALUE msgbuilder_rb, upb_label_t label, VALUE name, field_proto, FileBuilderContext_strdup(self->file_builder, default_value)); } + + if (rb_funcall(options, rb_intern("key?"), 1, + ID2SYM(rb_intern("packed"))) == Qtrue) { + VALUE packed = + rb_hash_lookup(options, ID2SYM(rb_intern("packed"))); + google_protobuf_FieldOptions *options = + google_protobuf_FieldDescriptorProto_mutable_options( + field_proto, + file_context->arena); + google_protobuf_FieldOptions_set_packed(options, RTEST(packed)); + } } if (oneof_index >= 0) { @@ -1614,7 +1625,8 @@ VALUE MessageBuilderContext_required(int argc, VALUE* argv, VALUE _self) { /* * call-seq: - * MessageBuilderContext.repeated(name, type, number, type_class = nil) + * MessageBuilderContext.repeated(name, type, number, type_class = nil, + * options = nil) * * Defines a new repeated field on this message type with the given type, tag * number, and type class (for message and enum fields). The type must be a Ruby @@ -1622,18 +1634,20 @@ VALUE MessageBuilderContext_required(int argc, VALUE* argv, VALUE _self) { * string, if present (as accepted by FieldDescriptor#submsg_name=). */ VALUE MessageBuilderContext_repeated(int argc, VALUE* argv, VALUE _self) { - VALUE name, type, number, type_class; + VALUE name, type, number; + VALUE type_class = Qnil, options = Qnil; + + rb_scan_args(argc, argv, "32", &name, &type, &number, &type_class, &options); - if (argc < 3) { - rb_raise(rb_eArgError, "Expected at least 3 arguments."); + // Allow passing (name, type, number, options) or + // (name, type, number, type_class, options) + if (argc == 4 && RB_TYPE_P(type_class, T_HASH)) { + options = type_class; + type_class = Qnil; } - name = argv[0]; - type = argv[1]; - number = argv[2]; - type_class = (argc > 3) ? argv[3] : Qnil; msgdef_add_field(_self, UPB_LABEL_REPEATED, name, type, number, type_class, - Qnil, -1); + options, -1); return Qnil; } diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index d1a66a6169c33..e769f83735d5e 100755 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -521,5 +521,15 @@ def test_map_freeze assert_raise(FrozenErrorType) { m.map_string_int32.delete('a') } assert_raise(FrozenErrorType) { m.map_string_int32.clear } end + + def test_packed_unpacked + m = proto_module::PackedUnpackedMessage.new + m.packed_repeated_int32.replace([42, 84, 300]) + assert_equal proto_module::PackedUnpackedMessage.encode(m), "\x0A\x04\x2A\x54\xAC\x02".b + + m = proto_module::PackedUnpackedMessage.new + m.repeated_int32.replace([42, 84, 300]) + assert_equal proto_module::PackedUnpackedMessage.encode(m), "\x58\x2A\x58\x54\x58\xAC\x02".b + end end end diff --git a/ruby/tests/basic_proto2.rb b/ruby/tests/basic_proto2.rb index 4c7ddd55b981f..2bcc83409ca07 100755 --- a/ruby/tests/basic_proto2.rb +++ b/ruby/tests/basic_proto2.rb @@ -260,5 +260,15 @@ def test_file_descriptor assert_equal "tests/basic_test_proto2.proto", file_descriptor.name assert_equal :proto2, file_descriptor.syntax end + + def test_packed_unpacked + m = proto_module::PackedUnpackedMessage.new + m.packed_repeated_int32.replace([42, 84, 300]) + assert_equal proto_module::PackedUnpackedMessage.encode(m), "\x0A\x04\x2A\x54\xAC\x02".b + + m = proto_module::PackedUnpackedMessage.new + m.repeated_int32.replace([42, 84, 300]) + assert_equal proto_module::PackedUnpackedMessage.encode(m), "\x58\x2A\x58\x54\x58\xAC\x02".b + end end end diff --git a/ruby/tests/basic_test.proto b/ruby/tests/basic_test.proto index a91854007535f..10d53da87d9d6 100644 --- a/ruby/tests/basic_test.proto +++ b/ruby/tests/basic_test.proto @@ -192,3 +192,23 @@ message MyStruct { string string = 1; google.protobuf.Struct struct = 2; } + +message PackedUnpackedMessage { + repeated int32 packed_repeated_int32 = 1; + repeated int64 packed_repeated_int64 = 2; + repeated uint32 packed_repeated_uint32 = 3; + repeated uint64 packed_repeated_uint64 = 4; + repeated bool packed_repeated_bool = 5; + repeated float packed_repeated_float = 6; + repeated double packed_repeated_double = 7; + repeated TestEnum packed_repeated_enum = 8; + + repeated int32 repeated_int32 = 11 [packed=false]; + repeated int64 repeated_int64 = 12 [packed=false]; + repeated uint32 repeated_uint32 = 13 [packed=false]; + repeated uint64 repeated_uint64 = 14 [packed=false]; + repeated bool repeated_bool = 15 [packed=false]; + repeated float repeated_float = 16 [packed=false]; + repeated double repeated_double = 17 [packed=false]; + repeated TestEnum repeated_enum = 18 [packed=false]; +} diff --git a/ruby/tests/basic_test_proto2.proto b/ruby/tests/basic_test_proto2.proto index 0c1a2b98363f0..d4cfc522eee7a 100644 --- a/ruby/tests/basic_test_proto2.proto +++ b/ruby/tests/basic_test_proto2.proto @@ -187,3 +187,23 @@ message MyStruct { optional string string = 1; optional google.protobuf.Struct struct = 2; } + +message PackedUnpackedMessage { + repeated int32 packed_repeated_int32 = 1 [packed=true]; + repeated int64 packed_repeated_int64 = 2 [packed=true]; + repeated uint32 packed_repeated_uint32 = 3 [packed=true]; + repeated uint64 packed_repeated_uint64 = 4 [packed=true]; + repeated bool packed_repeated_bool = 5 [packed=true]; + repeated float packed_repeated_float = 6 [packed=true]; + repeated double packed_repeated_double = 7 [packed=true]; + repeated TestEnum packed_repeated_enum = 8 [packed=true]; + + repeated int32 repeated_int32 = 11; + repeated int64 repeated_int64 = 12; + repeated uint32 repeated_uint32 = 13; + repeated uint64 repeated_uint64 = 14; + repeated bool repeated_bool = 15; + repeated float repeated_float = 16; + repeated double repeated_double = 17; + repeated TestEnum repeated_enum = 18; +} diff --git a/src/google/protobuf/compiler/ruby/ruby_generator.cc b/src/google/protobuf/compiler/ruby/ruby_generator.cc index 8c0aed1c71b1e..f614679eafe96 100644 --- a/src/google/protobuf/compiler/ruby/ruby_generator.cc +++ b/src/google/protobuf/compiler/ruby/ruby_generator.cc @@ -216,6 +216,11 @@ void GenerateField(const FieldDescriptor* field, io::Printer* printer) { DefaultValueForField(field)); } + if (field->is_packable()) { + printer->Print(", packed: $packed$", "packed", + field->is_packed() ? "true" : "false"); + } + printer->Print("\n"); } }