You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by dc...@apache.org on 2019/11/09 19:22:35 UTC
[thrift] branch master updated: THRIFT-5002: Fix argument
containers for inherited functions
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 59694a7 THRIFT-5002: Fix argument containers for inherited functions
59694a7 is described below
commit 59694a796f824ffcde745065efa36b8d0ddf8275
Author: John Boiles <jo...@gmail.com>
AuthorDate: Sat Nov 9 11:22:26 2019 -0800
THRIFT-5002: Fix argument containers for inherited functions
Fix a bug where remote.go client fails to compile when services
extend other services and the parent service has a function that
needs a container for its arguments.
Client:go
This closes #1925.
---
compiler/cpp/src/thrift/generate/t_go_generator.cc | 30 +++++++++++++++++-----
lib/go/test/Makefile.am | 5 ++--
lib/go/test/ServicesTest.thrift | 7 +++++
3 files changed, 34 insertions(+), 8 deletions(-)
diff --git a/compiler/cpp/src/thrift/generate/t_go_generator.cc b/compiler/cpp/src/thrift/generate/t_go_generator.cc
index 33b7547..e153a78 100644
--- a/compiler/cpp/src/thrift/generate/t_go_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_go_generator.cc
@@ -311,6 +311,7 @@ private:
std::string camelcase(const std::string& value) const;
void fix_common_initialism(std::string& value, int i) const;
std::string publicize(const std::string& value, bool is_args_or_result = false) const;
+ std::string publicize(const std::string& value, bool is_args_or_result, const std::string& service_name) const;
std::string privatize(const std::string& value) const;
std::string new_prefix(const std::string& value) const;
static std::string variable_name_to_go_name(const std::string& value);
@@ -464,7 +465,7 @@ void t_go_generator::fix_common_initialism(std::string& value, int i) const {
}
}
-std::string t_go_generator::publicize(const std::string& value, bool is_args_or_result) const {
+std::string t_go_generator::publicize(const std::string& value, bool is_args_or_result, const std::string& service_name) const {
if (value.size() <= 0) {
return value;
}
@@ -506,12 +507,16 @@ std::string t_go_generator::publicize(const std::string& value, bool is_args_or_
// Avoid naming collisions with other services
if (is_args_or_result) {
- prefix += publicize(service_name_);
+ prefix += publicize(service_name);
}
return prefix + value2;
}
+std::string t_go_generator::publicize(const std::string& value, bool is_args_or_result) const {
+ return publicize(value, is_args_or_result, service_name_);
+}
+
std::string t_go_generator::new_prefix(const std::string& value) const {
if (value.size() <= 0) {
return value;
@@ -2121,13 +2126,26 @@ void t_go_generator::generate_service_client(t_service* tservice) {
* @param tservice The service to generate a remote for.
*/
void t_go_generator::generate_service_remote(t_service* tservice) {
- vector<t_function*> functions = tservice->get_functions();
- t_service* parent = tservice->get_extends();
+ vector<t_function*> functions;
+ std::unordered_map<std::string, std::string> func_to_service;
- // collect inherited functions
+ // collect all functions including inherited functions
+ t_service* parent = tservice;
while (parent != NULL) {
vector<t_function*> p_functions = parent->get_functions();
functions.insert(functions.end(), p_functions.begin(), p_functions.end());
+
+ // We need to maintain a map of functions names to the name of their parent.
+ // This is because functions may come from a parent service, and if we need
+ // to create the arguments struct (e.g. `NewParentServiceNameFuncNameArgs()`)
+ // we need to make sure to specify the correct service name.
+ for (vector<t_function*>::iterator f_iter = p_functions.begin(); f_iter != p_functions.end(); ++f_iter) {
+ auto it = func_to_service.find((*f_iter)->get_name());
+ if (it == func_to_service.end()) {
+ func_to_service.emplace((*f_iter)->get_name(), parent->get_name());
+ }
+ }
+
parent = parent->get_extends();
}
@@ -2340,7 +2358,7 @@ void t_go_generator::generate_service_remote(t_service* tservice) {
std::vector<t_field*>::size_type num_args = args.size();
string funcName((*f_iter)->get_name());
string pubName(publicize(funcName));
- string argumentsName(publicize(funcName + "_args", true));
+ string argumentsName(publicize(funcName + "_args", true, func_to_service[funcName]));
f_remote << indent() << "case \"" << escape_string(funcName) << "\":" << endl;
indent_up();
f_remote << indent() << "if flag.NArg() - 1 != " << num_args << " {" << endl;
diff --git a/lib/go/test/Makefile.am b/lib/go/test/Makefile.am
index 244ddff..2a9d3f5 100644
--- a/lib/go/test/Makefile.am
+++ b/lib/go/test/Makefile.am
@@ -93,7 +93,8 @@ check: gopath
ignoreinitialismstest \
unionbinarytest \
conflictnamespacetestsuperthing \
- conflict/context/conflict_service-remote
+ conflict/context/conflict_service-remote \
+ servicestest/container_test-remote
GOPATH=`pwd`/gopath $(GO) test thrift tests dontexportrwtest
clean-local:
@@ -127,5 +128,5 @@ EXTRA_DIST = \
ConflictNamespaceTestB.thrift \
ConflictNamespaceTestC.thrift \
ConflictNamespaceTestD.thrift \
- ConflictNamespaceTestSuperThing.thrift
+ ConflictNamespaceTestSuperThing.thrift \
ConflictNamespaceServiceTest.thrift
diff --git a/lib/go/test/ServicesTest.thrift b/lib/go/test/ServicesTest.thrift
index 882b03a..666197f 100644
--- a/lib/go/test/ServicesTest.thrift
+++ b/lib/go/test/ServicesTest.thrift
@@ -107,5 +107,12 @@ service a_serv {
struct_a struct_a_func_2ex_1int_1s(1: i64 i, 2: string s) throws(1: moderate_disaster err1, 2:total_disaster err2)
struct_a struct_a_func_1struct_a(1: struct_a st)
+}
+
+service container_test_parent {
+ void parent_only_func(1: set<i32> s)
+}
+service container_test extends container_test_parent {
+ void child_only_func(1: set<i32> s)
}