You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by je...@apache.org on 2021/03/29 06:55:33 UTC
[thrift] branch master updated: THRIFT-5341 Old-Style-Cast &
missing Override Client: cpp Patch: Kashirin Alex
This is an automated email from the ASF dual-hosted git repository.
jensg 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 7e53750 THRIFT-5341 Old-Style-Cast & missing Override Client: cpp Patch: Kashirin Alex
7e53750 is described below
commit 7e537505f014d8ede86f30a7500136c2f3e99505
Author: Kashirin Alex <ka...@gmail.com>
AuthorDate: Thu Jan 28 06:57:51 2021 +0100
THRIFT-5341 Old-Style-Cast & missing Override
Client: cpp
Patch: Kashirin Alex
This closes #2318
---
.../cpp/src/thrift/generate/t_cpp_generator.cc | 174 ++++++++++++++++-----
1 file changed, 134 insertions(+), 40 deletions(-)
diff --git a/compiler/cpp/src/thrift/generate/t_cpp_generator.cc b/compiler/cpp/src/thrift/generate/t_cpp_generator.cc
index ceaa11f..4ef6acf 100644
--- a/compiler/cpp/src/thrift/generate/t_cpp_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_cpp_generator.cc
@@ -292,6 +292,13 @@ public:
(ttype->annotations_.find("cpp.customostream") != ttype->annotations_.end());
}
+ /**
+ * Determine if all fields of t_struct's storage do not throw
+ * Move/Copy Constructors and Assignments applicable for 'noexcept'
+ * Move defaults to 'noexcept'
+ */
+ bool is_struct_storage_not_throwing(t_struct* tstruct) const;
+
private:
/**
* Returns the include prefix to use for a file generated by program, or the
@@ -954,11 +961,13 @@ void t_cpp_generator::generate_constructor_helper(ostream& out,
indent(out) << tstruct->get_name() << "::" << tstruct->get_name();
if (is_move) {
- out << "( " << tstruct->get_name() << "&& ";
+ out << "(" << tstruct->get_name() << "&& ";
} else {
out << "(const " << tstruct->get_name() << "& ";
}
out << tmp_name << ") ";
+ if(is_move || is_struct_storage_not_throwing(tstruct))
+ out << "noexcept ";
if (is_exception)
out << ": TException() ";
out << "{" << endl;
@@ -976,11 +985,14 @@ void t_cpp_generator::generate_constructor_helper(ostream& out,
if ((*f_iter)->get_req() != t_field::T_REQUIRED)
has_nonrequired_fields = true;
indent(out) << (*f_iter)->get_name() << " = "
- << maybeMove(tmp_name + "." + (*f_iter)->get_name(), is_move) << ";" << endl;
+ << maybeMove(
+ tmp_name + "." + (*f_iter)->get_name(),
+ is_move && is_complex_type((*f_iter)->get_type()))
+ << ";" << endl;
}
if (has_nonrequired_fields) {
- indent(out) << "__isset = " << maybeMove(tmp_name + ".__isset", is_move) << ";" << endl;
+ indent(out) << "__isset = " << maybeMove(tmp_name + ".__isset", false) << ";" << endl;
}
indent_down();
@@ -1005,8 +1017,10 @@ void t_cpp_generator::generate_assignment_helper(ostream& out, t_struct* tstruct
} else {
out << "const " << tstruct->get_name() << "& ";
}
- out << tmp_name << ") {" << endl;
-
+ out << tmp_name << ") ";
+ if(is_move || is_struct_storage_not_throwing(tstruct))
+ out << "noexcept ";
+ out << "{" << endl;
indent_up();
const vector<t_field*>& members = tstruct->get_members();
@@ -1021,10 +1035,13 @@ void t_cpp_generator::generate_assignment_helper(ostream& out, t_struct* tstruct
if ((*f_iter)->get_req() != t_field::T_REQUIRED)
has_nonrequired_fields = true;
indent(out) << (*f_iter)->get_name() << " = "
- << maybeMove(tmp_name + "." + (*f_iter)->get_name(), is_move) << ";" << endl;
+ << maybeMove(
+ tmp_name + "." + (*f_iter)->get_name(),
+ is_move && is_complex_type((*f_iter)->get_type()))
+ << ";" << endl;
}
if (has_nonrequired_fields) {
- indent(out) << "__isset = " << maybeMove(tmp_name + ".__isset", is_move) << ";" << endl;
+ indent(out) << "__isset = " << maybeMove(tmp_name + ".__isset", false) << ";" << endl;
}
indent(out) << "return *this;" << endl;
@@ -1111,47 +1128,71 @@ void t_cpp_generator::generate_struct_declaration(ostream& out,
indent_up();
if (!pointers) {
+ bool ok_noexcept = is_struct_storage_not_throwing(tstruct);
// Copy constructor
- indent(out) << tstruct->get_name() << "(const " << tstruct->get_name() << "&);" << endl;
+ indent(out) << tstruct->get_name() << "(const " << tstruct->get_name() << "&)"
+ << (ok_noexcept? " noexcept" : "") << ';' << endl;
// Move constructor
if (gen_moveable_) {
- indent(out) << tstruct->get_name() << "(" << tstruct->get_name() << "&&);" << endl;
+ indent(out) << tstruct->get_name() << "(" << tstruct->get_name() << "&&) noexcept;"
+ << endl;
}
// Assignment Operator
- indent(out) << tstruct->get_name() << "& operator=(const " << tstruct->get_name() << "&);"
- << endl;
+ indent(out) << tstruct->get_name() << "& operator=(const " << tstruct->get_name() << "&)"
+ << (ok_noexcept? " noexcept" : "") << ';' << endl;
// Move assignment operator
if (gen_moveable_) {
- indent(out) << tstruct->get_name() << "& operator=(" << tstruct->get_name() << "&&);" << endl;
+ indent(out) << tstruct->get_name() << "& operator=(" << tstruct->get_name() << "&&) noexcept;"
+ << endl;
+ }
+
+ bool has_default_value = false;
+ for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
+ t_type* t = get_true_type((*m_iter)->get_type());
+ if (is_reference(*m_iter) || t->is_string()) {
+ t_const_value* cv = (*m_iter)->get_value();
+ if (cv != nullptr) {
+ has_default_value = true;
+ break;
+ }
+ }
}
// Default constructor
- indent(out) << tstruct->get_name() << "()";
+ std::string clsname_ctor = tstruct->get_name() + "()";
+ indent(out) << clsname_ctor << (has_default_value ? "" : " noexcept");
bool init_ctor = false;
+ std::string args_indent(
+ indent().size() + clsname_ctor.size() + (has_default_value ? 3 : -1), ' ');
for (m_iter = members.begin(); m_iter != members.end(); ++m_iter) {
t_type* t = get_true_type((*m_iter)->get_type());
if (t->is_base_type() || t->is_enum() || is_reference(*m_iter)) {
string dval;
- if (t->is_enum()) {
- dval += "(" + type_name(t) + ")";
- }
- dval += (t->is_string() || is_reference(*m_iter)) ? "" : "0";
t_const_value* cv = (*m_iter)->get_value();
if (cv != nullptr) {
- dval = render_const_value(out, (*m_iter)->get_name(), t, cv);
+ dval += render_const_value(out, (*m_iter)->get_name(), t, cv);
+ } else if (t->is_enum()) {
+ dval += "static_cast<" + type_name(t) + ">(0)";
+ } else {
+ dval += (t->is_string() || is_reference(*m_iter)) ? "" : "0";
}
if (!init_ctor) {
init_ctor = true;
- out << " : ";
- out << (*m_iter)->get_name() << "(" << dval << ")";
+ if(has_default_value) {
+ out << " : ";
+ } else {
+ out << '\n' << args_indent << ": ";
+ args_indent.append(" ");
+ }
} else {
- out << ", " << (*m_iter)->get_name() << "(" << dval << ")";
+ out << ",\n" << args_indent;
}
+ out << (*m_iter)->get_name() << "(" << dval << ")";
}
}
out << " {" << endl;
@@ -1248,7 +1289,10 @@ void t_cpp_generator::generate_struct_declaration(ostream& out,
<< "uint32_t read(Protocol_* iprot);" << endl;
} else {
out << indent() << "uint32_t read("
- << "::apache::thrift::protocol::TProtocol* iprot);" << endl;
+ << "::apache::thrift::protocol::TProtocol* iprot)";
+ if(!is_exception && !extends.empty())
+ out << " override";
+ out << ';' << endl;
}
}
if (write) {
@@ -1257,7 +1301,10 @@ void t_cpp_generator::generate_struct_declaration(ostream& out,
<< "uint32_t write(Protocol_* oprot) const;" << endl;
} else {
out << indent() << "uint32_t write("
- << "::apache::thrift::protocol::TProtocol* oprot) const;" << endl;
+ << "::apache::thrift::protocol::TProtocol* oprot) const";
+ if(!is_exception && !extends.empty())
+ out << " override";
+ out << ';' << endl;
}
}
out << endl;
@@ -1697,6 +1744,8 @@ void t_cpp_generator::generate_exception_what_method_decl(std::ostream& out,
out << tstruct->get_name() << "::";
}
out << "what() const noexcept";
+ if(!external)
+ out << " override";
}
namespace struct_ostream_operator_generator {
@@ -2059,8 +2108,10 @@ void t_cpp_generator::generate_service_interface_factory(t_service* tservice, st
f_header_ << indent() << "typedef " << service_if_name << " Handler;" << endl << endl << indent()
<< "virtual ~" << factory_name << "() {}" << endl << endl << indent() << "virtual "
<< service_if_name << "* getHandler("
- << "const ::apache::thrift::TConnectionInfo& connInfo) = 0;" << endl << indent()
- << "virtual void releaseHandler(" << base_if_name << "* /* handler */) = 0;" << endl;
+ << "const ::apache::thrift::TConnectionInfo& connInfo)"
+ << (extends.empty() ? "" : " override") << " = 0;" << endl << indent()
+ << "virtual void releaseHandler(" << base_if_name << "* /* handler */)"
+ << (extends.empty() ? "" : " override") << " = 0;" << endl << indent();
indent_down();
f_header_ << "};" << endl << endl;
@@ -2074,9 +2125,9 @@ void t_cpp_generator::generate_service_interface_factory(t_service* tservice, st
<< ">& iface) : iface_(iface) {}" << endl << indent() << "virtual ~"
<< singleton_factory_name << "() {}" << endl << endl << indent() << "virtual "
<< service_if_name << "* getHandler("
- << "const ::apache::thrift::TConnectionInfo&) {" << endl << indent()
+ << "const ::apache::thrift::TConnectionInfo&) override {" << endl << indent()
<< " return iface_.get();" << endl << indent() << "}" << endl << indent()
- << "virtual void releaseHandler(" << base_if_name << "* /* handler */) {}" << endl;
+ << "virtual void releaseHandler(" << base_if_name << "* /* handler */) override {}" << endl;
f_header_ << endl << " protected:" << endl << indent() << "::std::shared_ptr<" << service_if_name
<< "> iface_;" << endl;
@@ -2102,7 +2153,8 @@ void t_cpp_generator::generate_service_null(t_service* tservice, string style) {
vector<t_function*> functions = tservice->get_functions();
vector<t_function*>::iterator f_iter;
for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
- f_header_ << indent() << function_signature(*f_iter, style, "", false) << " {" << endl;
+ f_header_ << indent() << function_signature(*f_iter, style, "", false)
+ << " override {" << endl;
indent_up();
t_type* returntype = (*f_iter)->get_returntype();
@@ -2340,7 +2392,7 @@ void t_cpp_generator::generate_service_multiface(t_service* tservice) {
}
call += ")";
- f_header_ << indent() << function_signature(*f_iter, "") << " {" << endl;
+ f_header_ << indent() << function_signature(*f_iter, "") << " override {" << endl;
indent_up();
f_header_ << indent() << "size_t sz = ifaces_.size();" << endl << indent() << "size_t i = 0;"
<< endl << indent() << "for (; i < (sz - 1); ++i) {" << endl;
@@ -2547,7 +2599,8 @@ void t_cpp_generator::generate_service_client(t_service* tservice, string style)
vector<t_function*>::const_iterator f_iter;
for (f_iter = functions.begin(); f_iter != functions.end(); ++f_iter) {
generate_java_doc(f_header_, *f_iter);
- indent(f_header_) << function_signature(*f_iter, ifstyle) << ";" << endl;
+ indent(f_header_) << function_signature(*f_iter, ifstyle)
+ << " override;" << endl;
// TODO(dreiss): Use private inheritance to avoid generating thise in cob-style.
if (style == "Concurrent" && !(*f_iter)->is_oneway()) {
// concurrent clients need to move the seqid from the send function to the
@@ -3096,7 +3149,8 @@ void ProcessorGenerator::generate_class_definition() {
f_header_ << indent() << "virtual " << ret_type_ << "dispatchCall(" << finish_cob_
<< "::apache::thrift::protocol::TProtocol* iprot, "
<< "::apache::thrift::protocol::TProtocol* oprot, "
- << "const std::string& fname, int32_t seqid" << call_context_ << ");" << endl;
+ << "const std::string& fname, int32_t seqid" << call_context_
+ << ") override;" << endl;
if (generator_->gen_templates_) {
f_header_ << indent() << "virtual " << ret_type_ << "dispatchCallTemplated(" << finish_cob_
<< "Protocol_* iprot, Protocol_* oprot, "
@@ -3309,11 +3363,12 @@ void ProcessorGenerator::generate_factory() {
indent_up();
f_header_ << indent() << factory_class_name_ << "(const ::std::shared_ptr< " << if_factory_name
- << " >& handlerFactory) :" << endl << indent()
+ << " >& handlerFactory) noexcept :" << endl << indent()
<< " handlerFactory_(handlerFactory) {}" << endl << endl << indent()
<< "::std::shared_ptr< ::apache::thrift::"
<< (style_ == "Cob" ? "async::TAsyncProcessor" : "TProcessor") << " > "
- << "getProcessor(const ::apache::thrift::TConnectionInfo& connInfo);" << endl;
+ << "getProcessor(const ::apache::thrift::TConnectionInfo& connInfo) override;"
+ << endl;
f_header_ << endl << " protected:" << endl << indent() << "::std::shared_ptr< "
<< if_factory_name << " > handlerFactory_;" << endl;
@@ -3506,9 +3561,9 @@ void t_cpp_generator::generate_process_function(t_service* tservice,
<< ") {" << endl;
if (!tfunction->is_oneway()) {
indent_up();
- out << indent() << "result." << (*x_iter)->get_name() << " = " << (*x_iter)->get_name()
- << ";" << endl << indent() << "result.__isset." << (*x_iter)->get_name() << " = true;"
- << endl;
+ out << indent() << "result." << (*x_iter)->get_name()
+ << " = std::move(" << (*x_iter)->get_name() << ");" << endl
+ << indent() << "result.__isset." << (*x_iter)->get_name() << " = true;" << endl;
indent_down();
out << indent() << "}";
} else {
@@ -3945,7 +4000,8 @@ void t_cpp_generator::generate_deserialize_field(ostream& out,
} else if (type->is_enum()) {
string t = tmp("ecast");
out << indent() << "int32_t " << t << ";" << endl << indent() << "xfer += iprot->readI32(" << t
- << ");" << endl << indent() << name << " = (" << type_name(type) << ")" << t << ";" << endl;
+ << ");" << endl << indent() << name << " = static_cast<"
+ << type_name(type) << ">(" << t << ");" << endl;
} else {
printf("DO NOT KNOW HOW TO DESERIALIZE FIELD '%s' TYPE '%s'\n",
tfield->get_name().c_str(),
@@ -4150,7 +4206,7 @@ void t_cpp_generator::generate_serialize_field(ostream& out,
+ name;
}
} else if (type->is_enum()) {
- out << "writeI32((int32_t)" << name << ");";
+ out << "writeI32(static_cast<int32_t>(" << name << "));";
}
out << endl;
} else {
@@ -4485,13 +4541,13 @@ string t_cpp_generator::declare_field(t_field* tfield,
result += " = 0";
break;
case t_base_type::TYPE_DOUBLE:
- result += " = (double)0";
+ result += " = 0.0";
break;
default:
throw "compiler error: no C++ initializer for base type " + t_base_type::t_base_name(tbase);
}
} else if (type->is_enum()) {
- result += " = (" + type_name(type) + ")0";
+ result += " = static_cast<" + type_name(type) + ">(0)";
}
}
if (!reference) {
@@ -4619,6 +4675,44 @@ string t_cpp_generator::type_to_enum(t_type* type) {
throw "INVALID TYPE IN type_to_enum: " + type->get_name();
}
+
+bool t_cpp_generator::is_struct_storage_not_throwing(t_struct* tstruct) const {
+ vector<t_field*> members = tstruct->get_members();
+
+ for(size_t i=0; i < members.size(); ++i) {
+ t_type* type = get_true_type(members[i]->get_type());
+
+ if(type->is_enum())
+ continue;
+ if(type->is_xception())
+ return false;
+ if(type->is_base_type()) switch(((t_base_type*)type)->get_base()) {
+ case t_base_type::TYPE_BOOL:
+ case t_base_type::TYPE_I8:
+ case t_base_type::TYPE_I16:
+ case t_base_type::TYPE_I32:
+ case t_base_type::TYPE_I64:
+ case t_base_type::TYPE_DOUBLE:
+ continue;
+ case t_base_type::TYPE_VOID:
+ case t_base_type::TYPE_STRING:
+ default:
+ return false;
+ }
+ if(type->is_struct()) {
+ const vector<t_field*>& more = ((t_struct*)type)->get_members();
+ for(auto it = more.begin(); it < more.end(); ++it) {
+ if(std::find(members.begin(), members.end(), *it) == members.end())
+ members.push_back(*it);
+ }
+ continue;
+ }
+ return false; // rest-as-throwing(require-alloc)
+ }
+ return true;
+}
+
+
string t_cpp_generator::get_include_prefix(const t_program& program) const {
string include_prefix = program.get_include_prefix();
if (!use_include_prefix_ || (include_prefix.size() > 0 && include_prefix[0] == '/')) {