Merge pull request #8638 from vjpai/conform

Improve C++11 conformance

test/cpp/end2end/async_end2end_test.cc

@@ -352,15 +352,13 @@ void ServerWait(Server* server, int* notify) {
 }
 TEST_P(AsyncEnd2endTest, WaitAndShutdownTest) {
   int notify = 0;
-  std::thread* wait_thread =
-      new std::thread(&ServerWait, server_.get(), &notify);
+  std::thread wait_thread(&ServerWait, server_.get(), &notify);
   ResetStub();
   SendRpc(1);
   EXPECT_EQ(0, notify);
   server_->Shutdown();
-  wait_thread->join();
+  wait_thread.join();
   EXPECT_EQ(1, notify);
-  delete wait_thread;
 }
 
 TEST_P(AsyncEnd2endTest, ShutdownThenWait) {
@@ -991,7 +989,7 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest {
       expected_server_cq_result = false;
     }
 
-    std::thread* server_try_cancel_thd = NULL;
+    std::thread* server_try_cancel_thd = nullptr;
 
     auto verif = Verifier(GetParam().disable_blocking);
 
@@ -1027,7 +1025,7 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest {
       }
     }
 
-    if (server_try_cancel_thd != NULL) {
+    if (server_try_cancel_thd != nullptr) {
       server_try_cancel_thd->join();
       delete server_try_cancel_thd;
     }
@@ -1112,7 +1110,7 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest {
       expected_cq_result = false;
     }
 
-    std::thread* server_try_cancel_thd = NULL;
+    std::thread* server_try_cancel_thd = nullptr;
 
     auto verif = Verifier(GetParam().disable_blocking);
 
@@ -1150,7 +1148,7 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest {
       }
     }
 
