Merge pull request #15681 from ncteisen/even-more-clang

Even More Clang Tidy: performance-*

.clang-tidy

@@ -1,6 +1,6 @@
 ---
-Checks: 'modernize-use-nullptr,google-build-namespaces,google-build-explicit-make-pair,readability-function-size'
-WarningsAsErrors: 'modernize-use-nullptr,google-build-namespaces,google-build-explicit-make-pair,readability-function-size'
+Checks: 'modernize-use-nullptr,google-build-namespaces,google-build-explicit-make-pair,readability-function-size,performance-*'
+WarningsAsErrors: 'modernize-use-nullptr,google-build-namespaces,google-build-explicit-make-pair,readability-function-size,performance-*'
 CheckOptions:
   - key:    readability-function-size.StatementThreshold
     value:  '450'

src/cpp/server/server_builder.cc

@@ -24,6 +24,8 @@
 #include <grpcpp/resource_quota.h>
 #include <grpcpp/server.h>
 
+#include <utility>
+
 #include "src/core/lib/gpr/useful.h"
 #include "src/cpp/server/thread_pool_interface.h"
 
@@ -166,7 +168,7 @@ ServerBuilder& ServerBuilder::AddListeningPort(
     while (addr_uri[pos] == '/') ++pos;  // Skip slashes.
     addr = addr_uri.substr(pos);
   }
-  Port port = {addr, creds, selected_port};
+  Port port = {addr, std::move(creds), selected_port};
   ports_.push_back(port);
   return *this;
 }

src/cpp/server/server_cc.cc

@@ -219,7 +219,7 @@ class Server::SyncRequest final : public internal::CompletionQueueTag {
       }
     }
 
