[GitHub] [thrift] vernor1 commented on a change in pull request #2453: THRIFT-4868: Golang: Fix compilation for optional set with default values

[GitBox <gi...@apache.org>]

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



##########
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:
       @fishy Thank you for the clarification. Could you check the updated solution?
   Both types are now properly handled: `set<binary>` and `map<binary, T>`:
   ```
   root@2dda16a0c248:/thrift/src# make -Ctest/go check
   make: Entering directory '/thrift/src/test/go'
   Makefile:657: warning: overriding recipe for target 'check'
   Makefile:506: warning: ignoring old recipe for target 'check'
   mkdir -p src/gen
   /thrift/src/compiler/cpp/thrift -out src/gen --gen go:thrift_import=github.com/apache/thrift/lib/go/thrift,package_prefix=github.com/apache/thrift/test/go/src/gen/ ThriftTest.thrift
   [WARNING:/thrift/src/test/go/ThriftTest.thrift:43] No generator named 'noexist' could be found!
   [WARNING:/thrift/src/test/go/ThriftTest.thrift:45] cpp generator does not accept 'noexist' as sub-namespace!
   /thrift/src/compiler/cpp/thrift -out src/gen --gen go:thrift_import=github.com/apache/thrift/lib/go/thrift,package_prefix=github.com/apache/thrift/test/go/src/gen/ ../StressTest.thrift
   [WARNING:/thrift/src/test/StressTest.thrift:31] Consider using the more efficient "binary" type instead of "list<byte>".
   [WARNING:/thrift/src/test/StressTest.thrift:31] Consider using the more efficient "binary" type instead of "list<byte>".
   touch gopath
   sh genmock.sh
   go: downloading github.com/golang/mock v1.5.0
   go: downloading golang.org/x/mod v0.3.0
   go: downloading golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e
   go: downloading golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898
   /usr/local/bin/go test -mod=mod -v ./src/common/...
   === RUN   TestAllConnection
   --- PASS: TestAllConnection (0.02s)
   === RUN   TestHttpContextTimeout
   --- PASS: TestHttpContextTimeout (0.05s)
   PASS
   ok  	github.com/apache/thrift/test/go/src/common	0.088s
   make: Leaving directory '/thrift/src/test/go'
   ```




-- 
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: notifications-unsubscribe@thrift.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org