Skip to content

P
protobuf
Commit a2bea0a0 authored by Chris Fallin's avatar Chris Fallin
Browse files
Options

Properly support maps in Ruby protoc and another bugfix. 

Previously, we supported map fields in the Ruby DSL. However, we never
connected the final link in the chain and generated `map` DSL commands
for map fields in `.proto` files. My apologies -- I had been testing
with the DSL directly so I missed this.

Also fixed a handlerdata-setup-infinite-loop when a map value field's
type is its containing message.
parent 43beb626
Show whitespace changes
Inline Side-by-side
Showing with 63 additions and 73 deletions
ruby/ext/google/protobuf_c/encode_decode.c
View file @ a2bea0a0
......@@ -208,7 +208,7 @@ typedef struct {
size_t ofs;
upb_fieldtype_t key_field_type;
upb_fieldtype_t value_field_type;
VALUE value_field_typeclass;
const upb_def* value_field_subdef;
} map_handlerdata_t;
// Temporary frame for map parsing: at the beginning of a map entry message, a
......@@ -248,8 +248,15 @@ static bool endmap_handler(void *closure, const void *hd, upb_status* s) {
VALUE key = native_slot_get(
mapdata->key_field_type, Qnil,
&frame->key_storage);
VALUE value_field_typeclass = Qnil;
if (mapdata->value_field_type == UPB_TYPE_MESSAGE ||
mapdata->value_field_type == UPB_TYPE_ENUM) {
value_field_typeclass = get_def_obj(mapdata->value_field_subdef);
}
VALUE value = native_slot_get(
mapdata->value_field_type, mapdata->value_field_typeclass,
mapdata->value_field_type, value_field_typeclass,
&frame->value_storage);
Map_index_set(frame->map, key, value);
......@@ -280,17 +287,7 @@ static map_handlerdata_t* new_map_handlerdata(
MAP_VALUE_FIELD);
assert(value_field != NULL);
hd->value_field_type = upb_fielddef_type(value_field);
hd->value_field_typeclass = field_type_class(value_field);
// Ensure that value_field_typeclass is properly GC-rooted. We must do this
// because we hold a reference to the Ruby class in the handlerdata, which is
// owned by the handlers. The handlers are owned by *this* message's Ruby
// object, but each Ruby object is rooted independently at the def -> Ruby
// object map. So we have to ensure that the Ruby objects we depend on will
// stick around as long as we're around.
if (hd->value_field_typeclass != Qnil) {
rb_ary_push(desc->typeclass_references, hd->value_field_typeclass);
}
hd->value_field_subdef = upb_fielddef_subdef(value_field);
return hd;
}
......
ruby/tests/generated_code.rb
View file @ a2bea0a0
......@@ -27,16 +27,16 @@ Google::Protobuf::DescriptorPool.generated_pool.build do
repeated :repeated_bytes, :string, 29
repeated :repeated_enum, :enum, 30, "A.B.C.TestEnum"
repeated :repeated_msg, :message, 31, "A.B.C.TestMessage"
repeated :map_int32_string, :message, 61, "A.B.C.TestMessage.MapInt32StringEntry"
repeated :map_int64_string, :message, 62, "A.B.C.TestMessage.MapInt64StringEntry"
repeated :map_uint32_string, :message, 63, "A.B.C.TestMessage.MapUint32StringEntry"
repeated :map_uint64_string, :message, 64, "A.B.C.TestMessage.MapUint64StringEntry"
repeated :map_bool_string, :message, 65, "A.B.C.TestMessage.MapBoolStringEntry"
repeated :map_string_string, :message, 66, "A.B.C.TestMessage.MapStringStringEntry"
repeated :map_string_msg, :message, 67, "A.B.C.TestMessage.MapStringMsgEntry"
repeated :map_string_enum, :message, 68, "A.B.C.TestMessage.MapStringEnumEntry"
repeated :map_string_int32, :message, 69, "A.B.C.TestMessage.MapStringInt32Entry"
repeated :map_string_bool, :message, 70, "A.B.C.TestMessage.MapStringBoolEntry"
map :map_int32_string, :int32, :string, 61
map :map_int64_string, :int64, :string, 62
map :map_uint32_string, :uint32, :string, 63
map :map_uint64_string, :uint64, :string, 64
map :map_bool_string, :bool, :string, 65
map :map_string_string, :string, :string, 66
map :map_string_msg, :string, :message, 67, "A.B.C.TestMessage"
map :map_string_enum, :string, :enum, 68, "A.B.C.TestEnum"
map :map_string_int32, :string, :int32, 69
map :map_string_bool, :string, :bool, 70
optional :nested_message, :message, 80, "A.B.C.TestMessage.NestedMessage"
oneof :my_oneof do
optional :oneof_int32, :int32, 41
......@@ -52,46 +52,6 @@ Google::Protobuf::DescriptorPool.generated_pool.build do
optional :oneof_msg, :message, 51, "A.B.C.TestMessage"
end
end
add_message "A.B.C.TestMessage.MapInt32StringEntry" do
optional :key, :int32, 1
optional :value, :string, 2
end
add_message "A.B.C.TestMessage.MapInt64StringEntry" do
optional :key, :int64, 1
optional :value, :string, 2
end
add_message "A.B.C.TestMessage.MapUint32StringEntry" do
optional :key, :uint32, 1
optional :value, :string, 2
end
add_message "A.B.C.TestMessage.MapUint64StringEntry" do
optional :key, :uint64, 1
optional :value, :string, 2
end
add_message "A.B.C.TestMessage.MapBoolStringEntry" do
optional :key, :bool, 1
optional :value, :string, 2
end
add_message "A.B.C.TestMessage.MapStringStringEntry" do
optional :key, :string, 1
optional :value, :string, 2
end
add_message "A.B.C.TestMessage.MapStringMsgEntry" do
optional :key, :string, 1
optional :value, :message, 2, "A.B.C.TestMessage"
end
add_message "A.B.C.TestMessage.MapStringEnumEntry" do
optional :key, :string, 1
optional :value, :enum, 2, "A.B.C.TestEnum"
end
add_message "A.B.C.TestMessage.MapStringInt32Entry" do
optional :key, :string, 1
optional :value, :int32, 2
end
add_message "A.B.C.TestMessage.MapStringBoolEntry" do
optional :key, :string, 1
optional :value, :bool, 2
end
add_message "A.B.C.TestMessage.NestedMessage" do
optional :foo, :int32, 1
end
......@@ -107,16 +67,6 @@ module A
module B
module C
TestMessage = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage").msgclass
TestMessage::MapInt32StringEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapInt32StringEntry").msgclass
TestMessage::MapInt64StringEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapInt64StringEntry").msgclass
TestMessage::MapUint32StringEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapUint32StringEntry").msgclass
TestMessage::MapUint64StringEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapUint64StringEntry").msgclass
TestMessage::MapBoolStringEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapBoolStringEntry").msgclass
TestMessage::MapStringStringEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapStringStringEntry").msgclass
TestMessage::MapStringMsgEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapStringMsgEntry").msgclass
TestMessage::MapStringEnumEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapStringEnumEntry").msgclass
TestMessage::MapStringInt32Entry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapStringInt32Entry").msgclass
TestMessage::MapStringBoolEntry = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.MapStringBoolEntry").msgclass
TestMessage::NestedMessage = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.NestedMessage").msgclass
TestEnum = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestEnum").enummodule
end
......
src/google/protobuf/compiler/ruby/ruby_generator.cc
View file @ a2bea0a0
......@@ -102,6 +102,33 @@ std::string TypeName(const google::protobuf::FieldDescriptor* field) {
void GenerateField(const google::protobuf::FieldDescriptor* field,
google::protobuf::io::Printer* printer) {
if (field->is_map()) {
const FieldDescriptor* key_field =
field->message_type()->FindFieldByNumber(1);
const FieldDescriptor* value_field =
field->message_type()->FindFieldByNumber(2);
printer->Print(
"map :$name$, :$key_type$, :$value_type$, $number$",
"name", field->name(),
"key_type", TypeName(key_field),
"value_type", TypeName(value_field),
"number", IntToString(field->number()));
if (value_field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) {
printer->Print(
", \"$subtype$\"\n",
"subtype", value_field->message_type()->full_name());
} else if (value_field->cpp_type() == FieldDescriptor::CPPTYPE_ENUM) {
printer->Print(
", \"$subtype$\"\n",
"subtype", value_field->enum_type()->full_name());
} else {
printer->Print("\n");
}
} else {
printer->Print(
"$label$ :$name$, ",
"label", LabelForField(field),
......@@ -110,6 +137,7 @@ void GenerateField(const google::protobuf::FieldDescriptor* field,
":$type$, $number$",
"type", TypeName(field),
"number", IntToString(field->number()));
if (field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) {
printer->Print(
", \"$subtype$\"\n",
......@@ -121,6 +149,7 @@ void GenerateField(const google::protobuf::FieldDescriptor* field,
} else {
printer->Print("\n");
}
}
}
void GenerateOneof(const google::protobuf::OneofDescriptor* oneof,
......@@ -141,6 +170,13 @@ void GenerateOneof(const google::protobuf::OneofDescriptor* oneof,
void GenerateMessage(const google::protobuf::Descriptor* message,
google::protobuf::io::Printer* printer) {
// Don't generate MapEntry messages -- we use the Ruby extension's native
// support for map fields instead.
if (message->options().map_entry()) {
return;
}
printer->Print(
"add_message \"$name$\" do\n",
"name", message->full_name());
......@@ -213,6 +249,13 @@ void GenerateMessageAssignment(
const std::string& prefix,
const google::protobuf::Descriptor* message,
google::protobuf::io::Printer* printer) {
// Don't generate MapEntry messages -- we use the Ruby extension's native
// support for map fields instead.
if (message->options().map_entry()) {
return;
}
printer->Print(
"$prefix$$name$ = ",
"prefix", prefix,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment