Skip to content

Commit

Permalink
Remove t_type::is_service
Browse files Browse the repository at this point in the history
Summary:
Services are not (data) types in Thrift. We already decoupled the two in most of the compiler, now remove `t_type::is_service` which doesn't make sense and is effectively deprecated (there is a TODO to remove it among other functions).

For return types `is_service` should always return false since interactions are stored separately so removing those calls to `is_service` is a noop.

Reviewed By: iahs

Differential Revision: D70573343

fbshipit-source-id: 1e344edeae716c576270ea0d69bad93a881a3656
  • Loading branch information
vitaut authored and facebook-github-bot committed Mar 5, 2025
1 parent a5b2733 commit cc96834
Show file tree
Hide file tree
Showing 11 changed files with 78 additions and 81 deletions.
5 changes: 1 addition & 4 deletions third-party/thrift/src/thrift/compiler/ast/t_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@

#pragma once

#include <memory>
#include <string>
#include <vector>

#include <thrift/compiler/ast/t_interface.h>

Expand All @@ -31,7 +29,7 @@ class t_program;
*/
class t_service : public t_interface {
public:
explicit t_service(
t_service(
t_program* program, std::string name, const t_service* extends = nullptr)
: t_interface(program, std::move(name)), extends_(extends) {}

Expand All @@ -51,7 +49,6 @@ class t_service : public t_interface {
const t_service* get_extends() const { return extends_; }
void set_extends(const t_service* extends) { extends_ = extends; }
type get_type_value() const override { return type::t_service; }
bool is_service() const override { return true; }
};

} // namespace apache::thrift::compiler
1 change: 0 additions & 1 deletion third-party/thrift/src/thrift/compiler/ast/t_type.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ class t_type : public t_named {
virtual bool is_list() const { return false; }
virtual bool is_set() const { return false; }
virtual bool is_map() const { return false; }
virtual bool is_service() const { return false; }
virtual bool is_binary() const { return false; }
virtual bool is_paramlist() const { return false; }

Expand Down
3 changes: 1 addition & 2 deletions third-party/thrift/src/thrift/compiler/generate/go/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -494,8 +494,7 @@ std::string snakecase(const std::string& name) {
}

bool is_func_go_supported(const t_function* func) {
return !func->sink_or_stream() && !func->return_type()->is_service() &&
!func->interaction();
return !func->sink_or_stream() && !func->interaction();
}

bool is_go_reserved_word(const std::string& value) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,6 @@ class mstch_type : public mstch_base {
{"type:struct?", &mstch_type::is_struct},
{"type:union?", &mstch_type::is_union},
{"type:enum?", &mstch_type::is_enum},
{"type:service?", &mstch_type::is_service},
{"type:base?", &mstch_type::is_base},
{"type:container?", &mstch_type::is_container},
{"type:list?", &mstch_type::is_list},
Expand All @@ -843,7 +842,6 @@ class mstch_type : public mstch_base {
{"type:value_type", &mstch_type::get_value_type},
{"type:typedef_type", &mstch_type::get_typedef_type},
{"type:typedef", &mstch_type::get_typedef},
{"type:interaction?", &mstch_type::is_interaction},
});
}
mstch::node name() { return type_->get_name(); }
Expand Down Expand Up @@ -872,7 +870,6 @@ class mstch_type : public mstch_base {
}
mstch::node is_union() { return resolved_type_->is_union(); }
mstch::node is_enum() { return resolved_type_->is_enum(); }
mstch::node is_service() { return resolved_type_->is_service(); }
mstch::node is_base() { return resolved_type_->is_primitive_type(); }
mstch::node is_container() { return resolved_type_->is_container(); }
mstch::node is_list() { return resolved_type_->is_list(); }
Expand All @@ -888,7 +885,6 @@ class mstch_type : public mstch_base {
mstch::node get_value_type();
mstch::node get_typedef_type();
mstch::node get_typedef();
mstch::node is_interaction() { return type_->is_service(); }

protected:
const t_type* type_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ class t_java_deprecated_generator : public t_concat_generator {
// indicate if we can generate the method
// E.g. Java doesn't support streaming, so all streaming methods are skipped
bool can_generate_method(const t_function* func) {
return !func->sink_or_stream() && !func->return_type()->is_service();
return !func->sink_or_stream();
}

bool is_field_sensitive(const t_field* field) {
Expand Down
118 changes: 66 additions & 52 deletions third-party/thrift/src/thrift/compiler/generate/t_json_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,20 @@ class t_json_generator : public t_concat_generator {
void generate_service(const t_service* tservice) override;
void generate_xception(const t_structured* txception) override;

void print_type(const t_type* ttype);
template <typename T>
void print_spec(const T* type_or_service);
void print_name(const string& name);
void print_const_value(const t_const_value* tvalue);
void print_const_key(t_const_value* tvalue);
void print_lineno(const t_node& node);
string type_to_string(const t_type* type);
string type_to_spec_args(const t_type* ttype);

string to_string(const t_type* type);
string to_string(const t_service* service);

string to_spec_args(const t_type* type);
string to_spec_args(const t_service* service);
string to_spec_args_named(const t_named* named);

string type_name(const t_type* ttype);

bool should_resolve_to_true_type(const t_type* ttype);
Expand Down Expand Up @@ -221,9 +228,9 @@ void t_json_generator::generate_program() {
}

/**
* Converts the parse type to a string
* Converts the type to a string.
*/
string t_json_generator::type_to_string(const t_type* type) {
string t_json_generator::to_string(const t_type* type) {
if (should_resolve_to_true_type(type)) {
type = type->get_true_type();
}
Expand Down Expand Up @@ -262,14 +269,15 @@ string t_json_generator::type_to_string(const t_type* type) {
return "SET";
} else if (type->is_list()) {
return "LIST";
} else if (type->is_service()) {
return "SERVICE";
} else if (type->is_typedef()) {
return "TYPEDEF";
}

throw std::runtime_error(
"INVALID TYPE IN type_to_string: " + type->get_name());
throw std::runtime_error("INVALID TYPE IN to_string: " + type->get_name());
}

string t_json_generator::to_string(const t_service*) {
return "SERVICE";
}

/**
Expand All @@ -281,44 +289,47 @@ string t_json_generator::type_to_string(const t_type* type) {
* | tuple_spec // (for lists and sets)
* | { "key_type" : tuple_spec, "val_type" : tuple_spec} // (maps)
*/
string t_json_generator::type_to_spec_args(const t_type* ttype) {
if (should_resolve_to_true_type(ttype)) {
ttype = ttype->get_true_type();
string t_json_generator::to_spec_args(const t_type* type) {
if (should_resolve_to_true_type(type)) {
type = type->get_true_type();
}

if (ttype->is_primitive_type()) {
if (type->is_primitive_type()) {
return "null";
} else if (
ttype->is_struct() || ttype->is_exception() || ttype->is_service() ||
ttype->is_enum() || ttype->is_typedef()) {
string module = "";
if (ttype->program() != program_) {
module = ttype->program()->name() + ".";
}
return "\"" + module + ttype->get_name() + "\"";
} else if (ttype->is_map()) {
type->is_struct() || type->is_exception() || type->is_enum() ||
type->is_typedef()) {
return to_spec_args_named(type);
} else if (type->is_map()) {
return R"({ "key_type" : { "type_enum" : ")" +
type_to_string(((t_map*)ttype)->get_key_type()) +
R"(", "spec_args" : )" +
type_to_spec_args(((t_map*)ttype)->get_key_type()) +
to_string(((t_map*)type)->get_key_type()) + R"(", "spec_args" : )" +
to_spec_args(((t_map*)type)->get_key_type()) +
R"( }, "val_type" : { "type_enum" : ")" +
type_to_string(((t_map*)ttype)->get_val_type()) +
R"(", "spec_args" : )" +
type_to_spec_args(((t_map*)ttype)->get_val_type()) + "} } ";
} else if (ttype->is_set()) {
return R"({ "type_enum" : ")" +
type_to_string(((t_set*)ttype)->get_elem_type()) +
R"(", "spec_args" : )" +
type_to_spec_args(((t_set*)ttype)->get_elem_type()) + "} ";
} else if (ttype->is_list()) {
to_string(((t_map*)type)->get_val_type()) + R"(", "spec_args" : )" +
to_spec_args(((t_map*)type)->get_val_type()) + "} } ";
} else if (type->is_set()) {
return R"({ "type_enum" : ")" + to_string(((t_set*)type)->get_elem_type()) +
R"(", "spec_args" : )" + to_spec_args(((t_set*)type)->get_elem_type()) +
"} ";
} else if (type->is_list()) {
return R"({ "type_enum" : ")" +
type_to_string(((t_list*)ttype)->get_elem_type()) +
R"(", "spec_args" : )" +
type_to_spec_args(((t_list*)ttype)->get_elem_type()) + "} ";
to_string(((t_list*)type)->get_elem_type()) + R"(", "spec_args" : )" +
to_spec_args(((t_list*)type)->get_elem_type()) + "} ";
}