-    void Run(std::shared_ptr<GlobalCallbacks> global_callbacks) {
+    void Run(const std::shared_ptr<GlobalCallbacks>& global_callbacks) {
       ctx_.BeginCompletionOp(&call_);
       global_callbacks->PreSynchronousRequest(&ctx_);
       method_->handler()->RunHandler(internal::MethodHandler::HandlerParameter(
@@ -272,7 +272,7 @@ class Server::SyncRequestThreadManager : public ThreadManager {
         server_(server),
         server_cq_(server_cq),
         cq_timeout_msec_(cq_timeout_msec),
-        global_callbacks_(global_callbacks) {}
+        global_callbacks_(std::move(global_callbacks)) {}
 
   WorkStatus PollForWork(void** tag, bool* ok) override {
     *tag = nullptr;
@@ -378,7 +378,7 @@ Server::Server(
         sync_server_cqs,
     int min_pollers, int max_pollers, int sync_cq_timeout_msec)
     : max_receive_message_size_(max_receive_message_size),
-      sync_server_cqs_(sync_server_cqs),
+      sync_server_cqs_(std::move(sync_server_cqs)),
       started_(false),
       shutdown_(false),
       shutdown_notified_(false),

test/core/channel/channel_trace_test.cc

@@ -77,13 +77,13 @@ void ValidateChannelTraceData(grpc_json* json,
   ValidateJsonArraySize(json, "events", actual_num_events_expected);
 }
 
-void AddSimpleTrace(RefCountedPtr<ChannelTrace> tracer) {
+void AddSimpleTrace(const RefCountedPtr<ChannelTrace>& tracer) {
   tracer->AddTraceEvent(ChannelTrace::Severity::Info,
                         grpc_slice_from_static_string("simple trace"));
 }
 
 // checks for the existence of all the required members of the tracer.
-void ValidateChannelTrace(RefCountedPtr<ChannelTrace> tracer,
+void ValidateChannelTrace(const RefCountedPtr<ChannelTrace>& tracer,
                           size_t expected_num_event_logged, size_t max_nodes) {
   if (!max_nodes) return;
   char* json_str = tracer->RenderTrace();
@@ -95,7 +95,8 @@ void ValidateChannelTrace(RefCountedPtr<ChannelTrace> tracer,
   gpr_free(json_str);
 }
 
-void ValidateTraceDataMatchedUuidLookup(RefCountedPtr<ChannelTrace> tracer) {
+void ValidateTraceDataMatchedUuidLookup(
+    const RefCountedPtr<ChannelTrace>& tracer) {
   intptr_t uuid = tracer->GetUuid();
   if (uuid == -1) return;  // Doesn't make sense to lookup if tracing disabled
   char* tracer_json_str = tracer->RenderTrace();

test/core/channel/channelz_registry_test.cc

@@ -54,6 +54,7 @@ TEST(ChannelzRegistryTest, UuidStartsAboveZeroTest) {
 TEST(ChannelzRegistryTest, UuidsAreIncreasing) {
   int object_to_register;
   std::vector<intptr_t> uuids;
+  uuids.reserve(10);
   for (int i = 0; i < 10; ++i) {
     // reregister the same object. It's ok since we are just testing uuids
     uuids.push_back(ChannelzRegistry::Register(&object_to_register));

test/core/gprpp/ref_counted_ptr_test.cc

@@ -66,14 +66,14 @@ TEST(RefCountedPtr, MoveAssignment) {
 
 TEST(RefCountedPtr, CopyConstructor) {
   RefCountedPtr<Foo> foo(New<Foo>());
-  RefCountedPtr<Foo> foo2(foo);
+  const RefCountedPtr<Foo>& foo2(foo);
   EXPECT_NE(nullptr, foo.get());
   EXPECT_EQ(foo.get(), foo2.get());
 }
 
 TEST(RefCountedPtr, CopyAssignment) {
   RefCountedPtr<Foo> foo(New<Foo>());
-  RefCountedPtr<Foo> foo2 = foo;
+  const RefCountedPtr<Foo>& foo2 = foo;
   EXPECT_NE(nullptr, foo.get());
   EXPECT_EQ(foo.get(), foo2.get());
 }

test/cpp/codegen/golden_file_test.cc

@@ -37,8 +37,8 @@ DEFINE_string(
 const char kGoldenFilePath[] = "test/cpp/codegen/compiler_test_golden";
 const char kMockGoldenFilePath[] = "test/cpp/codegen/compiler_test_mock_golden";
 
-void run_test(std::basic_string<char> generated_file,
-              std::basic_string<char> golden_file) {
+void run_test(const std::basic_string<char>& generated_file,
+              const std::basic_string<char>& golden_file) {
   std::ifstream generated(generated_file);
   std::ifstream golden(golden_file);
 

test/cpp/end2end/async_end2end_test.cc

@@ -142,7 +142,7 @@ class Verifier {
   // to call the lambda
   void Verify(CompletionQueue* cq,
               std::chrono::system_clock::time_point deadline,
-              std::function<void(void)> lambda) {
+              const std::function<void(void)>& lambda) {
     if (expectations_.empty()) {
       bool ok;
       void* got_tag;
@@ -891,7 +891,7 @@ TEST_P(AsyncEnd2endTest, ClientInitialMetadataRpc) {
                         cq_.get(), tag(2));
   Verifier().Expect(2, true).Verify(cq_.get());
   EXPECT_EQ(send_request.message(), recv_request.message());
-  auto client_initial_metadata = srv_ctx.client_metadata();
+  const auto& client_initial_metadata = srv_ctx.client_metadata();
   EXPECT_EQ(meta1.second,
             ToString(client_initial_metadata.find(meta1.first)->second));
   EXPECT_EQ(meta2.second,
@@ -937,7 +937,7 @@ TEST_P(AsyncEnd2endTest, ServerInitialMetadataRpc) {
   srv_ctx.AddInitialMetadata(meta2.first, meta2.second);
   response_writer.SendInitialMetadata(tag(3));
   Verifier().Expect(3, true).Expect(4, true).Verify(cq_.get());
-  auto server_initial_metadata = cli_ctx.GetServerInitialMetadata();
+  const auto& server_initial_metadata = cli_ctx.GetServerInitialMetadata();
   EXPECT_EQ(meta1.second,
             ToString(server_initial_metadata.find(meta1.first)->second));
   EXPECT_EQ(meta2.second,
@@ -990,7 +990,7 @@ TEST_P(AsyncEnd2endTest, ServerTrailingMetadataRpc) {
 
   EXPECT_EQ(send_response.message(), recv_response.message());
   EXPECT_TRUE(recv_status.ok());
-  auto server_trailing_metadata = cli_ctx.GetServerTrailingMetadata();
+  const auto& server_trailing_metadata = cli_ctx.GetServerTrailingMetadata();
   EXPECT_EQ(meta1.second,
             ToString(server_trailing_metadata.find(meta1.first)->second));
   EXPECT_EQ(meta2.second,
@@ -1038,7 +1038,7 @@ TEST_P(AsyncEnd2endTest, MetadataRpc) {
                         cq_.get(), tag(2));
   Verifier().Expect(2, true).Verify(cq_.get());
   EXPECT_EQ(send_request.message(), recv_request.message());
-  auto client_initial_metadata = srv_ctx.client_metadata();
+  const auto& client_initial_metadata = srv_ctx.client_metadata();
   EXPECT_EQ(meta1.second,
             ToString(client_initial_metadata.find(meta1.first)->second));
   EXPECT_EQ(meta2.second,
@@ -1049,7 +1049,7 @@ TEST_P(AsyncEnd2endTest, MetadataRpc) {
   srv_ctx.AddInitialMetadata(meta4.first, meta4.second);
   response_writer.SendInitialMetadata(tag(3));
   Verifier().Expect(3, true).Expect(4, true).Verify(cq_.get());
-  auto server_initial_metadata = cli_ctx.GetServerInitialMetadata();
+  const auto& server_initial_metadata = cli_ctx.GetServerInitialMetadata();
   EXPECT_EQ(meta3.second,
             ToString(server_initial_metadata.find(meta3.first)->second));
   EXPECT_EQ(meta4.second,
@@ -1066,7 +1066,7 @@ TEST_P(AsyncEnd2endTest, MetadataRpc) {
 
   EXPECT_EQ(send_response.message(), recv_response.message());
   EXPECT_TRUE(recv_status.ok());
-  auto server_trailing_metadata = cli_ctx.GetServerTrailingMetadata();
+  const auto& server_trailing_metadata = cli_ctx.GetServerTrailingMetadata();
   EXPECT_EQ(meta5.second,
             ToString(server_trailing_metadata.find(meta5.first)->second));
   EXPECT_EQ(meta6.second,
@@ -1144,7 +1144,7 @@ TEST_P(AsyncEnd2endTest, ServerCheckDone) {
 
 TEST_P(AsyncEnd2endTest, UnimplementedRpc) {
   ChannelArguments args;
-  auto channel_creds = GetCredentialsProvider()->GetChannelCredentials(
+  const auto& channel_creds = GetCredentialsProvider()->GetChannelCredentials(
       GetParam().credentials_type, &args);
   std::shared_ptr<Channel> channel =
       !(GetParam().inproc)

test/cpp/end2end/client_lb_end2end_test.cc

@@ -686,9 +686,11 @@ TEST_F(ClientLbEnd2endTest, RoundRobinReresolve) {
   const int kNumServers = 3;
   std::vector<int> first_ports;
   std::vector<int> second_ports;
+  first_ports.reserve(kNumServers);
   for (int i = 0; i < kNumServers; ++i) {
     first_ports.push_back(grpc_pick_unused_port_or_die());
   }
+  second_ports.reserve(kNumServers);
   for (int i = 0; i < kNumServers; ++i) {
     second_ports.push_back(grpc_pick_unused_port_or_die());
   }

test/cpp/end2end/end2end_test.cc

@@ -71,8 +71,8 @@ class TestMetadataCredentialsPlugin : public MetadataCredentialsPlugin {
   static const char kGoodMetadataKey[];
   static const char kBadMetadataKey[];
 
-  TestMetadataCredentialsPlugin(grpc::string_ref metadata_key,
-                                grpc::string_ref metadata_value,
+  TestMetadataCredentialsPlugin(const grpc::string_ref& metadata_key,
+                                const grpc::string_ref& metadata_value,
                                 bool is_blocking, bool is_successful)
       : metadata_key_(metadata_key.data(), metadata_key.length()),
         metadata_value_(metadata_value.data(), metadata_value.length()),
@@ -168,7 +168,7 @@ const char TestAuthMetadataProcessor::kIdentityPropName[] = "novel identity";
 
 class Proxy : public ::grpc::testing::EchoTestService::Service {
  public:
-  Proxy(std::shared_ptr<Channel> channel)
+  Proxy(const std::shared_ptr<Channel>& channel)
       : stub_(grpc::testing::EchoTestService::NewStub(channel)) {}
 
   Status Echo(ServerContext* server_context, const EchoRequest* request,
@@ -683,6 +683,7 @@ TEST_P(End2endTest, SimpleRpcWithCustomUserAgentPrefix) {
 TEST_P(End2endTest, MultipleRpcsWithVariedBinaryMetadataValue) {
   ResetStub();
   std::vector<std::thread> threads;
+  threads.reserve(10);
   for (int i = 0; i < 10; ++i) {
     threads.emplace_back(SendRpc, stub_.get(), 10, true);
   }
@@ -694,6 +695,7 @@ TEST_P(End2endTest, MultipleRpcsWithVariedBinaryMetadataValue) {
 TEST_P(End2endTest, MultipleRpcs) {
   ResetStub();
   std::vector<std::thread> threads;
+  threads.reserve(10);
   for (int i = 0; i < 10; ++i) {
     threads.emplace_back(SendRpc, stub_.get(), 10, false);
   }
@@ -1272,6 +1274,7 @@ TEST_P(ProxyEnd2endTest, SimpleRpcWithEmptyMessages) {
 TEST_P(ProxyEnd2endTest, MultipleRpcs) {
   ResetStub();
   std::vector<std::thread> threads;
+  threads.reserve(10);
   for (int i = 0; i < 10; ++i) {
     threads.emplace_back(SendRpc, stub_.get(), 10, false);
   }

test/cpp/end2end/mock_test.cc

@@ -186,7 +186,7 @@ class TestServiceImpl : public EchoTestService::Service {
                         ServerWriter<EchoResponse>* writer) override {
     EchoResponse response;
     vector<grpc::string> tokens = split(request->message());
-    for (grpc::string token : tokens) {
+    for (const grpc::string& token : tokens) {
       response.set_message(token);
       writer->Write(response);
     }

test/cpp/end2end/thread_stress_test.cc

@@ -322,6 +322,7 @@ TYPED_TEST_CASE(End2endTest, CommonTypes);
 TYPED_TEST(End2endTest, ThreadStress) {
   this->common_.ResetStub();
   std::vector<std::thread> threads;
+  threads.reserve(kNumThreads);
   for (int i = 0; i < kNumThreads; ++i) {
     threads.emplace_back(SendRpc, this->common_.GetStub(), kNumRpcs);
   }

test/cpp/interop/http2_client.cc

@@ -42,16 +42,16 @@ const int kLargeRequestSize = 271828;
 const int kLargeResponseSize = 314159;
 }  // namespace
 
-Http2Client::ServiceStub::ServiceStub(std::shared_ptr<Channel> channel)
-    : channel_(channel) {
+Http2Client::ServiceStub::ServiceStub(const std::shared_ptr<Channel>& channel)
+    : channel_(std::move(channel)) {
   stub_ = TestService::NewStub(channel);
 }
 
 TestService::Stub* Http2Client::ServiceStub::Get() { return stub_.get(); }
 
-Http2Client::Http2Client(std::shared_ptr<Channel> channel)
+Http2Client::Http2Client(const std::shared_ptr<Channel>& channel)
     : serviceStub_(channel),
-      channel_(channel),
+      channel_(std::move(channel)),
       defaultRequest_(BuildDefaultRequest()) {}
 
 bool Http2Client::AssertStatusCode(const Status& s, StatusCode expected_code) {
@@ -140,7 +140,8 @@ bool Http2Client::DoPing() {
   return true;
 }
 
-void Http2Client::MaxStreamsWorker(std::shared_ptr<grpc::Channel> channel) {
+void Http2Client::MaxStreamsWorker(
+    const std::shared_ptr<grpc::Channel>& channel) {
   SimpleResponse response;
   AssertStatusCode(SendUnaryCall(&response), grpc::StatusCode::OK);
   GPR_ASSERT(response.payload().body() ==

test/cpp/interop/http2_client.h

@@ -31,7 +31,7 @@ namespace testing {
 
 class Http2Client {
  public:
-  explicit Http2Client(std::shared_ptr<Channel> channel);
+  explicit Http2Client(const std::shared_ptr<Channel>& channel);
   ~Http2Client() {}
 
   bool DoRstAfterHeader();
@@ -44,7 +44,7 @@ class Http2Client {
  private:
   class ServiceStub {
    public:
-    ServiceStub(std::shared_ptr<Channel> channel);
+    ServiceStub(const std::shared_ptr<Channel>& channel);
 
     TestService::Stub* Get();
 
@@ -53,7 +53,7 @@ class Http2Client {
     std::shared_ptr<Channel> channel_;
   };
 
-  void MaxStreamsWorker(std::shared_ptr<grpc::Channel> channel);
+  void MaxStreamsWorker(const std::shared_ptr<grpc::Channel>& channel);
   bool AssertStatusCode(const Status& s, StatusCode expected_code);
   Status SendUnaryCall(SimpleResponse* response);
   SimpleRequest BuildDefaultRequest();

test/cpp/interop/interop_client.cc

@@ -19,6 +19,7 @@
 #include <cinttypes>
 #include <fstream>
 #include <memory>
+#include <utility>
 
 #include <grpc/grpc.h>
 #include <grpc/support/alloc.h>
@@ -73,7 +74,7 @@ void UnaryCompressionChecks(const InteropClientContextInspector& inspector,
 }
 }  // namespace
 
-InteropClient::ServiceStub::ServiceStub(std::shared_ptr<Channel> channel,
+InteropClient::ServiceStub::ServiceStub(const std::shared_ptr<Channel>& channel,
                                         bool new_stub_every_call)
     : channel_(channel), new_stub_every_call_(new_stub_every_call) {
   // If new_stub_every_call is false, then this is our chance to initialize
@@ -99,7 +100,8 @@ InteropClient::ServiceStub::GetUnimplementedServiceStub() {
   return unimplemented_service_stub_.get();
 }
 
-void InteropClient::ServiceStub::Reset(std::shared_ptr<Channel> channel) {
+void InteropClient::ServiceStub::Reset(
+    const std::shared_ptr<Channel>& channel) {
   channel_ = channel;
 
   // Update stub_ as well. Note: If new_stub_every_call_ is true, we can reset
@@ -111,14 +113,14 @@ void InteropClient::ServiceStub::Reset(std::shared_ptr<Channel> channel) {
   }
 }
 
-void InteropClient::Reset(std::shared_ptr<Channel> channel) {
-  serviceStub_.Reset(channel);
+void InteropClient::Reset(const std::shared_ptr<Channel>& channel) {
+  serviceStub_.Reset(std::move(channel));
 }
 
-InteropClient::InteropClient(std::shared_ptr<Channel> channel,
+InteropClient::InteropClient(const std::shared_ptr<Channel>& channel,
                              bool new_stub_every_test_case,
                              bool do_not_abort_on_transient_failures)
-    : serviceStub_(channel, new_stub_every_test_case),
+    : serviceStub_(std::move(channel), new_stub_every_test_case),
       do_not_abort_on_transient_failures_(do_not_abort_on_transient_failures) {}
 
 bool InteropClient::AssertStatusOk(const Status& s,
@@ -180,7 +182,7 @@ bool InteropClient::PerformLargeUnary(SimpleRequest* request,
 
 bool InteropClient::PerformLargeUnary(SimpleRequest* request,
                                       SimpleResponse* response,
-                                      CheckerFn custom_checks_fn) {
+                                      const CheckerFn& custom_checks_fn) {
   ClientContext context;
   InteropClientContextInspector inspector(context);
   request->set_response_size(kLargeResponseSize);

test/cpp/interop/interop_client.h

@@ -40,12 +40,12 @@ class InteropClient {
   /// created for every test case
   /// If do_not_abort_on_transient_failures is true, abort() is not called in
   /// case of transient failures (like connection failures)
-  explicit InteropClient(std::shared_ptr<Channel> channel,
+  explicit InteropClient(const std::shared_ptr<Channel>& channel,
                          bool new_stub_every_test_case,
                          bool do_not_abort_on_transient_failures);
   ~InteropClient() {}
 
-  void Reset(std::shared_ptr<Channel> channel);
+  void Reset(const std::shared_ptr<Channel>& channel);
 
   bool DoEmpty();
   bool DoLargeUnary();
@@ -83,12 +83,13 @@ class InteropClient {
    public:
     // If new_stub_every_call = true, pointer to a new instance of
     // TestServce::Stub is returned by Get() everytime it is called
-    ServiceStub(std::shared_ptr<Channel> channel, bool new_stub_every_call);
+    ServiceStub(const std::shared_ptr<Channel>& channel,
+                bool new_stub_every_call);
 
     TestService::Stub* Get();
     UnimplementedService::Stub* GetUnimplementedServiceStub();
 
-    void Reset(std::shared_ptr<Channel> channel);
+    void Reset(const std::shared_ptr<Channel>& channel);
 
    private:
     std::unique_ptr<TestService::Stub> stub_;
@@ -102,7 +103,7 @@ class InteropClient {
 
   /// Run \a custom_check_fn as an additional check.
   bool PerformLargeUnary(SimpleRequest* request, SimpleResponse* response,
-                         CheckerFn custom_checks_fn);
+                         const CheckerFn& custom_checks_fn);
   bool AssertStatusOk(const Status& s,
                       const grpc::string& optional_debug_string);
   bool AssertStatusCode(const Status& s, StatusCode expected_code,

test/cpp/interop/interop_server.cc

@@ -317,25 +317,25 @@ class TestServiceImpl : public TestService::Service {
 };
 
 void grpc::testing::interop::RunServer(
-    std::shared_ptr<ServerCredentials> creds) {
+    const std::shared_ptr<ServerCredentials>& creds) {
   RunServer(creds, FLAGS_port, nullptr, nullptr);
 }
 
 void grpc::testing::interop::RunServer(
-    std::shared_ptr<ServerCredentials> creds,
+    const std::shared_ptr<ServerCredentials>& creds,
     std::unique_ptr<std::vector<std::unique_ptr<ServerBuilderOption>>>
         server_options) {
   RunServer(creds, FLAGS_port, nullptr, std::move(server_options));
 }
 
 void grpc::testing::interop::RunServer(
-    std::shared_ptr<ServerCredentials> creds, const int port,
+    const std::shared_ptr<ServerCredentials>& creds, const int port,
     ServerStartedCondition* server_started_condition) {
   RunServer(creds, port, server_started_condition, nullptr);
 }
 
 void grpc::testing::interop::RunServer(
-    std::shared_ptr<ServerCredentials> creds, const int port,
+    const std::shared_ptr<ServerCredentials>& creds, const int port,
     ServerStartedCondition* server_started_condition,
     std::unique_ptr<std::vector<std::unique_ptr<ServerBuilderOption>>>
         server_options) {

test/cpp/interop/server_helper.h

@@ -63,7 +63,7 @@ struct ServerStartedCondition {
 /// Run gRPC interop server using port FLAGS_port.
 ///
 /// \param creds The credentials associated with the server.
-void RunServer(std::shared_ptr<ServerCredentials> creds);
+void RunServer(const std::shared_ptr<ServerCredentials>& creds);
 
 /// Run gRPC interop server.
 ///
@@ -71,7 +71,7 @@ void RunServer(std::shared_ptr<ServerCredentials> creds);
 /// \param port Port to use for the server.
 /// \param server_started_condition (optional) Struct holding mutex, condition
 ///     variable, and condition used to notify when the server has started.
-void RunServer(std::shared_ptr<ServerCredentials> creds, int port,
+void RunServer(const std::shared_ptr<ServerCredentials>& creds, int port,
                ServerStartedCondition* server_started_condition);
 
 /// Run gRPC interop server.
@@ -79,7 +79,7 @@ void RunServer(std::shared_ptr<ServerCredentials> creds, int port,
 /// \param creds The credentials associated with the server.
 /// \param server_options List of options to set when building the server.
 void RunServer(
-    std::shared_ptr<ServerCredentials> creds,
+    const std::shared_ptr<ServerCredentials>& creds,
     std::unique_ptr<std::vector<std::unique_ptr<ServerBuilderOption>>>
         server_options);
 
@@ -91,7 +91,7 @@ void RunServer(
 /// \param server_started_condition (optional) Struct holding mutex, condition
 //     variable, and condition used to notify when the server has started.
 void RunServer(
-    std::shared_ptr<ServerCredentials> creds, const int port,
+    const std::shared_ptr<ServerCredentials>& creds, const int port,
     ServerStartedCondition* server_started_condition,
     std::unique_ptr<std::vector<std::unique_ptr<grpc::ServerBuilderOption>>>
         server_options);

test/cpp/interop/stress_interop_client.cc

@@ -68,7 +68,7 @@ TestCaseType WeightedRandomTestSelector::GetNextTest() const {
 
 StressTestInteropClient::StressTestInteropClient(
     int test_id, const grpc::string& server_address,
-    std::shared_ptr<Channel> channel,
+    const std::shared_ptr<Channel>& channel,
     const WeightedRandomTestSelector& test_selector, long test_duration_secs,
     long sleep_duration_ms, bool do_not_abort_on_transient_failures)
     : test_id_(test_id),
@@ -80,7 +80,8 @@ StressTestInteropClient::StressTestInteropClient(
       test_duration_secs_(test_duration_secs),
       sleep_duration_ms_(sleep_duration_ms) {}
 
-void StressTestInteropClient::MainLoop(std::shared_ptr<QpsGauge> qps_gauge) {
+void StressTestInteropClient::MainLoop(
+    const std::shared_ptr<QpsGauge>& qps_gauge) {
   gpr_log(GPR_INFO, "Running test %d. ServerAddr: %s", test_id_,
           server_address_.c_str());
 

test/cpp/interop/stress_interop_client.h

@@ -91,14 +91,14 @@ class WeightedRandomTestSelector {
 class StressTestInteropClient {
  public:
   StressTestInteropClient(int test_id, const grpc::string& server_address,
-                          std::shared_ptr<Channel> channel,
+                          const std::shared_ptr<Channel>& channel,
                           const WeightedRandomTestSelector& test_selector,
                           long test_duration_secs, long sleep_duration_ms,
                           bool do_not_abort_on_transient_failures);
 
   // The main function. Use this as the thread entry point.
   // qps_gauge is the QpsGauge to record the requests per second metric
-  void MainLoop(std::shared_ptr<QpsGauge> qps_gauge);
+  void MainLoop(const std::shared_ptr<QpsGauge>& qps_gauge);
 
  private:
   bool RunTest(TestCaseType test_case);

test/cpp/microbenchmarks/bm_chttp2_hpack.cc

@@ -208,6 +208,7 @@ class SingleInternedBinaryElem {
  private:
   static grpc_slice MakeBytes() {
     std::vector<char> v;
+    v.reserve(kLength);
     for (int i = 0; i < kLength; i++) {
       v.push_back(static_cast<char>(rand()));
     }
@@ -246,6 +247,7 @@ class SingleNonInternedBinaryElem {
  private:
   static grpc_slice MakeBytes() {
     std::vector<char> v;
+    v.reserve(kLength);
     for (int i = 0; i < kLength; i++) {
       v.push_back(static_cast<char>(rand()));
     }

test/cpp/naming/resolver_component_test.cc

@@ -90,7 +90,7 @@ namespace {
 class GrpcLBAddress final {
  public:
   GrpcLBAddress(std::string address, bool is_balancer)
-      : is_balancer(is_balancer), address(address) {}
+      : is_balancer(is_balancer), address(std::move(address)) {}
 
   bool operator==(const GrpcLBAddress& other) const {
     return this->is_balancer == other.is_balancer &&
@@ -109,7 +109,7 @@ vector<GrpcLBAddress> ParseExpectedAddrs(std::string expected_addrs) {
   std::vector<GrpcLBAddress> out;
   while (expected_addrs.size() != 0) {
     // get the next <ip>,<port> (v4 or v6)
-    size_t next_comma = expected_addrs.find(",");
+    size_t next_comma = expected_addrs.find(',');
     if (next_comma == std::string::npos) {
       gpr_log(GPR_ERROR,
               "Missing ','. Expected_addrs arg should be a semicolon-separated "
@@ -120,7 +120,7 @@ vector<GrpcLBAddress> ParseExpectedAddrs(std::string expected_addrs) {
     std::string next_addr = expected_addrs.substr(0, next_comma);
     expected_addrs = expected_addrs.substr(next_comma + 1, std::string::npos);
     // get the next is_balancer 'bool' associated with this address
-    size_t next_semicolon = expected_addrs.find(";");
+    size_t next_semicolon = expected_addrs.find(';');
     bool is_balancer =
         gpr_is_true(expected_addrs.substr(0, next_semicolon).c_str());
     out.emplace_back(GrpcLBAddress(next_addr, is_balancer));

test/cpp/naming/resolver_component_tests_runner_invoker.cc

@@ -99,21 +99,21 @@ namespace grpc {
 
 namespace testing {
 
-void InvokeResolverComponentTestsRunner(std::string test_runner_bin_path,
-                                        std::string test_bin_path,
-                                        std::string dns_server_bin_path,
-                                        std::string records_config_path,
-                                        std::string dns_resolver_bin_path,
-                                        std::string tcp_connect_bin_path) {
+void InvokeResolverComponentTestsRunner(
+    std::string test_runner_bin_path, const std::string& test_bin_path,
+    const std::string& dns_server_bin_path,
+    const std::string& records_config_path,
+    const std::string& dns_resolver_bin_path,
+    const std::string& tcp_connect_bin_path) {
   int dns_server_port = grpc_pick_unused_port_or_die();
 
-  SubProcess* test_driver =
-      new SubProcess({test_runner_bin_path, "--test_bin_path=" + test_bin_path,
-                      "--dns_server_bin_path=" + dns_server_bin_path,
-                      "--records_config_path=" + records_config_path,
-                      "--dns_server_port=" + std::to_string(dns_server_port),
-                      "--dns_resolver_bin_path=" + dns_resolver_bin_path,
-                      "--tcp_connect_bin_path=" + tcp_connect_bin_path});
+  SubProcess* test_driver = new SubProcess(
+      {std::move(test_runner_bin_path), "--test_bin_path=" + test_bin_path,
+       "--dns_server_bin_path=" + dns_server_bin_path,
+       "--records_config_path=" + records_config_path,
+       "--dns_server_port=" + std::to_string(dns_server_port),
+       "--dns_resolver_bin_path=" + dns_resolver_bin_path,
+       "--tcp_connect_bin_path=" + tcp_connect_bin_path});
   gpr_mu test_driver_mu;
   gpr_mu_init(&test_driver_mu);
   gpr_cv test_driver_cv;

test/cpp/qps/client.h

@@ -450,7 +450,7 @@ class ClientImpl : public Client {
 
    private:
     void set_channel_args(const ClientConfig& config, ChannelArguments* args) {
-      for (auto channel_arg : config.channel_args()) {
+      for (const auto& channel_arg : config.channel_args()) {
         if (channel_arg.value_case() == ChannelArg::kStrValue) {
           args->SetString(channel_arg.name(), channel_arg.str_value());
         } else if (channel_arg.value_case() == ChannelArg::kIntValue) {

test/cpp/qps/client_async.cc

@@ -24,6 +24,7 @@
 #include <sstream>
 #include <string>
 #include <thread>
+#include <utility>
 #include <vector>
 
 #include <grpc/grpc.h>
@@ -78,7 +79,7 @@ class ClientRpcContextUnaryImpl : public ClientRpcContext {
         response_(),
         next_state_(State::READY),
         callback_(on_done),
-        next_issue_(next_issue),
+        next_issue_(std::move(next_issue)),
         prepare_req_(prepare_req) {}
   ~ClientRpcContextUnaryImpl() override {}
   void Start(CompletionQueue* cq, const ClientConfig& config) override {
@@ -298,7 +299,7 @@ class AsyncClient : public ClientImpl<StubType, RequestType> {
 };
 
 static std::unique_ptr<BenchmarkService::Stub> BenchmarkStubCreator(
-    std::shared_ptr<Channel> ch) {
+    const std::shared_ptr<Channel>& ch) {
   return BenchmarkService::NewStub(ch);
 }
 
@@ -313,7 +314,7 @@ class AsyncUnaryClient final
   ~AsyncUnaryClient() override {}
 
  private:
-  static void CheckDone(grpc::Status s, SimpleResponse* response,
+  static void CheckDone(const grpc::Status& s, SimpleResponse* response,
                         HistogramEntry* entry) {
     entry->set_status(s.error_code());
   }
@@ -326,7 +327,7 @@ class AsyncUnaryClient final
                                     std::function<gpr_timespec()> next_issue,
                                     const SimpleRequest& req) {
     return new ClientRpcContextUnaryImpl<SimpleRequest, SimpleResponse>(
-        stub, req, next_issue, AsyncUnaryClient::PrepareReq,
+        stub, req, std::move(next_issue), AsyncUnaryClient::PrepareReq,
         AsyncUnaryClient::CheckDone);
   }
 };
@@ -349,7 +350,7 @@ class ClientRpcContextStreamingPingPongImpl : public ClientRpcContext {
         response_(),
         next_state_(State::INVALID),
         callback_(on_done),
-        next_issue_(next_issue),
+        next_issue_(std::move(next_issue)),
         prepare_req_(prepare_req),
         coalesce_(false) {}
   ~ClientRpcContextStreamingPingPongImpl() override {}
@@ -497,7 +498,7 @@ class AsyncStreamingPingPongClient final
   ~AsyncStreamingPingPongClient() override {}
 
  private:
-  static void CheckDone(grpc::Status s, SimpleResponse* response) {}
+  static void CheckDone(const grpc::Status& s, SimpleResponse* response) {}
   static std::unique_ptr<
       grpc::ClientAsyncReaderWriter<SimpleRequest, SimpleResponse>>
   PrepareReq(BenchmarkService::Stub* stub, grpc::ClientContext* ctx,
@@ -510,7 +511,8 @@ class AsyncStreamingPingPongClient final
                                     const SimpleRequest& req) {
     return new ClientRpcContextStreamingPingPongImpl<SimpleRequest,
                                                      SimpleResponse>(
-        stub, req, next_issue, AsyncStreamingPingPongClient::PrepareReq,
+        stub, req, std::move(next_issue),
+        AsyncStreamingPingPongClient::PrepareReq,
         AsyncStreamingPingPongClient::CheckDone);
   }
 };
@@ -533,7 +535,7 @@ class ClientRpcContextStreamingFromClientImpl : public ClientRpcContext {
         response_(),
         next_state_(State::INVALID),
         callback_(on_done),
-        next_issue_(next_issue),
+        next_issue_(std::move(next_issue)),
         prepare_req_(prepare_req) {}
   ~ClientRpcContextStreamingFromClientImpl() override {}
   void Start(CompletionQueue* cq, const ClientConfig& config) override {
@@ -628,7 +630,7 @@ class AsyncStreamingFromClientClient final
   ~AsyncStreamingFromClientClient() override {}
 
  private:
-  static void CheckDone(grpc::Status s, SimpleResponse* response) {}
+  static void CheckDone(const grpc::Status& s, SimpleResponse* response) {}
   static std::unique_ptr<grpc::ClientAsyncWriter<SimpleRequest>> PrepareReq(
       BenchmarkService::Stub* stub, grpc::ClientContext* ctx,
       SimpleResponse* resp, CompletionQueue* cq) {
@@ -640,7 +642,8 @@ class AsyncStreamingFromClientClient final
                                     const SimpleRequest& req) {
     return new ClientRpcContextStreamingFromClientImpl<SimpleRequest,
                                                        SimpleResponse>(
-        stub, req, next_issue, AsyncStreamingFromClientClient::PrepareReq,
+        stub, req, std::move(next_issue),
+        AsyncStreamingFromClientClient::PrepareReq,
         AsyncStreamingFromClientClient::CheckDone);
   }
 };
@@ -663,7 +666,7 @@ class ClientRpcContextStreamingFromServerImpl : public ClientRpcContext {
         response_(),
         next_state_(State::INVALID),
         callback_(on_done),
-        next_issue_(next_issue),
+        next_issue_(std::move(next_issue)),
         prepare_req_(prepare_req) {}
   ~ClientRpcContextStreamingFromServerImpl() override {}
   void Start(CompletionQueue* cq, const ClientConfig& config) override {
@@ -742,7 +745,7 @@ class AsyncStreamingFromServerClient final
   ~AsyncStreamingFromServerClient() override {}
 
  private:
-  static void CheckDone(grpc::Status s, SimpleResponse* response) {}
+  static void CheckDone(const grpc::Status& s, SimpleResponse* response) {}
   static std::unique_ptr<grpc::ClientAsyncReader<SimpleResponse>> PrepareReq(
       BenchmarkService::Stub* stub, grpc::ClientContext* ctx,
       const SimpleRequest& req, CompletionQueue* cq) {
@@ -754,7 +757,8 @@ class AsyncStreamingFromServerClient final
                                     const SimpleRequest& req) {
     return new ClientRpcContextStreamingFromServerImpl<SimpleRequest,
                                                        SimpleResponse>(
-        stub, req, next_issue, AsyncStreamingFromServerClient::PrepareReq,
+        stub, req, std::move(next_issue),
+        AsyncStreamingFromServerClient::PrepareReq,
         AsyncStreamingFromServerClient::CheckDone);
   }
 };
@@ -775,9 +779,9 @@ class ClientRpcContextGenericStreamingImpl : public ClientRpcContext {
         req_(req),
         response_(),
         next_state_(State::INVALID),
-        callback_(on_done),
-        next_issue_(next_issue),
-        prepare_req_(prepare_req) {}
+        callback_(std::move(on_done)),
+        next_issue_(std::move(next_issue)),
+        prepare_req_(std::move(prepare_req)) {}
   ~ClientRpcContextGenericStreamingImpl() override {}
   void Start(CompletionQueue* cq, const ClientConfig& config) override {
     GPR_ASSERT(!config.use_coalesce_api());  // not supported yet.
@@ -891,7 +895,7 @@ class ClientRpcContextGenericStreamingImpl : public ClientRpcContext {
 };
 
 static std::unique_ptr<grpc::GenericStub> GenericStubCreator(
-    std::shared_ptr<Channel> ch) {
+    const std::shared_ptr<Channel>& ch) {
   return std::unique_ptr<grpc::GenericStub>(new grpc::GenericStub(ch));
 }
 
@@ -907,7 +911,7 @@ class GenericAsyncStreamingClient final
   ~GenericAsyncStreamingClient() override {}
 
  private:
-  static void CheckDone(grpc::Status s, ByteBuffer* response) {}
+  static void CheckDone(const grpc::Status& s, ByteBuffer* response) {}
   static std::unique_ptr<grpc::GenericClientAsyncReaderWriter> PrepareReq(
       grpc::GenericStub* stub, grpc::ClientContext* ctx,
       const grpc::string& method_name, CompletionQueue* cq) {
@@ -918,7 +922,8 @@ class GenericAsyncStreamingClient final
                                     std::function<gpr_timespec()> next_issue,
                                     const ByteBuffer& req) {
     return new ClientRpcContextGenericStreamingImpl(
-        stub, req, next_issue, GenericAsyncStreamingClient::PrepareReq,
+        stub, req, std::move(next_issue),
+        GenericAsyncStreamingClient::PrepareReq,
         GenericAsyncStreamingClient::CheckDone);
   }
 };

test/cpp/qps/client_sync.cc

@@ -44,7 +44,7 @@ namespace grpc {
 namespace testing {
 
 static std::unique_ptr<BenchmarkService::Stub> BenchmarkStubCreator(
-    std::shared_ptr<Channel> ch) {
+    const std::shared_ptr<Channel>& ch) {
   return BenchmarkService::NewStub(ch);
 }
 
@@ -192,7 +192,7 @@ class SynchronousStreamingClient : public SynchronousClient {
     new (&context_[thread_idx]) ClientContext();
   }
 
-  void CleanupAllStreams(std::function<void(size_t)> cleaner) {
+  void CleanupAllStreams(const std::function<void(size_t)>& cleaner) {
     std::vector<std::thread> cleanup_threads;
     for (size_t i = 0; i < num_threads_; i++) {
       cleanup_threads.emplace_back([this, i, cleaner] {

test/cpp/qps/driver.cc

@@ -96,16 +96,20 @@ static deque<string> get_workers(const string& env_name) {
 }
 
 // helpers for postprocess_scenario_result
-static double WallTime(ClientStats s) { return s.time_elapsed(); }
-static double SystemTime(ClientStats s) { return s.time_system(); }
-static double UserTime(ClientStats s) { return s.time_user(); }
-static double CliPollCount(ClientStats s) { return s.cq_poll_count(); }
-static double SvrPollCount(ServerStats s) { return s.cq_poll_count(); }
-static double ServerWallTime(ServerStats s) { return s.time_elapsed(); }
-static double ServerSystemTime(ServerStats s) { return s.time_system(); }
-static double ServerUserTime(ServerStats s) { return s.time_user(); }
-static double ServerTotalCpuTime(ServerStats s) { return s.total_cpu_time(); }
-static double ServerIdleCpuTime(ServerStats s) { return s.idle_cpu_time(); }
+static double WallTime(const ClientStats& s) { return s.time_elapsed(); }
+static double SystemTime(const ClientStats& s) { return s.time_system(); }
+static double UserTime(const ClientStats& s) { return s.time_user(); }
+static double CliPollCount(const ClientStats& s) { return s.cq_poll_count(); }
+static double SvrPollCount(const ServerStats& s) { return s.cq_poll_count(); }
+static double ServerWallTime(const ServerStats& s) { return s.time_elapsed(); }
+static double ServerSystemTime(const ServerStats& s) { return s.time_system(); }
+static double ServerUserTime(const ServerStats& s) { return s.time_user(); }
+static double ServerTotalCpuTime(const ServerStats& s) {
+  return s.total_cpu_time();
+}
+static double ServerIdleCpuTime(const ServerStats& s) {
+  return s.idle_cpu_time();
+}
 static int Cores(int n) { return n; }
 
 // Postprocess ScenarioResult and populate result summary.
@@ -156,7 +160,7 @@ static void postprocess_scenario_result(ScenarioResult* result) {
     int64_t successes = 0;
     int64_t failures = 0;
     for (int i = 0; i < result->request_results_size(); i++) {
-      RequestResultCount rrc = result->request_results(i);
+      const RequestResultCount& rrc = result->request_results(i);
       if (rrc.status_code() == 0) {
         successes += rrc.count();
       } else {
@@ -213,7 +217,6 @@ std::unique_ptr<ScenarioResult> RunScenario(
   // To be added to the result, containing the final configuration used for
   // client and config (including host, etc.)
   ClientConfig result_client_config;
-  const ServerConfig result_server_config = initial_server_config;
 
   // Get client, server lists; ignore if inproc test
   auto workers = (!run_inproc) ? get_workers("QPS_WORKERS") : deque<string>();
@@ -280,7 +283,7 @@ std::unique_ptr<ScenarioResult> RunScenario(
           local_workers[i]->InProcessChannel(channel_args));
     }
 
-    ServerConfig server_config = initial_server_config;
+    const ServerConfig& server_config = initial_server_config;
     if (server_config.core_limit() != 0) {
       gpr_log(GPR_ERROR,
               "server config core limit is set but ignored by driver");

test/cpp/qps/qps_interarrival_test.cc

@@ -28,7 +28,8 @@
 using grpc::testing::InterarrivalTimer;
 using grpc::testing::RandomDistInterface;
 
-static void RunTest(RandomDistInterface&& r, int threads, std::string title) {
+static void RunTest(RandomDistInterface&& r, int threads,
+                    const std::string& title) {
   InterarrivalTimer timer;
   timer.init(r, threads);
   grpc_histogram* h(grpc_histogram_create(0.01, 60e9));

test/cpp/qps/report.h

@@ -129,7 +129,7 @@ class JsonReporter : public Reporter {
 
 class RpcReporter : public Reporter {
  public:
-  RpcReporter(const string& name, std::shared_ptr<grpc::Channel> channel)
+  RpcReporter(const string& name, const std::shared_ptr<grpc::Channel>& channel)
       : Reporter(name), stub_(ReportQpsScenarioService::NewStub(channel)) {}
 
  private:

test/cpp/qps/server_sync.cc

@@ -129,7 +129,7 @@ class BenchmarkServiceImpl final : public BenchmarkService::Service {
   template <class W>
   static Status ServerPush(ServerContext* context, W* stream,
                            const SimpleResponse& response,
-                           std::function<bool()> done) {
+                           const std::function<bool()>& done) {
     while ((done == nullptr) || !done()) {
       // TODO(vjpai): Add potential for rate-pacing on this
       if (!stream->Write(response)) {

test/cpp/server/load_reporter/load_data_store_test.cc

@@ -50,8 +50,8 @@ class LoadDataStoreTest : public ::testing::Test {
   bool PerBalancerStoresContains(
       const LoadDataStore& load_data_store,
       const std::set<PerBalancerStore*>* per_balancer_stores,
-      const grpc::string hostname, const grpc::string lb_id,
-      const grpc::string load_key) {
+      const grpc::string& hostname, const grpc::string& lb_id,
+      const grpc::string& load_key) {
     auto original_per_balancer_store =
         load_data_store.FindPerBalancerStore(hostname, lb_id);
     EXPECT_NE(original_per_balancer_store, nullptr);
@@ -155,7 +155,7 @@ TEST_F(LoadDataStoreTest, OrphanAssignmentIsSticky) {
   active_lb_ids.erase(orphaned_lb_id);
   // Find which LB is assigned the orphaned store.
   grpc::string assigned_lb_id = "";
-  for (auto lb_id : active_lb_ids) {
+  for (const auto& lb_id : active_lb_ids) {
     if (PerBalancerStoresContains(
             load_data_store,
             load_data_store.GetAssignedStores(kHostname1, lb_id), kHostname1,
@@ -169,7 +169,7 @@ TEST_F(LoadDataStoreTest, OrphanAssignmentIsSticky) {
   // orphaned_lb_id shouldn't change.
   for (size_t _ = 0; _ < 10; ++_) {
     grpc::string lb_id_to_close = "";
-    for (auto lb_id : active_lb_ids) {
+    for (const auto& lb_id : active_lb_ids) {
       if (lb_id != assigned_lb_id) {
         lb_id_to_close = lb_id;
         break;
@@ -187,7 +187,7 @@ TEST_F(LoadDataStoreTest, OrphanAssignmentIsSticky) {
   load_data_store.ReportStreamClosed(kHostname1, assigned_lb_id);
   active_lb_ids.erase(assigned_lb_id);
   size_t orphaned_lb_id_occurences = 0;
-  for (auto lb_id : active_lb_ids) {
+  for (const auto& lb_id : active_lb_ids) {
     if (PerBalancerStoresContains(
             load_data_store,
             load_data_store.GetAssignedStores(kHostname1, lb_id), kHostname1,

test/cpp/util/.clang-tidy

@@ -0,0 +1,6 @@
+---
+Checks: 'modernize-use-nullptr,google-build-namespaces,google-build-explicit-make-pair,readability-function-size,performance-*,-performance-unnecessary-copy-initialization'
+WarningsAsErrors: 'modernize-use-nullptr,google-build-namespaces,google-build-explicit-make-pair,readability-function-size,performance-*,-performance-unnecessary-copy-initialization'
+CheckOptions:
+  - key:    readability-function-size.StatementThreshold
+    value:  '450'

test/cpp/util/byte_buffer_test.cc

@@ -41,7 +41,7 @@ class ByteBufferTest : public ::testing::Test {};
 TEST_F(ByteBufferTest, CopyCtor) {
   ByteBuffer buffer1;
   EXPECT_FALSE(buffer1.Valid());
-  ByteBuffer buffer2 = buffer1;
+  const ByteBuffer& buffer2 = buffer1;
   EXPECT_FALSE(buffer2.Valid());
 }
 

test/cpp/util/cli_call.cc

@@ -19,6 +19,7 @@
 #include "test/cpp/util/cli_call.h"
 
 #include <iostream>
+#include <utility>
 
 #include <grpc/grpc.h>
 #include <grpc/slice.h>
@@ -39,7 +40,7 @@ Status CliCall::Call(std::shared_ptr<grpc::Channel> channel,
                      const OutgoingMetadataContainer& metadata,
                      IncomingMetadataContainer* server_initial_metadata,
                      IncomingMetadataContainer* server_trailing_metadata) {
-  CliCall call(channel, method, metadata);
+  CliCall call(std::move(channel), method, metadata);
   call.Write(request);
   call.WritesDone();
   if (!call.Read(response, server_initial_metadata)) {
@@ -48,7 +49,7 @@ Status CliCall::Call(std::shared_ptr<grpc::Channel> channel,
   return call.Finish(server_trailing_metadata);
 }
 
-CliCall::CliCall(std::shared_ptr<grpc::Channel> channel,
+CliCall::CliCall(const std::shared_ptr<grpc::Channel>& channel,
                  const grpc::string& method,
                  const OutgoingMetadataContainer& metadata)
     : stub_(new grpc::GenericStub(channel)) {

test/cpp/util/cli_call.h

@@ -42,7 +42,8 @@ class CliCall final {
   typedef std::multimap<grpc::string_ref, grpc::string_ref>
       IncomingMetadataContainer;
 
-  CliCall(std::shared_ptr<grpc::Channel> channel, const grpc::string& method,
+  CliCall(const std::shared_ptr<grpc::Channel>& channel,
+          const grpc::string& method,
           const OutgoingMetadataContainer& metadata);
   ~CliCall();
 

test/cpp/util/proto_file_parser.cc

@@ -63,7 +63,7 @@ class ErrorPrinter : public protobuf::compiler::MultiFileErrorCollector {
   ProtoFileParser* parser_;  // not owned
 };
 
-ProtoFileParser::ProtoFileParser(std::shared_ptr<grpc::Channel> channel,
+ProtoFileParser::ProtoFileParser(const std::shared_ptr<grpc::Channel>& channel,
                                  const grpc::string& proto_path,
                                  const grpc::string& protofiles)
     : has_error_(false),

test/cpp/util/proto_file_parser.h

@@ -36,7 +36,7 @@ class ProtoFileParser {
   // The parser will search proto files using the server reflection service
   // provided on the given channel. The given protofiles in a source tree rooted
   // from proto_path will also be searched.
-  ProtoFileParser(std::shared_ptr<grpc::Channel> channel,
+  ProtoFileParser(const std::shared_ptr<grpc::Channel>& channel,
                   const grpc::string& proto_path,
                   const grpc::string& protofiles);
 

test/cpp/util/proto_reflection_descriptor_database.cc

@@ -35,7 +35,7 @@ ProtoReflectionDescriptorDatabase::ProtoReflectionDescriptorDatabase(
     : stub_(std::move(stub)) {}
 
 ProtoReflectionDescriptorDatabase::ProtoReflectionDescriptorDatabase(
-    std::shared_ptr<grpc::Channel> channel)
+    const std::shared_ptr<grpc::Channel>& channel)
     : stub_(ServerReflection::NewStub(channel)) {}
 
 ProtoReflectionDescriptorDatabase::~ProtoReflectionDescriptorDatabase() {
@@ -79,7 +79,7 @@ bool ProtoReflectionDescriptorDatabase::FindFileByName(
     AddFileFromResponse(response.file_descriptor_response());
   } else if (response.message_response_case() ==
              ServerReflectionResponse::MessageResponseCase::kErrorResponse) {
-    const ErrorResponse error = response.error_response();
+    const ErrorResponse& error = response.error_response();
     if (error.error_code() == StatusCode::NOT_FOUND) {
       gpr_log(GPR_INFO, "NOT_FOUND from server for FindFileByName(%s)",
               filename.c_str());
@@ -126,7 +126,7 @@ bool ProtoReflectionDescriptorDatabase::FindFileContainingSymbol(
     AddFileFromResponse(response.file_descriptor_response());
   } else if (response.message_response_case() ==
              ServerReflectionResponse::MessageResponseCase::kErrorResponse) {
-    const ErrorResponse error = response.error_response();
+    const ErrorResponse& error = response.error_response();
     if (error.error_code() == StatusCode::NOT_FOUND) {
       missing_symbols_.insert(symbol_name);
       gpr_log(GPR_INFO,
@@ -182,7 +182,7 @@ bool ProtoReflectionDescriptorDatabase::FindFileContainingExtension(
     AddFileFromResponse(response.file_descriptor_response());
   } else if (response.message_response_case() ==
              ServerReflectionResponse::MessageResponseCase::kErrorResponse) {
-    const ErrorResponse error = response.error_response();
+    const ErrorResponse& error = response.error_response();
     if (error.error_code() == StatusCode::NOT_FOUND) {
       if (missing_extensions_.find(containing_type) ==
           missing_extensions_.end()) {
@@ -238,7 +238,7 @@ bool ProtoReflectionDescriptorDatabase::FindAllExtensionNumbers(
     return true;
   } else if (response.message_response_case() ==
              ServerReflectionResponse::MessageResponseCase::kErrorResponse) {
-    const ErrorResponse error = response.error_response();
+    const ErrorResponse& error = response.error_response();
     if (error.error_code() == StatusCode::NOT_FOUND) {
       gpr_log(GPR_INFO, "NOT_FOUND from server for FindAllExtensionNumbers(%s)",
               extendee_type.c_str());
@@ -265,14 +265,14 @@ bool ProtoReflectionDescriptorDatabase::GetServices(
 
   if (response.message_response_case() ==
       ServerReflectionResponse::MessageResponseCase::kListServicesResponse) {
-    const ListServiceResponse ls_response = response.list_services_response();
+    const ListServiceResponse& ls_response = response.list_services_response();
     for (int i = 0; i < ls_response.service_size(); ++i) {
       (*output).push_back(ls_response.service(i).name());
     }
     return true;
   } else if (response.message_response_case() ==
              ServerReflectionResponse::MessageResponseCase::kErrorResponse) {
-    const ErrorResponse error = response.error_response();
+    const ErrorResponse& error = response.error_response();
     gpr_log(GPR_INFO,
             "Error on GetServices()\n\tError code: %d\n"
             "\tError Message: %s",

test/cpp/util/proto_reflection_descriptor_database.h

@@ -38,7 +38,7 @@ class ProtoReflectionDescriptorDatabase : public protobuf::DescriptorDatabase {
       std::unique_ptr<reflection::v1alpha::ServerReflection::Stub> stub);
 
   explicit ProtoReflectionDescriptorDatabase(
-      std::shared_ptr<grpc::Channel> channel);
+      const std::shared_ptr<grpc::Channel>& channel);
 
   virtual ~ProtoReflectionDescriptorDatabase();
 

test/cpp/util/string_ref_test.cc

@@ -60,7 +60,7 @@ TEST_F(StringRefTest, FromString) {
 TEST_F(StringRefTest, CopyConstructor) {
   string_ref s1(kTestString);
   ;
-  string_ref s2(s1);
+  const string_ref& s2(s1);
   EXPECT_EQ(s1.length(), s2.length());
   EXPECT_EQ(s1.data(), s2.data());
 }