[
https://issues.apache.org/jira/browse/THRIFT-4617?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16595897#comment-16595897
]
ASF GitHub Bot commented on THRIFT-4617:
----------------------------------------
jeking3 closed pull request #1578: THRIFT-4617: Prepend service-specific struct
names with service name
URL: https://github.com/apache/thrift/pull/1578
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/compiler/cpp/src/thrift/generate/t_rs_generator.cc
b/compiler/cpp/src/thrift/generate/t_rs_generator.cc
index dce4288ad9..dc11fd3ee2 100644
--- a/compiler/cpp/src/thrift/generate/t_rs_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_rs_generator.cc
@@ -350,7 +350,8 @@ class t_rs_generator : public t_generator {
void render_sync_handler_failed_default_exception_branch(t_function *tfunc);
void render_sync_handler_send_exception_response(t_function *tfunc, const
string &err_var);
void render_service_call_structs(t_service* tservice);
- void render_result_value_struct(t_function* tfunc);
+ void render_service_call_args_struct(t_function* tfunc);
+ void render_service_call_result_value_struct(t_function* tfunc);
string handler_successful_return_struct(t_function* tfunc);
@@ -473,6 +474,9 @@ class t_rs_generator : public t_generator {
// Return the name of the function that users will have to implement to
handle incoming service calls.
string service_call_handler_function_name(t_function* tfunc);
+ // Return the name of the struct used to pack the arguments for the thrift
service call.
+ string service_call_args_struct_name(t_function* tfunc);
+
// Return the name of the struct used to pack the return value
// and user-defined exceptions for the thrift service call.
string service_call_result_struct_name(t_function* tfunc);
@@ -2050,9 +2054,9 @@ void
t_rs_generator::render_service_call_structs(t_service* tservice) {
// contains the exceptions as well
for(func_iter = functions.begin(); func_iter != functions.end();
++func_iter) {
t_function* tfunc = (*func_iter);
- render_struct(rust_struct_name(tfunc->get_arglist()),
tfunc->get_arglist(), t_rs_generator::T_ARGS);
+ render_service_call_args_struct(tfunc);
if (!tfunc->is_oneway()) {
- render_result_value_struct(tfunc);
+ render_service_call_result_value_struct(tfunc);
}
}
}
@@ -2297,7 +2301,7 @@ void t_rs_generator::render_sync_send(t_function* tfunc) {
f_gen_
<< indent()
<< "let call_args = "
- << rust_struct_name(tfunc->get_arglist())
+ << service_call_args_struct_name(tfunc)
<< " { "
<< struct_fields
<< " };"
@@ -2411,7 +2415,12 @@ string t_rs_generator::struct_to_invocation(t_struct*
tstruct, const string& fie
return args.str();
}
-void t_rs_generator::render_result_value_struct(t_function* tfunc) {
+void t_rs_generator::render_service_call_args_struct(t_function* tfunc) {
+ string args_struct_name(service_call_args_struct_name(tfunc));
+ render_struct(args_struct_name, tfunc->get_arglist(),
t_rs_generator::T_ARGS);
+}
+
+void t_rs_generator::render_service_call_result_value_struct(t_function*
tfunc) {
string result_struct_name = service_call_result_struct_name(tfunc);
t_struct result(program_, result_struct_name);
@@ -2668,7 +2677,7 @@ void
t_rs_generator::render_sync_process_function(t_function *tfunc, const strin
<< "let "
<< (has_non_void_args(tfunc) ? "args" : "_")
<< " = "
- << rust_struct_name(tfunc->get_arglist())
+ << service_call_args_struct_name(tfunc)
<< "::read_from_in_protocol(i_prot)?;"
<< endl;
@@ -3212,8 +3221,13 @@ string
t_rs_generator::service_call_handler_function_name(t_function* tfunc) {
return "handle_" + rust_snake_case(tfunc->get_name());
}
+string t_rs_generator::service_call_args_struct_name(t_function* tfunc) {
+ // Thrift automatically appends `Args` to the arglist name. No need to do it
here.
+ return rust_camel_case(service_name_) +
rust_camel_case(tfunc->get_arglist()->get_name());
+}
+
string t_rs_generator::service_call_result_struct_name(t_function* tfunc) {
- return rust_camel_case(tfunc->get_name()) + RESULT_STRUCT_SUFFIX;
+ return rust_camel_case(service_name_) + rust_camel_case(tfunc->get_name()) +
RESULT_STRUCT_SUFFIX;
}
string t_rs_generator::rust_sync_client_marker_trait_name(t_service* tservice)
{
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> Avoid generating conflicting struct names in Rust code
> ------------------------------------------------------
>
> Key: THRIFT-4617
> URL: https://issues.apache.org/jira/browse/THRIFT-4617
> Project: Thrift
> Issue Type: Bug
> Components: Rust - Compiler
> Reporter: Flavien Raynaud
> Priority: Minor
> Fix For: 0.12.0
>
>
> If a thrift definition file contains multiple services, with functions having
> the same name, the generated code is not valid. There are conflicting
> definitions of {{*Args}} and {{*Result}} structs.
> {noformat}
> service backend {
> i32 send(
> 1: i32 arg
> );
> }
> service other_backend {
> string send(
> 1: i32 arg
> )
> }
> {noformat}
> Will generate the following code in the same file (I only pasted the
> interesting bits):
> {noformat}
> struct SendResult {
> result_value: Option<i32>,
> }
> // ...
> struct SendResult {
> result_value: Option<String>,
> }
> {noformat}
> And rustc would complain with errors like:
> {noformat}
> error[E0428]: the name `SendResult` is defined multiple times
> --> src/test.rs:496:1
> |
> 234 | struct SendResult {
> | ----------------- previous definition of the type `SendResult` here
> ...
> 496 | struct SendResult {
> | ^^^^^^^^^^^^^^^^^ `SendResult` redefined here
> |
> = note: `SendResult` must be defined only once in the type namespace of
> this module
> {noformat}
> ----
> Another (very similar) issue occurs if a user-defined struct happens to be
> called (keeping the same example as before) {{SendResult}}:
> {noformat}
> struct SendResult {
> 1: i32 resultCode,
> 2: optional string errorCause,
> }
> service backend {
> SendResult send(
> 1: i32 arg
> );
> }
> {noformat}
> Will generate the following code in the same file (I only pasted the
> interesting bits)::
> {noformat}
> pub struct SendResult {
> pub result_code: Option<i32>,
> pub error_cause: Option<String>,
> }
> // ...
> struct SendResult {
> result_value: Option<SendResult>,
> }
> {noformat}
> Again, conflicting definitions of structs and rustc would complain.
> ----
> The approach taken by the Go generator (I haven't looked at other languages
> too much, some other generators probably do the same) for generating internal
> structs related to a service call is to [prepend the service
> name|https://github.com/apache/thrift/blob/master/compiler/cpp/src/thrift/generate/t_go_generator.cc#L502]
> to each of the structs.
> Using the same mechanism would solve both the issues cited above.
> I will give it a try and open a PR, but am also definitely open to any
> feedback on the idea :)
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
