This is an automated email from the ASF dual-hosted git repository. dcelasun pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/thrift.git
The following commit(s) were added to refs/heads/master by this push: new 88591e3 THRIFT-4573 Support binary fields in union counts 88591e3 is described below commit 88591e32e710a0524327153c8b629d5b461e35e0 Author: D. Can Celasun <c...@dcc.im> AuthorDate: Thu May 17 08:52:11 2018 +0200 THRIFT-4573 Support binary fields in union counts This commit also fixes another, related issue: Since union support was added in b3654df, `Count*` methods (and count checks in `Write` methods) were only generated if there was at least 1 pointer field. But pointer fields are not the only nullable types in Go, slices and maps can also be set the nil, which are now taken into account. Client: go --- compiler/cpp/src/thrift/generate/t_go_generator.cc | 11 +++++-- lib/go/test/Makefile.am | 6 +++- lib/go/test/UnionBinaryTest.thrift | 25 +++++++++++++++ lib/go/test/tests/union_binary_test.go | 36 ++++++++++++++++++++++ 4 files changed, 75 insertions(+), 3 deletions(-) diff --git a/compiler/cpp/src/thrift/generate/t_go_generator.cc b/compiler/cpp/src/thrift/generate/t_go_generator.cc index 5b65d2c..b5742f6 100644 --- a/compiler/cpp/src/thrift/generate/t_go_generator.cc +++ b/compiler/cpp/src/thrift/generate/t_go_generator.cc @@ -1387,6 +1387,14 @@ void t_go_generator::generate_go_struct_definition(ostream& out, } out << endl; } + + // num_setable is used for deciding if Count* methods will be generated for union fields. + // This applies to all nullable fields including slices (used for set, list and binary) and maps, not just pointers. + t_type* type = fieldType->get_true_type(); + if (is_pointer_field(*m_iter)|| type->is_map() || type->is_set() || type->is_list() || type->is_binary()) { + num_setable += 1; + } + if (is_pointer_field(*m_iter)) { string goOptType = type_to_go_type_with_opt(fieldType, true); string maybepointer = goOptType != goType ? "*" : ""; @@ -1397,7 +1405,6 @@ void t_go_generator::generate_go_struct_definition(ostream& out, out << indent() << " }" << endl; out << indent() << "return " << maybepointer << "p." << publicized_name << endl; out << indent() << "}" << endl; - num_setable += 1; } else { out << endl; out << indent() << "func (p *" << tstruct_name << ") Get" << publicized_name << "() " @@ -1491,7 +1498,7 @@ void t_go_generator::generate_countsetfields_helper(ostream& out, t_type* type = (*f_iter)->get_type()->get_true_type(); - if (!(is_pointer_field(*f_iter) || type->is_map() || type->is_set() || type->is_list())) + if (!(is_pointer_field(*f_iter) || type->is_map() || type->is_set() || type->is_list() || type->is_binary())) continue; const string field_name(publicize(escape_string((*f_iter)->get_name()))); diff --git a/lib/go/test/Makefile.am b/lib/go/test/Makefile.am index e93ec5c..b7ba870 100644 --- a/lib/go/test/Makefile.am +++ b/lib/go/test/Makefile.am @@ -32,6 +32,7 @@ gopath: $(THRIFT) $(THRIFTTEST) \ TypedefFieldTest.thrift \ RefAnnotationFieldsTest.thrift \ UnionDefaultValueTest.thrift \ + UnionBinaryTest.thrift \ ErrorTest.thrift \ NamesTest.thrift \ InitialismsTest.thrift \ @@ -50,6 +51,7 @@ gopath: $(THRIFT) $(THRIFTTEST) \ $(THRIFT) $(THRIFTARGS) TypedefFieldTest.thrift $(THRIFT) $(THRIFTARGS) RefAnnotationFieldsTest.thrift $(THRIFT) $(THRIFTARGS) UnionDefaultValueTest.thrift + $(THRIFT) $(THRIFTARGS) UnionBinaryTest.thrift $(THRIFT) $(THRIFTARGS) ErrorTest.thrift $(THRIFT) $(THRIFTARGS) NamesTest.thrift $(THRIFT) $(THRIFTARGS) InitialismsTest.thrift @@ -74,7 +76,8 @@ check: gopath namestest \ initialismstest \ dontexportrwtest \ - ignoreinitialismstest + ignoreinitialismstest \ + unionbinarytest GOPATH=`pwd`/gopath $(GO) test thrift tests dontexportrwtest clean-local: @@ -95,6 +98,7 @@ EXTRA_DIST = \ OptionalFieldsTest.thrift \ RefAnnotationFieldsTest.thrift \ UnionDefaultValueTest.thrift \ + UnionBinaryTest.thrift \ ServicesTest.thrift \ TypedefFieldTest.thrift \ ErrorTest.thrift \ diff --git a/lib/go/test/UnionBinaryTest.thrift b/lib/go/test/UnionBinaryTest.thrift new file mode 100644 index 0000000..f77112b --- /dev/null +++ b/lib/go/test/UnionBinaryTest.thrift @@ -0,0 +1,25 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# + +# See https://issues.apache.org/jira/browse/THRIFT-4573 +union Sample { + 1: map<string, string> u1, + 2: binary u2, + 3: list<string> u3 +} diff --git a/lib/go/test/tests/union_binary_test.go b/lib/go/test/tests/union_binary_test.go new file mode 100644 index 0000000..bdae2cb --- /dev/null +++ b/lib/go/test/tests/union_binary_test.go @@ -0,0 +1,36 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package tests + +import ( + "testing" + "unionbinarytest" +) + + +// See https://issues.apache.org/jira/browse/THRIFT-4573 +func TestUnionBinary(t *testing.T) { + s := unionbinarytest.NewSample() + s.U1 = map[string]string{} + s.U2 = []byte{} + if n := s.CountSetFieldsSample(); n != 2 { + t.Errorf("Expected 2 set fields, got %d!", n) + } +} -- To stop receiving notification emails like this one, please contact dcela...@apache.org.