throw std::runtime_error(
"INVALID TYPE IN type_to_spec_args: " + ttype->get_name());
throw std::runtime_error("INVALID TYPE IN to_spec_args: " + type->get_name());
}

string t_json_generator::to_spec_args(const t_service* service) {
return to_spec_args_named(service);
}

string t_json_generator::to_spec_args_named(const t_named* named) {
std::string module;
if (named->program() != program_) {
module = named->program()->name() + ".";
}
return "\"" + module + named->get_name() + "\"";
}

/**
Expand All @@ -336,12 +347,13 @@ string t_json_generator::type_name(const t_type* ttype) {
}

/**
* Prints out the provided type spec
* Prints out the provided type or service spec.
*/
void t_json_generator::print_type(const t_type* ttype) {
indent(f_out_) << R"("type_enum" : ")" << type_to_string(ttype) << "\","
<< endl;
indent(f_out_) << "\"spec_args\" : " << type_to_spec_args(ttype);
template <typename T>
void t_json_generator::print_spec(const T* type_or_service) {
indent(f_out_) << R"("type_enum" : ")" << to_string(type_or_service)
<< "\",\n";
indent(f_out_) << "\"spec_args\" : " << to_spec_args(type_or_service);
}

void t_json_generator::print_name(const string& name) {
Expand Down Expand Up @@ -533,7 +545,7 @@ void t_json_generator::generate_typedef(const t_typedef* ttypedef) {
indent(f_out_) << "\"" << ttypedef->get_name() << "\" : {" << endl;
indent_up();
print_lineno(*ttypedef);
print_type(ttypedef->get_type());
print_spec(ttypedef->get_type());
print_node_annotations(
*ttypedef,
/*add_heading_comma=*/true,
Expand Down Expand Up @@ -604,7 +616,7 @@ void t_json_generator::generate_const(const t_const* tconst) {
indent(f_out_) << "\"value\" : ";
print_const_value(tconst->value());
f_out_ << "," << endl;
print_type(tconst->type());
print_spec(tconst->type());
print_node_annotations(
*tconst,
/*add_heading_comma=*/true,
Expand Down Expand Up @@ -643,7 +655,7 @@ void t_json_generator::generate_struct(const t_structured* tstruct) {
}
indent(f_out_) << "\"" << (*mem_iter)->get_name() << "\" : {" << endl;
indent_up();
print_type((*mem_iter)->get_type());
print_spec((*mem_iter)->get_type());
f_out_ << "," << endl
<< indent() << "\"required\" : "
<< ((*mem_iter)->get_req() != t_field::e_req::optional ? "true"
Expand Down Expand Up @@ -693,7 +705,7 @@ void t_json_generator::generate_service(const t_service* tservice) {
if (tservice->get_extends()) {
indent(f_out_) << "\"extends\" : {" << endl;
indent_up();
print_type(tservice->get_extends());
print_spec(tservice->get_extends());
f_out_ << endl;
indent_down();
indent(f_out_) << "}";
Expand Down Expand Up @@ -722,9 +734,11 @@ void t_json_generator::generate_service(const t_service* tservice) {
indent(f_out_) << "\"return_type\" : {" << endl;
indent_up();
const t_function* fun = *fn_iter;
const auto& ret = fun->is_interaction_constructor() ? fun->interaction()
: fun->return_type();
print_type(ret.get_type());
if (fun->is_interaction_constructor()) {
print_spec(static_cast<const t_service*>(fun->interaction().get_type()));
} else {
print_spec(fun->return_type().get_type());
}
f_out_ << endl;
indent_down();
indent(f_out_) << "}," << endl;
Expand All @@ -742,7 +756,7 @@ void t_json_generator::generate_service(const t_service* tservice) {
indent(f_out_) << "{" << endl;
indent_up();
print_name((*arg_iter)->get_name());
print_type((*arg_iter)->get_type());
print_spec((*arg_iter)->get_type());
if ((*arg_iter)->get_value() != nullptr) {
f_out_ << "," << endl << indent() << "\"value\" : ";
print_const_value((*arg_iter)->get_value());
Expand Down Expand Up @@ -771,7 +785,7 @@ void t_json_generator::generate_service(const t_service* tservice) {
if (ex_iter != exceptions.begin()) {
f_out_ << "," << endl;
}
indent(f_out_) << type_to_spec_args((*ex_iter).get_type());
indent(f_out_) << to_spec_args((*ex_iter).get_type());
}
f_out_ << endl;
indent_down();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1127,7 +1127,7 @@ class cpp_mstch_type : public mstch_type {
return resolved_type_->is_container() || resolved_type_->is_enum();
}
mstch::node resolves_to_complex_return() {
return is_complex_return(resolved_type_) && !resolved_type_->is_service();
return is_complex_return(resolved_type_);
}
mstch::node resolves_to_fixed_size() {
return resolved_type_->is_bool() || resolved_type_->is_byte() ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ mstch::map t_mstch_generator::dump(const t_type& orig_type) {
{"struct?", type.is_struct() || type.is_exception()},
{"union?", type.is_union()},
{"enum?", type.is_enum()},
{"service?", type.is_service()},
{"base?", type.is_primitive_type()},
{"container?", type.is_container()},
{"list?", type.is_list()},
Expand All @@ -133,8 +132,6 @@ mstch::map t_mstch_generator::dump(const t_type& orig_type) {
result.emplace("struct", dump(dynamic_cast<const t_struct&>(type), true));
} else if (type.is_enum()) {
result.emplace("enum", dump(dynamic_cast<const t_enum&>(type)));
} else if (type.is_service()) {
result.emplace("service", dump(dynamic_cast<const t_service&>(type)));
} else if (type.is_list()) {
result.emplace(
"list_elem_type",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,10 @@ class t_py_generator : public t_concat_generator {
const t_structured* tstruct, const t_field* tfield);
std::string render_field_default_value(const t_field* tfield);
std::string type_name(const t_type* ttype);
std::string service_name(const t_service* service) {
return get_real_py_module(service->program()) + "." +
rename_reserved_keywords(service->get_name());
}
std::string function_signature(
const t_function* tfunction, std::string prefix = "");
std::string function_signature_if(
Expand Down Expand Up @@ -2315,7 +2319,7 @@ void t_py_generator::generate_service_interface(
string extends = "";
string extends_if = "";
if (tservice->get_extends() != nullptr) {
extends = type_name(tservice->get_extends());
extends = service_name(tservice->get_extends());
extends_if = "(" + extends + "." + iface_prefix + "Iface)";
}

Expand Down Expand Up @@ -2381,7 +2385,7 @@ void t_py_generator::generate_service_client(const t_service* tservice) {
string extends = "";
string extends_client = "";
if (tservice->get_extends() != nullptr) {
extends = type_name(tservice->get_extends());
extends = service_name(tservice->get_extends());
extends_client = extends + ".Client, ";
}

Expand Down Expand Up @@ -2927,7 +2931,7 @@ void t_py_generator::generate_service_server(
string extends = "";
string extends_processor = "";
if (tservice->get_extends() != nullptr) {
extends = type_name(tservice->get_extends());
extends = service_name(tservice->get_extends());
extends_processor = extends + "." + class_prefix + "Processor, ";
}

Expand Down Expand Up @@ -3781,10 +3785,6 @@ string t_py_generator::argument_list(const t_paramlist& tparamlist) {

string t_py_generator::type_name(const t_type* ttype) {
const t_program* program = ttype->program();
if (ttype->is_service()) {
return get_real_py_module(program) + "." +
rename_reserved_keywords(ttype->get_name());
}
if (program != nullptr && program != program_ &&
!program_->includes().empty()) {
return get_real_py_module(program) + ".ttypes." +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,4 @@
{{/function:stream_first_response_type}}
{{/function:stream_has_first_response?}}
}{{/function:stream?}}{{!
}}{{#type:service?}}{
"type" : "service",
"name" : "{{type:name}}"{{#type:external?}},
"path" : "{{type:file_name}}"{{/type:external?}}
}{{/type:service?}}{{!
}}{{/function:return_type}}
Loading

0 comments on commit cc96834

Please sign in to comment.