fishy commented on code in PR #3788:
URL: https://github.com/apache/thrift/pull/3788#discussion_r3941040189
##########
compiler/cpp/src/thrift/generate/t_go_generator.cc:
##########
@@ -4027,28 +4027,71 @@ void
t_go_generator::generate_serialize_container(ostream& out,
if (pointer_field) {
wrapped_prefix = "(" + prefix + ")";
}
- string keyType = type_to_go_type(tmap->get_key_type());
- out << indent() << "for i := 0; i < len(" << prefix << "); i++ {" <<
'\n';
- indent_up();
- out << indent() << "for j := i + 1; j < len(" << prefix << "); j++ {" <<
'\n';
- indent_up();
- out << indent() << "if func(tgt, src " << keyType << ") bool {" << '\n';
- indent_up();
- generate_go_equals(out, tmap->get_key_type(), "tgt", "src");
- out << indent() << "return true" << '\n';
- indent_down();
- out << indent() << "}(" << wrapped_prefix << "[i].Key, " <<
wrapped_prefix << "[j].Key) {"
- << '\n';
- indent_up();
- out << indent() << "return
thrift.NewTProtocolExceptionWithType(thrift.INVALID_DATA, "
- << "fmt.Errorf(\"%T error writing map field %q: keys are not
unique\", "
- << wrapped_prefix << ", \"" << escape_string(prefix) << "\"))" <<
'\n';
- indent_down();
- out << indent() << "}" << '\n';
- indent_down();
- out << indent() << "}" << '\n';
- indent_down();
- out << indent() << "}" << '\n';
+ string not_unique
+ = "return thrift.NewTProtocolExceptionWithType(thrift.INVALID_DATA, "
+ "fmt.Errorf(\"%T error writing map field %q: keys are not
unique\", "
+ + wrapped_prefix + ", \"" + escape_string(prefix) + "\"))";
+ if (is_comparable_struct_key(tmap->get_key_type())) {
+ // Every field of the key struct is a non-pointer scalar, so the struct
+ // value is a valid Go map key and "==" agrees with Equals. Collecting
+ // the keys in a set costs linear time, where comparing every pair
+ // costs quadratic time.
+ string seen = tmp("seen");
+ string sawNil = tmp("sawNil");
+ string entry = tmp("e");
+ string keyValueType =
publicize(type_name(tmap->get_key_type()->get_true_type()));
+ out << indent() << "if len(" << wrapped_prefix << ") > 1 {" << '\n';
+ indent_up();
+ out << indent() << seen << " := make(map[" << keyValueType <<
"]struct{}, len("
+ << wrapped_prefix << "))" << '\n';
+ out << indent() << sawNil << " := false" << '\n';
+ out << indent() << "for _, " << entry << " := range " <<
wrapped_prefix << " {" << '\n';
+ indent_up();
+ // A nil key writes as an empty struct, and Equals treats two of them
as
+ // equal, so track nil separately rather than dereferencing it.
+ out << indent() << "if " << entry << ".Key == nil {" << '\n';
+ indent_up();
+ out << indent() << "if " << sawNil << " {" << '\n';
+ indent_up();
+ out << indent() << not_unique << '\n';
+ indent_down();
+ out << indent() << "}" << '\n';
+ out << indent() << sawNil << " = true" << '\n';
+ out << indent() << "continue" << '\n';
+ indent_down();
+ out << indent() << "}" << '\n';
+ out << indent() << "if _, ok := " << seen << "[*" << entry << ".Key];
ok {" << '\n';
+ indent_up();
+ out << indent() << not_unique << '\n';
+ indent_down();
+ out << indent() << "}" << '\n';
+ out << indent() << seen << "[*" << entry << ".Key] = struct{}{}" <<
'\n';
+ indent_down();
+ out << indent() << "}" << '\n';
+ indent_down();
+ out << indent() << "}" << '\n';
+ } else {
Review Comment:
I would suggest to add a test case for this into `StructKeyTest.thrift`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
