You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by yu...@apache.org on 2023/02/16 15:12:08 UTC
[thrift] branch 0.18.1 updated: THRIFT-5685: Revert "THRIFT-5601: Fix forward typedef in go compiler"
This is an automated email from the ASF dual-hosted git repository.
yuxuan pushed a commit to branch 0.18.1
in repository https://gitbox.apache.org/repos/asf/thrift.git
The following commit(s) were added to refs/heads/0.18.1 by this push:
new 8fcdf7d5d THRIFT-5685: Revert "THRIFT-5601: Fix forward typedef in go compiler"
8fcdf7d5d is described below
commit 8fcdf7d5d058a83436abdb30eee28cdc26d51bef
Author: Yuxuan 'fishy' Wang <yu...@reddit.com>
AuthorDate: Wed Feb 15 12:19:44 2023 -0800
THRIFT-5685: Revert "THRIFT-5601: Fix forward typedef in go compiler"
This reverts commit b39370ec3bc96d201bbc82fbde136f98ae605ed1, and also
adds a test case for THRIFT-5685.
---
compiler/cpp/src/thrift/generate/t_go_generator.cc | 47 +++++++++-------------
compiler/cpp/src/thrift/generate/t_go_generator.h | 3 +-
.../{ForwardTypedef.thrift => ForwardType.thrift} | 9 ++---
lib/go/test/Makefile.am | 7 ++--
.../forwardtype_test.go} | 26 +++++++-----
5 files changed, 44 insertions(+), 48 deletions(-)
diff --git a/compiler/cpp/src/thrift/generate/t_go_generator.cc b/compiler/cpp/src/thrift/generate/t_go_generator.cc
index 680243d1a..90353ce9b 100644
--- a/compiler/cpp/src/thrift/generate/t_go_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_go_generator.cc
@@ -175,7 +175,7 @@ bool t_go_generator::is_pointer_field(t_field* tfield, bool in_container_value)
return has_default;
}
- throw "INVALID TYPE IN is_pointer_field: " + type->get_name();
+ throw "INVALID TYPE IN type_to_go_type: " + type->get_name();
}
std::string t_go_generator::camelcase(const std::string& value) const {
@@ -1088,7 +1088,7 @@ string t_go_generator::render_const_value(t_type* type, t_const_value* value, co
t_type* ktype = ((t_map*)type)->get_key_type();
t_type* vtype = ((t_map*)type)->get_val_type();
const map<t_const_value*, t_const_value*, t_const_value::value_compare>& val = value->get_map();
- out << "map[" << type_to_go_key_type(ktype) << "]" << type_to_go_container_value_type(vtype) << "{" << endl;
+ out << "map[" << type_to_go_key_type(ktype) << "]" << type_to_go_type(vtype) << "{" << endl;
indent_up();
map<t_const_value*, t_const_value*, t_const_value::value_compare>::const_iterator v_iter;
@@ -1102,7 +1102,7 @@ string t_go_generator::render_const_value(t_type* type, t_const_value* value, co
} else if (type->is_list()) {
t_type* etype = ((t_list*)type)->get_elem_type();
const vector<t_const_value*>& val = value->get_list();
- out << "[]" << type_to_go_container_value_type(etype) << "{" << endl;
+ out << "[]" << type_to_go_type(etype) << "{" << endl;
indent_up();
vector<t_const_value*>::const_iterator v_iter;
@@ -1115,7 +1115,7 @@ string t_go_generator::render_const_value(t_type* type, t_const_value* value, co
} else if (type->is_set()) {
t_type* etype = ((t_set*)type)->get_elem_type();
const vector<t_const_value*>& val = value->get_list();
- out << "[]" << type_to_go_container_value_type(etype) << "{" << endl;
+ out << "[]" << type_to_go_type(etype) << "{" << endl;
indent_up();
vector<t_const_value*>::const_iterator v_iter;
@@ -1258,7 +1258,7 @@ void t_go_generator::generate_go_struct_definition(ostream& out,
}
t_type* fieldType = (*m_iter)->get_type();
- string goType = type_to_go_type_with_opt(fieldType, is_pointer_field(*m_iter), false);
+ string goType = type_to_go_type_with_opt(fieldType, is_pointer_field(*m_iter));
map<string,string>tags;
tags["db"]=escape_string((*m_iter)->get_name());
@@ -1317,7 +1317,7 @@ void t_go_generator::generate_go_struct_definition(ostream& out,
t_const_value* def_value;
get_publicized_name_and_def_value(*m_iter, &publicized_name, &def_value);
t_type* fieldType = (*m_iter)->get_type();
- string goType = type_to_go_type_with_opt(fieldType, false, false);
+ string goType = type_to_go_type_with_opt(fieldType, false);
string def_var_name = tstruct_name + "_" + publicized_name + "_DEFAULT";
if ((*m_iter)->get_req() == t_field::T_OPTIONAL || is_pointer_field(*m_iter)) {
out << indent() << "var " << def_var_name << " " << goType;
@@ -1335,7 +1335,7 @@ void t_go_generator::generate_go_struct_definition(ostream& out,
}
if (is_pointer_field(*m_iter)) {
- string goOptType = type_to_go_type_with_opt(fieldType, true, false);
+ string goOptType = type_to_go_type_with_opt(fieldType, true);
string maybepointer = goOptType != goType ? "*" : "";
out << indent() << "func (p *" << tstruct_name << ") Get" << publicized_name << "() "
<< goType << " {" << endl;
@@ -1758,7 +1758,7 @@ void t_go_generator::generate_go_struct_equals(ostream& out,
field_name = (*f_iter)->get_name();
t_type* field_type = (*f_iter)->get_type();
publicize_field_name = publicize(field_name);
- string goType = type_to_go_type_with_opt(field_type, is_pointer_field(*f_iter), false);
+ string goType = type_to_go_type_with_opt(field_type, is_pointer_field(*f_iter));
string tgt = "p." + publicize_field_name;
string src = "other." + publicize_field_name;
@@ -3431,7 +3431,7 @@ void t_go_generator::generate_serialize_container(ostream& out,
if (pointer_field) {
wrapped_prefix = "(" + prefix + ")";
}
- string goType = type_to_go_container_value_type(tset->get_elem_type());
+ string goType = type_to_go_type(tset->get_elem_type());
out << indent() << "if func(tgt, src " << goType << ") bool {" << endl;
indent_up();
generate_go_equals(out, tset->get_elem_type(), "tgt", "src");
@@ -3892,40 +3892,31 @@ string t_go_generator::type_to_go_key_type(t_type* type) {
}
if (resolved_type->is_map() || resolved_type->is_list() || resolved_type->is_set()) {
- throw "Cannot produce a valid type for a Go map key: " + type_to_go_container_value_type(type) + " - aborting.";
+ throw "Cannot produce a valid type for a Go map key: " + type_to_go_type(type) + " - aborting.";
}
if (resolved_type->is_binary())
return "string";
- return type_to_go_container_value_type(type);
-}
-
-/**
- * Converts the parse type to a go type to be used in a container value
- */
-string t_go_generator::type_to_go_container_value_type(t_type* type) {
- return type_to_go_type_with_opt(type, false, true);
+ return type_to_go_type(type);
}
/**
* Converts the parse type to a go type
*/
string t_go_generator::type_to_go_type(t_type* type) {
- return type_to_go_type_with_opt(type, false, false);
+ return type_to_go_type_with_opt(type, false);
}
/**
* Converts the parse type to a go type, taking into account whether the field
- * associated with the type is T_OPTIONAL and whether it's used in a container
- * type.
+ * associated with the type is T_OPTIONAL.
*/
string t_go_generator::type_to_go_type_with_opt(t_type* type,
- bool optional_field,
- bool is_container_value) {
+ bool optional_field) {
string maybe_pointer(optional_field ? "*" : "");
- if (is_container_value && type->is_typedef() && ((t_typedef*)type)->is_forward_typedef()) {
+ if (type->is_typedef() && ((t_typedef*)type)->is_forward_typedef()) {
type = ((t_typedef*)type)->get_true_type();
}
@@ -3974,21 +3965,21 @@ string t_go_generator::type_to_go_type_with_opt(t_type* type,
} else if (type->is_map()) {
t_map* t = (t_map*)type;
string keyType = type_to_go_key_type(t->get_key_type());
- string valueType = type_to_go_container_value_type(t->get_val_type());
+ string valueType = type_to_go_type(t->get_val_type());
return maybe_pointer + string("map[") + keyType + "]" + valueType;
} else if (type->is_set()) {
t_set* t = (t_set*)type;
- string elemType = type_to_go_container_value_type(t->get_elem_type());
+ string elemType = type_to_go_type(t->get_elem_type());
return maybe_pointer + string("[]") + elemType;
} else if (type->is_list()) {
t_list* t = (t_list*)type;
- string elemType = type_to_go_container_value_type(t->get_elem_type());
+ string elemType = type_to_go_type(t->get_elem_type());
return maybe_pointer + string("[]") + elemType;
} else if (type->is_typedef()) {
return maybe_pointer + publicize(type_name(type));
}
- throw "INVALID TYPE IN type_to_go_type_with_opt: " + type->get_name();
+ throw "INVALID TYPE IN type_to_go_type: " + type->get_name();
}
/** See the comment inside generate_go_struct_definition for what this is. */
diff --git a/compiler/cpp/src/thrift/generate/t_go_generator.h b/compiler/cpp/src/thrift/generate/t_go_generator.h
index ad6cee636..a67485c55 100644
--- a/compiler/cpp/src/thrift/generate/t_go_generator.h
+++ b/compiler/cpp/src/thrift/generate/t_go_generator.h
@@ -264,9 +264,8 @@ public:
std::string argument_list(t_struct* tstruct);
std::string type_to_enum(t_type* ttype);
std::string type_to_go_type(t_type* ttype);
- std::string type_to_go_type_with_opt(t_type* ttype, bool optional_field, bool is_container_value);
+ std::string type_to_go_type_with_opt(t_type* ttype, bool optional_field);
std::string type_to_go_key_type(t_type* ttype);
- std::string type_to_go_container_value_type(t_type* ttype);
std::string type_to_spec_args(t_type* ttype);
void indent_up() { t_generator::indent_up(); }
diff --git a/lib/go/test/ForwardTypedef.thrift b/lib/go/test/ForwardType.thrift
similarity index 85%
copy from lib/go/test/ForwardTypedef.thrift
copy to lib/go/test/ForwardType.thrift
index 4266b7a28..0433c97cf 100644
--- a/lib/go/test/ForwardTypedef.thrift
+++ b/lib/go/test/ForwardType.thrift
@@ -17,17 +17,14 @@
* under the License.
*/
-// https://issues.apache.org/jira/browse/THRIFT-5601
+// https://issues.apache.org/jira/browse/THRIFT-5685
-namespace go forwardtypedef
+namespace go forwardtypetest
struct Struct {
- 1: optional Def foo
- 2: optional Exc bar
+ 1: optional Exc foo
}
-typedef i32 Def
-
exception Exc {
1: optional i32 code
}
diff --git a/lib/go/test/Makefile.am b/lib/go/test/Makefile.am
index c255a8e48..cb8928bc8 100644
--- a/lib/go/test/Makefile.am
+++ b/lib/go/test/Makefile.am
@@ -62,7 +62,7 @@ gopath: $(THRIFT) $(THRIFTTEST) \
ProcessorMiddlewareTest.thrift \
ClientMiddlewareExceptionTest.thrift \
ValidateTest.thrift \
- ForwardTypedef.thrift
+ ForwardType.thrift
mkdir -p gopath/src
grep -v list.*map.*list.*map $(THRIFTTEST) | grep -v 'set<Insanity>' > ThriftTest.thrift
$(THRIFT) $(THRIFTARGS) -r IncludesTest.thrift
@@ -97,7 +97,7 @@ gopath: $(THRIFT) $(THRIFTTEST) \
$(THRIFT) $(THRIFTARGS_SKIP_REMOTE) ProcessorMiddlewareTest.thrift
$(THRIFT) $(THRIFTARGS) ClientMiddlewareExceptionTest.thrift
$(THRIFT) $(THRIFTARGS) ValidateTest.thrift
- $(THRIFT) $(THRIFTARGS) ForwardTypedef.thrift
+ $(THRIFT) $(THRIFTARGS) ForwardType.thrift
ln -nfs ../../tests gopath/src/tests
cp -r ./dontexportrwtest gopath/src
touch gopath
@@ -124,7 +124,7 @@ check: gopath
./gopath/src/processormiddlewaretest \
./gopath/src/clientmiddlewareexceptiontest \
./gopath/src/validatetest \
- ./gopath/src/forwardtypedef
+ ./gopath/src/forwardtypetest
$(GO) test github.com/apache/thrift/lib/go/thrift
$(GO) test ./gopath/src/tests ./gopath/src/dontexportrwtest
@@ -155,6 +155,7 @@ EXTRA_DIST = \
DuplicateImportsTest.thrift \
ErrorTest.thrift \
EqualsTest.thrift \
+ ForwardType.thrift \
GoTagTest.thrift \
IgnoreInitialismsTest.thrift \
IncludesTest.thrift \
diff --git a/lib/go/test/ForwardTypedef.thrift b/lib/go/test/tests/forwardtype_test.go
similarity index 63%
rename from lib/go/test/ForwardTypedef.thrift
rename to lib/go/test/tests/forwardtype_test.go
index 4266b7a28..99b7890e6 100644
--- a/lib/go/test/ForwardTypedef.thrift
+++ b/lib/go/test/tests/forwardtype_test.go
@@ -17,17 +17,25 @@
* under the License.
*/
-// https://issues.apache.org/jira/browse/THRIFT-5601
+package tests
-namespace go forwardtypedef
+import (
+ "testing"
-struct Struct {
- 1: optional Def foo
- 2: optional Exc bar
-}
+ "github.com/apache/thrift/lib/go/test/gopath/src/forwardtypetest"
+ "github.com/apache/thrift/lib/go/thrift"
+)
-typedef i32 Def
+func TestForwardType(t *testing.T) {
+ // See https://issues.apache.org/jira/browse/THRIFT-5685
-exception Exc {
- 1: optional i32 code
+ const code = int32(1)
+ foo := &forwardtypetest.Struct{
+ Foo: &forwardtypetest.Exc{
+ Code: thrift.Pointer(code),
+ },
+ }
+ if got, want := foo.GetFoo().GetCode(), code; got != want {
+ t.Errorf("code got %v want %v", got, want)
+ }
}