Jens-G commented on code in PR #2943:
URL: https://github.com/apache/thrift/pull/2943#discussion_r1546279552
##########
compiler/cpp/src/thrift/generate/t_js_generator.cc:
##########
@@ -43,7 +43,7 @@ using std::stringstream;
using std::unordered_map;
using std::vector;
-static const string endl = "\n"; // avoid ostream << std::endl flushes
+static const string br = "\n"; // Line break
Review Comment:
Same here. If renaming endl to br was the only purpose why not everywhere?
##########
compiler/cpp/src/thrift/generate/t_dart_generator.cc:
##########
@@ -37,8 +37,7 @@ using std::string;
using std::stringstream;
using std::vector;
-static const string endl = "\n"; // avoid ostream << std::endl flushes
-static const string endl2 = "\n\n";
+static const string br = "\n"; // Line break
Review Comment:
We rename `endl `to `br `so that it does no longer is called endl ?
Seriously? Why is that? And why only here? Can we change that?
##########
test/cpp/src/TestServer.cpp:
##########
@@ -847,6 +847,6 @@ int main(int argc, char** argv) {
server.reset();
}
- cout << "done." << endl;
+ cout << "done." << '\n';
return 0;
Review Comment:
missed one in line 648 and 816
##########
compiler/cpp/src/thrift/generate/t_cpp_generator.cc:
##########
@@ -2902,187 +2900,187 @@ void
t_cpp_generator::generate_service_client(t_service* tservice, string style)
if (gen_templates_) {
indent(out) << template_header;
}
- indent(out) << function_signature(&recv_function, "", scope) << endl;
+ indent(out) << function_signature(&recv_function, "", scope) << '\n';
scope_up(out);
- out << endl <<
- indent() << "int32_t rseqid = 0;" << endl <<
- indent() << "std::string fname;" << endl <<
- indent() << "::apache::thrift::protocol::TMessageType mtype;" <<
endl;
+ out << '\n' <<
+ indent() << "int32_t rseqid = 0;" << '\n' <<
+ indent() << "std::string fname;" << '\n' <<
+ indent() << "::apache::thrift::protocol::TMessageType mtype;" <<
'\n';
if(style == "Concurrent") {
- out <<
- endl <<
- indent() << "// the read mutex gets dropped and reacquired as part
of waitForWork()" << endl <<
- indent() << "// The destructor of this sentry wakes up other
clients" << endl <<
- indent() << "::apache::thrift::async::TConcurrentRecvSentry
sentry(this->sync_.get(), seqid);" << endl;
+ out << '\n' <<
+ indent() << "// the read mutex gets dropped and reacquired as part
of waitForWork()" << '\n' <<
+ indent() << "// The destructor of this sentry wakes up other
clients" << '\n' <<
+ indent() << "::apache::thrift::async::TConcurrentRecvSentry
sentry(this->sync_.get(), seqid);" << '\n';
}
if (style == "Cob" && !gen_no_client_completion_) {
- out << indent() << "bool completed = false;" << endl << endl <<
indent() << "try {";
+ out << indent() << "bool completed = false;" << '\n' << '\n' <<
indent() << "try {";
indent_up();
}
- out << endl;
+ out << '\n';
if (style == "Concurrent") {
out <<
- indent() << "while(true) {" << endl <<
- indent() << " if(!this->sync_->getPending(fname, mtype, rseqid))
{" << endl;
+ indent() << "while(true) {" << '\n' <<
+ indent() << " if(!this->sync_->getPending(fname, mtype, rseqid))
{" << '\n';
indent_up();
indent_up();
}
out <<
- indent() << _this << "iprot_->readMessageBegin(fname, mtype,
rseqid);" << endl;
+ indent() << _this << "iprot_->readMessageBegin(fname, mtype,
rseqid);" << '\n';
if (style == "Concurrent") {
scope_down(out);
- out << indent() << "if(seqid == rseqid) {" << endl;
+ out << indent() << "if(seqid == rseqid) {" << '\n';
indent_up();
}
out <<
- indent() << "if (mtype == ::apache::thrift::protocol::T_EXCEPTION)
{" << endl <<
- indent() << " ::apache::thrift::TApplicationException x;" << endl <<
- indent() << " x.read(" << _this << "iprot_);" << endl <<
- indent() << " " << _this << "iprot_->readMessageEnd();" << endl <<
- indent() << " " << _this << "iprot_->getTransport()->readEnd();" <<
endl;
+ indent() << "if (mtype == ::apache::thrift::protocol::T_EXCEPTION)
{" << '\n' <<
+ indent() << " ::apache::thrift::TApplicationException x;" << '\n' <<
+ indent() << " x.read(" << _this << "iprot_);" << '\n' <<
+ indent() << " " << _this << "iprot_->readMessageEnd();" << '\n' <<
+ indent() << " " << _this << "iprot_->getTransport()->readEnd();" <<
'\n';
if (style == "Cob" && !gen_no_client_completion_) {
- out << indent() << " completed = true;" << endl << indent() << "
completed__(true);"
- << endl;
+ out << indent() << " completed = true;" << '\n' << indent() << "
completed__(true);"
+ << '\n';
}
if (style == "Concurrent") {
- out << indent() << " sentry.commit();" << endl;
+ out << indent() << " sentry.commit();" << '\n';
}
out <<
- indent() << " throw x;" << endl <<
- indent() << "}" << endl <<
- indent() << "if (mtype != ::apache::thrift::protocol::T_REPLY) {" <<
endl <<
- indent() << " " << _this << "iprot_->skip(" <<
"::apache::thrift::protocol::T_STRUCT);" << endl <<
- indent() << " " << _this << "iprot_->readMessageEnd();" << endl <<
- indent() << " " << _this << "iprot_->getTransport()->readEnd();" <<
endl;
+ indent() << " throw x;" << '\n' <<
+ indent() << "}" << '\n' <<
+ indent() << "if (mtype != ::apache::thrift::protocol::T_REPLY) {" <<
'\n' <<
+ indent() << " " << _this << "iprot_->skip(" <<
"::apache::thrift::protocol::T_STRUCT);" << '\n' <<
+ indent() << " " << _this << "iprot_->readMessageEnd();" << '\n' <<
+ indent() << " " << _this << "iprot_->getTransport()->readEnd();" <<
'\n';
if (style == "Cob" && !gen_no_client_completion_) {
- out << indent() << " completed = true;" << endl << indent() << "
completed__(false);"
- << endl;
+ out << indent() << " completed = true;" << '\n' << indent() << "
completed__(false);"
+ << '\n';
}
out <<
- indent() << "}" << endl <<
- indent() << "if (fname.compare(\"" << (*f_iter)->get_name() << "\")
!= 0) {" << endl <<
- indent() << " " << _this << "iprot_->skip(" <<
"::apache::thrift::protocol::T_STRUCT);" << endl <<
- indent() << " " << _this << "iprot_->readMessageEnd();" << endl <<
- indent() << " " << _this << "iprot_->getTransport()->readEnd();" <<
endl;
+ indent() << "}" << '\n' <<
+ indent() << "if (fname.compare(\"" << (*f_iter)->get_name() << "\")
!= 0) {" << '\n' <<
+ indent() << " " << _this << "iprot_->skip(" <<
"::apache::thrift::protocol::T_STRUCT);" << '\n' <<
+ indent() << " " << _this << "iprot_->readMessageEnd();" << '\n' <<
+ indent() << " " << _this << "iprot_->getTransport()->readEnd();" <<
'\n';
if (style == "Cob" && !gen_no_client_completion_) {
- out << indent() << " completed = true;" << endl << indent() << "
completed__(false);"
- << endl;
+ out << indent() << " completed = true;" << '\n' << indent() << "
completed__(false);"
+ << '\n';
}
if (style == "Concurrent") {
- out << endl <<
- indent() << " // in a bad state, don't commit" << endl <<
- indent() << " using
::apache::thrift::protocol::TProtocolException;" << endl <<
- indent() << " throw
TProtocolException(TProtocolException::INVALID_DATA);" << endl;
+ out << '\n' <<
+ indent() << " // in a bad state, don't commit" << '\n' <<
+ indent() << " using
::apache::thrift::protocol::TProtocolException;" << '\n' <<
+ indent() << " throw
TProtocolException(TProtocolException::INVALID_DATA);" << '\n';
}
- out << indent() << "}" << endl;
+ out << indent() << "}" << '\n';
if (!(*f_iter)->get_returntype()->is_void()
&& !is_complex_type((*f_iter)->get_returntype())) {
t_field returnfield((*f_iter)->get_returntype(), "_return");
- out << indent() << declare_field(&returnfield) << endl;
+ out << indent() << declare_field(&returnfield) << '\n';
}
- out << indent() << resultname << " result;" << endl;
+ out << indent() << resultname << " result;" << '\n';
if (!(*f_iter)->get_returntype()->is_void()) {
- out << indent() << "result.success = &_return;" << endl;
+ out << indent() << "result.success = &_return;" << '\n';
}
- out << indent() << "result.read(" << _this << "iprot_);" << endl <<
indent() << _this
- << "iprot_->readMessageEnd();" << endl << indent() << _this
- << "iprot_->getTransport()->readEnd();" << endl << endl;
+ out << indent() << "result.read(" << _this << "iprot_);" << '\n' <<
indent() << _this
+ << "iprot_->readMessageEnd();" << '\n' << indent() << _this
+ << "iprot_->getTransport()->readEnd();" << '\n' << '\n';
// Careful, only look for _result if not a void function
if (!(*f_iter)->get_returntype()->is_void()) {
if (is_complex_type((*f_iter)->get_returntype())) {
out <<
- indent() << "if (result.__isset.success) {" << endl;
+ indent() << "if (result.__isset.success) {" << '\n';
out <<
- indent() << " // _return pointer has now been filled" << endl;
+ indent() << " // _return pointer has now been filled" << '\n';
if (style == "Cob" && !gen_no_client_completion_) {
- out << indent() << " completed = true;" << endl << indent() <<
" completed__(true);"
- << endl;
+ out << indent() << " completed = true;" << '\n' << indent() <<
" completed__(true);"
+ << '\n';
}
if (style == "Concurrent") {
- out << indent() << " sentry.commit();" << endl;
+ out << indent() << " sentry.commit();" << '\n';
}
out <<
- indent() << " return;" << endl <<
- indent() << "}" << endl;
+ indent() << " return;" << '\n' <<
+ indent() << "}" << '\n';
} else {
- out << indent() << "if (result.__isset.success) {" << endl;
+ out << indent() << "if (result.__isset.success) {" << '\n';
if (style == "Cob" && !gen_no_client_completion_) {
- out << indent() << " completed = true;" << endl << indent() <<
" completed__(true);"
- << endl;
+ out << indent() << " completed = true;" << '\n' << indent() <<
" completed__(true);"
+ << '\n';
}
if (style == "Concurrent") {
- out << indent() << " sentry.commit();" << endl;
+ out << indent() << " sentry.commit();" << '\n';
}
- out << indent() << " return _return;" << endl << indent() << "}"
<< endl;
+ out << indent() << " return _return;" << '\n' << indent() << "}"
<< '\n';
}
}
t_struct* xs = (*f_iter)->get_xceptions();
const std::vector<t_field*>& xceptions = xs->get_members();
vector<t_field*>::const_iterator x_iter;
for (x_iter = xceptions.begin(); x_iter != xceptions.end(); ++x_iter) {
- out << indent() << "if (result.__isset." << (*x_iter)->get_name() <<
") {" << endl;
+ out << indent() << "if (result.__isset." << (*x_iter)->get_name() <<
") {" << '\n';
if (style == "Cob" && !gen_no_client_completion_) {
- out << indent() << " completed = true;" << endl << indent() << "
completed__(true);"
- << endl;
+ out << indent() << " completed = true;" << '\n' << indent() << "
completed__(true);"
+ << '\n';
}
if (style == "Concurrent") {
- out << indent() << " sentry.commit();" << endl;
+ out << indent() << " sentry.commit();" << '\n';
}
- out << indent() << " throw result." << (*x_iter)->get_name() << ";"
<< endl << indent()
- << "}" << endl;
+ out << indent() << " throw result." << (*x_iter)->get_name() << ";"
<< '\n' << indent()
+ << "}" << '\n';
}
// We only get here if we are a void function
if ((*f_iter)->get_returntype()->is_void()) {
if (style == "Cob" && !gen_no_client_completion_) {
- out << indent() << "completed = true;" << endl << indent() <<
"completed__(true);"
- << endl;
+ out << indent() << "completed = true;" << '\n' << indent() <<
"completed__(true);"
+ << '\n';
}
if (style == "Concurrent") {
- out << indent() << "sentry.commit();" << endl;
+ out << indent() << "sentry.commit();" << '\n';
}
- indent(out) << "return;" << endl;
+ indent(out) << "return;" << '\n';
} else {
if (style == "Cob" && !gen_no_client_completion_) {
- out << indent() << "completed = true;" << endl << indent() <<
"completed__(true);"
- << endl;
+ out << indent() << "completed = true;" << '\n' << indent() <<
"completed__(true);"
+ << '\n';
}
if (style == "Concurrent") {
- out << indent() << "// in a bad state, don't commit" << endl;
+ out << indent() << "// in a bad state, don't commit" << '\n';
}
out << indent() << "throw "
"::apache::thrift::TApplicationException(::apache::thrift::"
"TApplicationException::MISSING_RESULT, \"" <<
(*f_iter)->get_name()
- << " failed: unknown result\");" << endl;
+ << " failed: unknown result\");" << '\n';
}
if (style == "Concurrent") {
indent_down();
indent_down();
- out <<
- indent() << " }" << endl <<
- indent() << " // seqid != rseqid" << endl <<
- indent() << " this->sync_->updatePending(fname, mtype, rseqid);"
<< endl <<
- endl <<
- indent() << " // this will temporarily unlock the readMutex, and
let other clients get work done" << endl <<
- indent() << " this->sync_->waitForWork(seqid);" << endl <<
- indent() << "} // end while(true)" << endl;
+ out << indent() << " }" << '\n'
+ << indent() << " // seqid != rseqid" << '\n'
+ << indent() << " this->sync_->updatePending(fname, mtype,
rseqid);" << '\n'
+ << '\n'
+ << indent()
+ << " // this will temporarily unlock the readMutex, and let
other clients get work "
+ "done" << '\n'
Review Comment:
Every time I stop here ... for the sake of this PR I would really like it to
keep it in one line (instead of 3 lines) as before.
##########
test/cpp/src/StressTest.cpp:
##########
@@ -280,23 +280,23 @@ int main(int argc, char** argv) {
usage << argv[0] << " [--port=<port number>] [--server]
[--server-type=<server-type>] "
"[--protocol-type=<protocol-type>]
[--workers=<worker-count>] "
"[--clients=<client-count>] [--loop=<loop-count>] "
- "[--client-type=<client-type>]" << endl
+ "[--client-type=<client-type>]" << '\n'
<< "\tclients Number of client threads to create - 0 implies no
clients, i.e. "
- "server only. Default is " << clientCount << endl
- << "\thelp Prints this help text." << endl
- << "\tcall Service method to call. Default is " << callName
<< endl
- << "\tloop The number of remote thrift calls each client
makes. Default is " << loopCount << endl
+ "server only. Default is " << clientCount << '\n'
+ << "\thelp Prints this help text." << '\n'
+ << "\tcall Service method to call. Default is " << callName
<< '\n'
+ << "\tloop The number of remote thrift calls each client
makes. Default is " << loopCount << '\n'
<< "\tport The port the server and clients should bind to "
- "for thrift network connections. Default is " <<
port << endl
- << "\tserver Run the Thrift server in this process. Default
is " << runServer << endl
- << "\tserver-type Type of server, \"simple\" or \"thread-pool\".
Default is " << serverType << endl
- << "\tprotocol-type Type of protocol, \"binary\", \"ascii\", or
\"xml\". Default is " << protocolType << endl
- << "\tlog-request Log all request to ./requestlog.tlog. Default is
" << logRequests << endl
- << "\treplay-request Replay requests from log file (./requestlog.tlog)
Default is " << replayRequests << endl
+ "for thrift network connections. Default is " <<
port << '\n'
+ << "\tserver Run the Thrift server in this process. Default
is " << runServer << '\n'
+ << "\tserver-type Type of server, \"simple\" or \"thread-pool\".
Default is " << serverType << '\n'
+ << "\tprotocol-type Type of protocol, \"binary\", \"ascii\", or
\"xml\". Default is " << protocolType << '\n'
+ << "\tlog-request Log all request to ./requestlog.tlog. Default is
" << logRequests << '\n'
+ << "\treplay-request Replay requests from log file (./requestlog.tlog)
Default is " << replayRequests << '\n'
<< "\tworkers Number of thread pools workers. Only valid "
- "for thread-pool server type. Default is " <<
workerCount << endl
- << "\tclient-type Type of client, \"regular\" or \"concurrent\".
Default is " << clientType << endl
- << endl;
+ "for thread-pool server type. Default is " <<
workerCount << '\n'
+ << "\tclient-type Type of client, \"regular\" or \"concurrent\".
Default is " << clientType << '\n'
+ << '\n';
Review Comment:
missed about 4 more occurrences in that file
##########
compiler/cpp/src/thrift/generate/t_kotlin_generator.cc:
##########
@@ -1972,16 +1970,15 @@ bool t_kotlin_generator::is_enum_map(t_type* ttype) {
*/
string t_kotlin_generator::kotlin_package() {
if (!package_name_.empty()) {
- return string("package ") + package_name_ + endl + endl;
+ return string("package ") + package_name_ + string{"\n\n"};
}
Review Comment:
`string{"\n\n"}` instead of `string("\n\n")`?
##########
lib/cpp/test/OneWayHTTPTest.cpp:
##########
@@ -55,9 +55,6 @@ using apache::thrift::transport::TServerSocket;
using apache::thrift::transport::TSocket;
using apache::thrift::transport::TTransportException;
using std::shared_ptr;
-using std::cout;
-using std::cerr;
-using std::endl;
using std::string;
Review Comment:
line 70 and 75 and 201
##########
contrib/fb303/cpp/ServiceTracker.cpp:
##########
@@ -344,7 +344,7 @@ ServiceTracker::defaultLogMethod(int level, const string
&message)
break;
}
cout << '[' << level_string << "] [" << now_pretty << "] "
- << message << endl;
+ << message << '\n';
}
Review Comment:
There is also TClientInfo.cpp
##########
lib/cpp/test/concurrency/Tests.cpp:
##########
@@ -163,23 +163,23 @@ int main(int argc, char** argv) {
ThreadManagerTests threadManagerTests;
- std::cout << "\t\tThreadManager api test:" << std::endl;
+ std::cout << "\t\tThreadManager api test:" << '\n';
if (!threadManagerTests.apiTest()) {
std::cerr << "\t\tThreadManager apiTest FAILED" << std::endl;
return 1;
}
std::cout << "\t\tThreadManager load test: worker count: " << workerCount
- << " task count: " << taskCount << " delay: " << delay <<
std::endl;
+ << " task count: " << taskCount << " delay: " << delay << '\n';
if (!threadManagerTests.loadTest(taskCount, delay, workerCount)) {
std::cerr << "\t\tThreadManager loadTest FAILED" << std::endl;
return 1;
}
std::cout << "\t\tThreadManager block test: worker count: " <<
workerCount
- << " delay: " << delay << std::endl;
+ << " delay: " << delay << '\n';
if (!threadManagerTests.blockTest(delay, workerCount)) {
std::cerr << "\t\tThreadManager blockTest FAILED" << std::endl;
Review Comment:
you missed a bunch in that file
##########
test/cpp/src/StressTestNonBlocking.cpp:
##########
@@ -528,7 +524,7 @@ int main(int argc, char** argv) {
averageTime /= clientCount;
cout << "workers :" << workerCount << ", client : " << clientCount << ",
loops : " << loopCount
- << ", rate : " << (clientCount * loopCount * 1000) / ((double)(time01
- time00)) << endl;
+ << ", rate : " << (clientCount * loopCount * 1000) / ((double)(time01
- time00)) << '\n';
count_map count = serviceHandler->getCount();
count_map::iterator iter;
Review Comment:
missed one in line 155, 337, 415, 477, 534
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
