fishy commented on a change in pull request #2453:
URL: https://github.com/apache/thrift/pull/2453#discussion_r705484141



##########
File path: compiler/cpp/src/thrift/generate/t_go_generator.cc
##########
@@ -4021,7 +4021,7 @@ string t_go_generator::type_to_go_key_type(t_type* type) {
   }
 
   if (resolved_type->is_binary())
-    return "string";
+    return "[]byte";

Review comment:
       this is not the correct fix.
   
   this function is called `type_to_go_key_type`, it's used to generate go type 
for keys in maps, and you cannot use `[]byte` as a map key in go. this will 
break all the maps using binary as the key type.
   
   the correct fix is to call `type_to_go_type` instead in `render_const_value` 
for sets (because in go we don't actually render sets as maps, we render them 
as slices instead).




-- 
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]


Reply via email to