-    if (server_try_cancel_thd != NULL) {
+    if (server_try_cancel_thd != nullptr) {
       server_try_cancel_thd->join();
       delete server_try_cancel_thd;
     }
@@ -1252,7 +1250,7 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest {
       expected_cq_result = false;
     }
 
-    std::thread* server_try_cancel_thd = NULL;
+    std::thread* server_try_cancel_thd = nullptr;
 
     auto verif = Verifier(GetParam().disable_blocking);
 
@@ -1332,7 +1330,7 @@ class AsyncEnd2endServerTryCancelTest : public AsyncEnd2endTest {
       EXPECT_EQ(verif.Next(cq_.get(), ignore_cq_result), 8);
     }
 
-    if (server_try_cancel_thd != NULL) {
+    if (server_try_cancel_thd != nullptr) {
       server_try_cancel_thd->join();
       delete server_try_cancel_thd;
     }

test/cpp/end2end/end2end_test.cc

@@ -656,25 +656,23 @@ TEST_P(End2endTest, SimpleRpcWithCustomeUserAgentPrefix) {
 
 TEST_P(End2endTest, MultipleRpcsWithVariedBinaryMetadataValue) {
   ResetStub();
-  std::vector<std::thread*> threads;
+  std::vector<std::thread> threads;
   for (int i = 0; i < 10; ++i) {
-    threads.push_back(new std::thread(SendRpc, stub_.get(), 10, true));
+    threads.emplace_back(SendRpc, stub_.get(), 10, true);
   }
   for (int i = 0; i < 10; ++i) {
-    threads[i]->join();
-    delete threads[i];
+    threads[i].join();
   }
 }
 
 TEST_P(End2endTest, MultipleRpcs) {
   ResetStub();
-  std::vector<std::thread*> threads;
+  std::vector<std::thread> threads;
   for (int i = 0; i < 10; ++i) {
-    threads.push_back(new std::thread(SendRpc, stub_.get(), 10, false));
+    threads.emplace_back(SendRpc, stub_.get(), 10, false);
   }
   for (int i = 0; i < 10; ++i) {
-    threads[i]->join();
-    delete threads[i];
+    threads[i].join();
   }
 }
 
@@ -1058,13 +1056,12 @@ TEST_P(ProxyEnd2endTest, SimpleRpcWithEmptyMessages) {
 
 TEST_P(ProxyEnd2endTest, MultipleRpcs) {
   ResetStub();
-  std::vector<std::thread*> threads;
+  std::vector<std::thread> threads;
   for (int i = 0; i < 10; ++i) {
-    threads.push_back(new std::thread(SendRpc, stub_.get(), 10, false));
+    threads.emplace_back(SendRpc, stub_.get(), 10, false);
   }
   for (int i = 0; i < 10; ++i) {
-    threads[i]->join();
-    delete threads[i];
+    threads[i].join();
   }
 }
 

test/cpp/end2end/test_service_impl.cc

@@ -194,7 +194,7 @@ Status TestServiceImpl::RequestStream(ServerContext* context,
     return Status::CANCELLED;
   }
 
-  std::thread* server_try_cancel_thd = NULL;
+  std::thread* server_try_cancel_thd = nullptr;
   if (server_try_cancel == CANCEL_DURING_PROCESSING) {
     server_try_cancel_thd =
         new std::thread(&TestServiceImpl::ServerTryCancel, this, context);
@@ -212,7 +212,7 @@ Status TestServiceImpl::RequestStream(ServerContext* context,
   }
   gpr_log(GPR_INFO, "Read: %d messages", num_msgs_read);
 
-  if (server_try_cancel_thd != NULL) {
+  if (server_try_cancel_thd != nullptr) {
     server_try_cancel_thd->join();
     delete server_try_cancel_thd;
     return Status::CANCELLED;
@@ -248,7 +248,7 @@ Status TestServiceImpl::ResponseStream(ServerContext* context,
   }
 
   EchoResponse response;
-  std::thread* server_try_cancel_thd = NULL;
+  std::thread* server_try_cancel_thd = nullptr;
   if (server_try_cancel == CANCEL_DURING_PROCESSING) {
     server_try_cancel_thd =
         new std::thread(&TestServiceImpl::ServerTryCancel, this, context);
@@ -259,7 +259,7 @@ Status TestServiceImpl::ResponseStream(ServerContext* context,
     writer->Write(response);
   }
 
-  if (server_try_cancel_thd != NULL) {
+  if (server_try_cancel_thd != nullptr) {
     server_try_cancel_thd->join();
     delete server_try_cancel_thd;
     return Status::CANCELLED;
@@ -295,7 +295,7 @@ Status TestServiceImpl::BidiStream(
     return Status::CANCELLED;
   }
 
-  std::thread* server_try_cancel_thd = NULL;
+  std::thread* server_try_cancel_thd = nullptr;
   if (server_try_cancel == CANCEL_DURING_PROCESSING) {
     server_try_cancel_thd =
         new std::thread(&TestServiceImpl::ServerTryCancel, this, context);
@@ -307,7 +307,7 @@ Status TestServiceImpl::BidiStream(
     stream->Write(response);
   }
 
-  if (server_try_cancel_thd != NULL) {
+  if (server_try_cancel_thd != nullptr) {
     server_try_cancel_thd->join();
     delete server_try_cancel_thd;
     return Status::CANCELLED;

test/cpp/end2end/thread_stress_test.cc

@@ -232,19 +232,19 @@ class CommonStressTestSyncServer : public CommonStressTest<TestServiceImpl> {
 class CommonStressTestAsyncServer
     : public CommonStressTest<grpc::testing::EchoTestService::AsyncService> {
  public:
+  CommonStressTestAsyncServer() : contexts_(kNumAsyncServerThreads * 100) {}
   void SetUp() override {
     shutting_down_ = false;
     ServerBuilder builder;
     SetUpStart(&builder, &service_);
     cq_ = builder.AddCompletionQueue();
     SetUpEnd(&builder);
-    contexts_ = new Context[kNumAsyncServerThreads * 100];
     for (int i = 0; i < kNumAsyncServerThreads * 100; i++) {
       RefreshContext(i);
     }
     for (int i = 0; i < kNumAsyncServerThreads; i++) {
-      server_threads_.push_back(
-          new std::thread(&CommonStressTestAsyncServer::ProcessRpcs, this));
+      server_threads_.emplace_back(&CommonStressTestAsyncServer::ProcessRpcs,
+                                   this);
     }
   }
   void TearDown() override {
@@ -256,8 +256,7 @@ class CommonStressTestAsyncServer
     }
 
     for (int i = 0; i < kNumAsyncServerThreads; i++) {
-      server_threads_[i]->join();
-      delete server_threads_[i];
+      server_threads_[i].join();
     }
 
     void* ignored_tag;
@@ -265,7 +264,6 @@ class CommonStressTestAsyncServer
     while (cq_->Next(&ignored_tag, &ignored_ok))
       ;
     TearDownEnd();
-    delete[] contexts_;
   }
 
  private:
@@ -311,12 +309,13 @@ class CommonStressTestAsyncServer
         response_writer;
     EchoRequest recv_request;
     enum { READY, DONE } state;
-  } * contexts_;
+  };
+  std::vector<Context> contexts_;
   ::grpc::testing::EchoTestService::AsyncService service_;
   std::unique_ptr<ServerCompletionQueue> cq_;
   bool shutting_down_;
   std::mutex mu_;
-  std::vector<std::thread*> server_threads_;
+  std::vector<std::thread> server_threads_;
 };
 
 template <class Common>
@@ -353,14 +352,12 @@ typedef ::testing::Types<CommonStressTestSyncServer,
 TYPED_TEST_CASE(End2endTest, CommonTypes);
 TYPED_TEST(End2endTest, ThreadStress) {
   this->common_.ResetStub();
-  std::vector<std::thread*> threads;
+  std::vector<std::thread> threads;
   for (int i = 0; i < kNumThreads; ++i) {
-    threads.push_back(
-        new std::thread(SendRpc, this->common_.GetStub(), kNumRpcs));
+    threads.emplace_back(SendRpc, this->common_.GetStub(), kNumRpcs);
   }
   for (int i = 0; i < kNumThreads; ++i) {
-    threads[i]->join();
-    delete threads[i];
+    threads[i].join();
   }
 }
 
@@ -442,26 +439,24 @@ class AsyncClientEnd2endTest : public ::testing::Test {
 TYPED_TEST_CASE(AsyncClientEnd2endTest, CommonTypes);
 TYPED_TEST(AsyncClientEnd2endTest, ThreadStress) {
   this->common_.ResetStub();
-  std::vector<std::thread *> send_threads, completion_threads;
+  std::vector<std::thread> send_threads, completion_threads;
   for (int i = 0; i < kNumAsyncReceiveThreads; ++i) {
-    completion_threads.push_back(new std::thread(
+    completion_threads.emplace_back(
         &AsyncClientEnd2endTest_ThreadStress_Test<TypeParam>::AsyncCompleteRpc,
-        this));
+        this);
   }
   for (int i = 0; i < kNumAsyncSendThreads; ++i) {
-    send_threads.push_back(new std::thread(
+    send_threads.emplace_back(
         &AsyncClientEnd2endTest_ThreadStress_Test<TypeParam>::AsyncSendRpc,
-        this, kNumRpcs));
+        this, kNumRpcs);
   }
   for (int i = 0; i < kNumAsyncSendThreads; ++i) {
-    send_threads[i]->join();
-    delete send_threads[i];
+    send_threads[i].join();
   }
 
   this->Wait();
   for (int i = 0; i < kNumAsyncReceiveThreads; ++i) {
-    completion_threads[i]->join();
-    delete completion_threads[i];
+    completion_threads[i].join();
   }
 }
 

test/cpp/qps/client.h

@@ -163,10 +163,9 @@ class Client {
 
     MaybeStartRequests();
 
-    // avoid std::vector for old compilers that expect a copy constructor
     if (reset) {
-      Histogram* to_merge = new Histogram[threads_.size()];
-      StatusHistogram* to_merge_status = new StatusHistogram[threads_.size()];
+      std::vector<Histogram> to_merge(threads_.size());
+      std::vector<StatusHistogram> to_merge_status(threads_.size());
 
       for (size_t i = 0; i < threads_.size(); i++) {
         threads_[i]->BeginSwap(&to_merge[i], &to_merge_status[i]);
@@ -177,8 +176,6 @@ class Client {
         latencies.Merge(to_merge[i]);
         MergeStatusHistogram(to_merge_status[i], &statuses);
       }
-      delete[] to_merge;
-      delete[] to_merge_status;
       timer_result = timer->Mark();
     } else {
       // merge snapshots of each thread histogram

test/cpp/qps/client_async.cc

@@ -177,7 +177,6 @@ class AsyncClient : public ClientImpl<StubType, RequestType> {
       shutdown_state_.emplace_back(new PerThreadShutdownState());
     }
 
-    using namespace std::placeholders;
     int t = 0;
     for (int ch = 0; ch < config.client_channels(); ch++) {
       for (int i = 0; i < config.outstanding_rpcs_per_channel(); i++) {

test/cpp/qps/client_sync.cc

@@ -138,10 +138,9 @@ class SynchronousUnaryClient final : public SynchronousClient {
 class SynchronousStreamingClient final : public SynchronousClient {
  public:
   SynchronousStreamingClient(const ClientConfig& config)
-      : SynchronousClient(config) {
-    context_ = new grpc::ClientContext[num_threads_];
-    stream_ = new std::unique_ptr<
-        grpc::ClientReaderWriter<SimpleRequest, SimpleResponse>>[num_threads_];
+      : SynchronousClient(config),
+        context_(num_threads_),
+        stream_(num_threads_) {
     for (size_t thread_idx = 0; thread_idx < num_threads_; thread_idx++) {
       auto* stub = channels_[thread_idx % channels_.size()].get_stub();
       stream_[thread_idx] = stub->StreamingCall(&context_[thread_idx]);
@@ -161,8 +160,6 @@ class SynchronousStreamingClient final : public SynchronousClient {
         }
       }
     }
-    delete[] stream_;
-    delete[] context_;
   }
 
   bool ThreadFunc(HistogramEntry* entry, size_t thread_idx) override {
@@ -182,8 +179,9 @@ class SynchronousStreamingClient final : public SynchronousClient {
  private:
   // These are both conceptually std::vector but cannot be for old compilers
   // that expect contained classes to support copy constructors
-  grpc::ClientContext* context_;
-  std::unique_ptr<grpc::ClientReaderWriter<SimpleRequest, SimpleResponse>>*
+  std::vector<grpc::ClientContext> context_;
+  std::vector<
+      std::unique_ptr<grpc::ClientReaderWriter<SimpleRequest, SimpleResponse>>>
       stream_;
 };
 

test/cpp/qps/driver.cc

@@ -192,30 +192,6 @@ static void postprocess_scenario_result(ScenarioResult* result) {
   }
 }
 
-// Namespace for classes and functions used only in RunScenario
-// Using this rather than local definitions to workaround gcc-4.4 limitations
-// regarding using templates without linkage
-namespace runsc {
-
-// ClientContext allocator
-static ClientContext* AllocContext(list<ClientContext>* contexts) {
-  contexts->emplace_back();
-  auto context = &contexts->back();
-  context->set_wait_for_ready(true);
-  return context;
-}
-
-struct ServerData {
-  unique_ptr<WorkerService::Stub> stub;
-  unique_ptr<ClientReaderWriter<ServerArgs, ServerStatus>> stream;
-};
-
-struct ClientData {
-  unique_ptr<WorkerService::Stub> stub;
-  unique_ptr<ClientReaderWriter<ClientArgs, ClientStatus>> stream;
-};
-}  // namespace runsc
-
 std::unique_ptr<ScenarioResult> RunScenario(
     const ClientConfig& initial_client_config, size_t num_clients,
     const ServerConfig& initial_server_config, size_t num_servers,
@@ -225,6 +201,12 @@ std::unique_ptr<ScenarioResult> RunScenario(
 
   // ClientContext allocations (all are destroyed at scope exit)
   list<ClientContext> contexts;
+  auto alloc_context = [](list<ClientContext>* contexts) {
+    contexts->emplace_back();
+    auto context = &contexts->back();
+    context->set_wait_for_ready(true);
+    return context;
+  };
 
   // To be added to the result, containing the final configuration used for
   // client and config (including host, etc.)
@@ -277,10 +259,11 @@ std::unique_ptr<ScenarioResult> RunScenario(
   workers.resize(num_clients + num_servers);
 
   // Start servers
-  using runsc::ServerData;
-  // servers is array rather than std::vector to avoid gcc-4.4 issues
-  // where class contained in std::vector must have a copy constructor
-  auto* servers = new ServerData[num_servers];
+  struct ServerData {
+    unique_ptr<WorkerService::Stub> stub;
+    unique_ptr<ClientReaderWriter<ServerArgs, ServerStatus>> stream;
+  };
+  std::vector<ServerData> servers(num_servers);
   for (size_t i = 0; i < num_servers; i++) {
     gpr_log(GPR_INFO, "Starting server on %s (worker #%" PRIuPTR ")",
             workers[i].c_str(), i);
@@ -324,8 +307,7 @@ std::unique_ptr<ScenarioResult> RunScenario(
 
     ServerArgs args;
     *args.mutable_setup() = server_config;
-    servers[i].stream =
-        servers[i].stub->RunServer(runsc::AllocContext(&contexts));
+    servers[i].stream = servers[i].stub->RunServer(alloc_context(&contexts));
     if (!servers[i].stream->Write(args)) {
       gpr_log(GPR_ERROR, "Could not write args to server %zu", i);
     }
@@ -343,10 +325,11 @@ std::unique_ptr<ScenarioResult> RunScenario(
   // Targets are all set by now
   result_client_config = client_config;
   // Start clients
-  using runsc::ClientData;
-  // clients is array rather than std::vector to avoid gcc-4.4 issues
-  // where class contained in std::vector must have a copy constructor
-  auto* clients = new ClientData[num_clients];
+  struct ClientData {
+    unique_ptr<WorkerService::Stub> stub;
+    unique_ptr<ClientReaderWriter<ClientArgs, ClientStatus>> stream;
+  };
+  std::vector<ClientData> clients(num_clients);
   size_t channels_allocated = 0;
   for (size_t i = 0; i < num_clients; i++) {
     const auto& worker = workers[i + num_servers];
@@ -395,8 +378,7 @@ std::unique_ptr<ScenarioResult> RunScenario(
 
     ClientArgs args;
     *args.mutable_setup() = per_client_config;
-    clients[i].stream =
-        clients[i].stub->RunClient(runsc::AllocContext(&contexts));
+    clients[i].stream = clients[i].stub->RunClient(alloc_context(&contexts));
     if (!clients[i].stream->Write(args)) {
       gpr_log(GPR_ERROR, "Could not write args to client %zu", i);
     }
@@ -516,7 +498,6 @@ std::unique_ptr<ScenarioResult> RunScenario(
               s.error_message().c_str());
     }
   }
-  delete[] clients;
 
   merged_latencies.FillProto(result->mutable_latencies());
   for (std::unordered_map<int, int64_t>::iterator it = merged_statuses.begin();
@@ -559,8 +540,6 @@ std::unique_ptr<ScenarioResult> RunScenario(
     }
   }
 
-  delete[] servers;
-
   postprocess_scenario_result(result.get());
   return result;
 }

test/cpp/util/config_grpc_cli.h

@@ -77,7 +77,7 @@ namespace compiler {
 typedef GRPC_CUSTOM_DISKSOURCETREE DiskSourceTree;
 typedef GRPC_CUSTOM_IMPORTER Importer;
 typedef GRPC_CUSTOM_MULTIFILEERRORCOLLECTOR MultiFileErrorCollector;
-}  // namespace importer
+}  // namespace compiler
 
 }  // namespace protobuf
 }  // namespace grpc