Modernize code

gloo/algorithm.cc

@@ -21,7 +21,7 @@ Algorithm::Algorithm(const std::shared_ptr<Context>& context)
       contextSize_(context_->size) {}
 
 // Have to provide implementation for pure virtual destructor.
-Algorithm::~Algorithm() noexcept(false) {}
+Algorithm::~Algorithm() noexcept(false) = default;
 
 std::unique_ptr<transport::Pair>& Algorithm::getPair(int i) {
   return context_->getPair(i);

gloo/algorithm.h

@@ -100,7 +100,7 @@ const ReductionFunction<T>* ReductionFunction<T>::max =
 template <typename T>
 class LocalOp {
  public:
-  virtual ~LocalOp() noexcept(false) {}
+  virtual ~LocalOp() noexcept(false) = default;
   virtual void runAsync() = 0;
   virtual void wait() = 0;
 

gloo/allgather_ring.h

@@ -52,9 +52,9 @@ class AllgatherRing : public Algorithm {
         rightPair_->createRecvBuffer(notificationSlot, &dummy_, sizeof(dummy_));
   }
 
-  virtual ~AllgatherRing() {}
+  ~AllgatherRing() override = default;
 
-  void run() {
+  void run() override {
     // Short circuit if there is only a single process or the output is empty.
     if (this->contextSize_ == 1 || count_ == 0) {
       return;

gloo/allreduce.cc

@@ -110,11 +110,11 @@ void allreduce(const detail::AllreduceOptionsImpl& opts) {
 
   // Assert the size of all inputs and outputs is identical.
   const size_t totalBytes = opts.elements * opts.elementSize;
-  for (size_t i = 0; i < out.size(); i++) {
-    GLOO_ENFORCE_EQ(out[i]->size, totalBytes);
+  for (const auto& i : out) {
+    GLOO_ENFORCE_EQ(i->size, totalBytes);
   }
-  for (size_t i = 0; i < in.size(); i++) {
-    GLOO_ENFORCE_EQ(in[i]->size, totalBytes);
+  for (const auto& i : in) {
+    GLOO_ENFORCE_EQ(i->size, totalBytes);
   }
 
   // Initialize local reduction and broadcast functions.
@@ -560,8 +560,7 @@ void bcube(
     }
 
     // Wait for send and receive operations to complete.
-    for (size_t i = 0; i < group.ranks.size(); i++) {
-      const auto peer = group.ranks[i];
+    for (unsigned long peer : group.ranks) {
       if (peer == context->rank) {
         continue;
       }
@@ -640,8 +639,7 @@ void bcube(
     }
 
     // Wait for operations to complete.
-    for (size_t i = 0; i < group.ranks.size(); i++) {
-      const auto peer = group.ranks[i];
+    for (unsigned long peer : group.ranks) {
       if (peer == context->rank) {
         continue;
       }

gloo/allreduce.h

@@ -42,17 +42,15 @@ struct AllreduceOptionsImpl {
   };
 
   explicit AllreduceOptionsImpl(const std::shared_ptr<Context>& context)
-      : context(context),
-        timeout(context->getTimeout()),
-        algorithm(UNSPECIFIED) {}
+      : context(context), timeout(context->getTimeout()) {}
 
   std::shared_ptr<Context> context;
 
   // End-to-end timeout for this operation.
   std::chrono::milliseconds timeout;
 
   // Algorithm selection.
-  Algorithm algorithm;
+  Algorithm algorithm{UNSPECIFIED};
 
   // Input and output buffers.
   // The output is used as input if input is not specified.

gloo/allreduce_bcube.h

@@ -345,7 +345,7 @@ class AllreduceBcube : public Algorithm {
 #define DEBUG_PRINT_RECV(stage)
 #endif
 
-  void run() {
+  void run() override {
     if (totalNumElems_ == 0) {
       return;
     }

gloo/allreduce_halving_doubling.h

@@ -82,14 +82,7 @@ class AllreduceHalvingDoubling : public Algorithm {
         sendOffsets_(steps_),
         recvOffsets_(steps_),
         sendCounts_(steps_, 0),
-        recvCounts_(steps_, 0),
-        sendCountToLargerBlock_(0),
-        offsetToMyBinaryBlock_(0),
-        myBinaryBlockSize_(0),
-        stepsWithinBlock_(0),
-        rankInBinaryBlock_(0),
-        nextSmallerBlockSize_(0),
-        nextLargerBlockSize_(0) {
+        recvCounts_(steps_, 0) {
     if (count_ == 0 || this->contextSize_ == 1) {
       return;
     }
@@ -222,7 +215,7 @@ class AllreduceHalvingDoubling : public Algorithm {
     }
   }
 
-  void run() {
+  void run() override {
     if (count_ == 0) {
       return;
     }
@@ -394,7 +387,7 @@ class AllreduceHalvingDoubling : public Algorithm {
 
   std::vector<size_t> sendCounts_;
   std::vector<size_t> recvCounts_;
-  size_t sendCountToLargerBlock_;
+  size_t sendCountToLargerBlock_{0};
 
   int dummy_;
   std::vector<std::unique_ptr<transport::Buffer>> sendNotificationBufs_;
@@ -404,12 +397,12 @@ class AllreduceHalvingDoubling : public Algorithm {
   // binary blocks and keep track of which block each process is in, as well as
   // the adjoining larger and smaller blocks (with which communication will be
   // required)
-  uint32_t offsetToMyBinaryBlock_;
-  uint32_t myBinaryBlockSize_;
-  uint32_t stepsWithinBlock_;
-  uint32_t rankInBinaryBlock_;
-  uint32_t nextSmallerBlockSize_;
-  uint32_t nextLargerBlockSize_;
+  uint32_t offsetToMyBinaryBlock_{0};
+  uint32_t myBinaryBlockSize_{0};
+  uint32_t stepsWithinBlock_{0};
+  uint32_t rankInBinaryBlock_{0};
+  uint32_t nextSmallerBlockSize_{0};
+  uint32_t nextLargerBlockSize_{0};
 
   int slotOffset_;
 };

gloo/allreduce_local.cc

@@ -8,7 +8,7 @@
 
 #include "gloo/allreduce_local.h"
 
-#include <string.h>
+#include <cstring>
 
 namespace gloo {
 

gloo/allreduce_local.h

@@ -22,9 +22,9 @@ class AllreduceLocal : public Algorithm {
       const int count,
       const ReductionFunction<T>* fn = ReductionFunction<T>::sum);
 
-  virtual ~AllreduceLocal() = default;
+  ~AllreduceLocal() override = default;
 
-  virtual void run() override;
+  void run() override;
 
  protected:
   std::vector<T*> ptrs_;

gloo/allreduce_ring.h

@@ -57,7 +57,7 @@ class AllreduceRing : public Algorithm {
         rightPair->createRecvBuffer(notificationSlot, &dummy_, sizeof(dummy_));
   }
 
-  virtual ~AllreduceRing() {
+  ~AllreduceRing() override {
     if (inbox_ != nullptr) {
       free(inbox_);
     }
@@ -66,7 +66,7 @@ class AllreduceRing : public Algorithm {
     }
   }
 
-  void run() {
+  void run() override {
     if (count_ == 0) {
       return;
     }

gloo/allreduce_ring_chunked.h

@@ -41,8 +41,8 @@ class AllreduceRingChunked : public Algorithm {
     chunkBytes_ = chunkSize_ * sizeof(T);
 
     // Allocate inboxes
-    for (int i = 0; i < 2; i++) {
-      inbox_[i] = static_cast<T*>(malloc(bytes_));
+    for (auto& i : inbox_) {
+      i = static_cast<T*>(malloc(bytes_));
     }
 
     if (count_ == 0 || this->contextSize_ == 1) {
@@ -72,15 +72,15 @@ class AllreduceRingChunked : public Algorithm {
         rightPair->createRecvBuffer(notificationSlot, &dummy_, sizeof(dummy_));
   }
 
-  virtual ~AllreduceRingChunked() {
-    for (int i = 0; i < 2; i++) {
-      if (inbox_[i] != nullptr) {
-        free(inbox_[i]);
+  ~AllreduceRingChunked() override {
+    for (auto& i : inbox_) {
+      if (i != nullptr) {
+        free(i);
       }
     }
   }
 
-  void run() {
+  void run() override {
     if (count_ == 0) {
       return;
     }

gloo/barrier.h

@@ -19,7 +19,7 @@ class Barrier : public Algorithm {
   explicit Barrier(const std::shared_ptr<Context>& context)
       : Algorithm(context) {}
 
-  virtual ~Barrier() {}
+  ~Barrier() override = default;
 };
 
 class BarrierOptions {

gloo/barrier_all_to_all.h

@@ -17,7 +17,7 @@ class BarrierAllToAll : public Barrier {
   explicit BarrierAllToAll(const std::shared_ptr<Context>& context)
       : Barrier(context) {}
 
-  void run() {
+  void run() override {
     // Create send/recv buffers for every peer
     auto slot = this->context_->nextSlot();
 

gloo/barrier_all_to_one.h

@@ -19,7 +19,7 @@ class BarrierAllToOne : public Barrier {
       int rootRank = 0)
       : Barrier(context), rootRank_(rootRank) {}
 
-  void run() {
+  void run() override {
     auto slot = this->context_->nextSlot();
     auto timeout = this->context_->getTimeout();
 

gloo/broadcast_one_to_all.h

@@ -38,7 +38,7 @@ class BroadcastOneToAll : public Algorithm {
     GLOO_ENFORCE_LT(rootPointerRank_, ptrs_.size());
   }
 
-  void run() {
+  void run() override {
     if (contextSize_ == 1) {
       broadcastLocally();
       return;

gloo/common/logging.h

@@ -83,7 +83,7 @@ class EnforceNotMet : public std::exception {
     return msg_stack_;
   }
 
-  virtual const char* what() const noexcept override;
+  const char* what() const noexcept override;
 
  private:
   std::vector<std::string> msg_stack_;

gloo/common/memory.h

@@ -74,7 +74,7 @@ class ShareableNonOwningPtr;
 template <typename T>
 class NonOwningPtr final {
  public:
-  NonOwningPtr() {}
+  NonOwningPtr() = default;
 
   explicit NonOwningPtr(const WeakNonOwningPtr<T>& ptr)
       : ptr_(ptr.ptr_.lock()) {}
@@ -99,7 +99,7 @@ class NonOwningPtr final {
 template <typename T>
 class WeakNonOwningPtr final {
  public:
-  WeakNonOwningPtr() {}
+  WeakNonOwningPtr() = default;
 
   explicit WeakNonOwningPtr(const ShareableNonOwningPtr<T>& ref)
       : ptr_(ref.ptr_) {}

gloo/context.cc

@@ -18,13 +18,13 @@ namespace gloo {
 static const std::chrono::seconds kTimeoutDefault = std::chrono::seconds(30);
 
 Context::Context(int rank, int size, int base)
-    : rank(rank), size(size), base(base), slot_(0), timeout_(kTimeoutDefault) {
+    : rank(rank), size(size), base(base),  timeout_(kTimeoutDefault) {
   GLOO_ENFORCE_GE(rank, 0);
   GLOO_ENFORCE_LT(rank, size);
   GLOO_ENFORCE_GE(size, 1);
 }
 
-Context::~Context() {}
+Context::~Context() = default;
 
 std::shared_ptr<transport::Device>& Context::getDevice() {
   GLOO_ENFORCE(device_, "Device not set!");

gloo/context.h

@@ -60,7 +60,7 @@ class Context {
  protected:
   std::shared_ptr<transport::Device> device_;
   std::shared_ptr<transport::Context> transportContext_;
-  int slot_;
+  int slot_{0};
   std::chrono::milliseconds timeout_;
 };
 

gloo/rendezvous/context.cc

@@ -9,6 +9,7 @@
 #include "gloo/rendezvous/context.h"
 
 #include <memory>
+#include <utility>
 
 #include "gloo/common/logging.h"
 #include "gloo/rendezvous/store.h"
@@ -20,7 +21,7 @@ namespace rendezvous {
 Context::Context(int rank, int size, int base)
     : ::gloo::Context(rank, size, base) {}
 
-Context::~Context() {}
+Context::~Context() = default;
 
 void Context::connectFullMesh(
     std::shared_ptr<rendezvous::Store> store,
@@ -35,7 +36,7 @@ void Context::connectFullMesh(
 }
 
 ContextFactory::ContextFactory(std::shared_ptr<::gloo::Context> backingContext)
-    : backingContext_(backingContext) {
+    : backingContext_(std::move(backingContext)) {
   // We make sure that we have a fully connected context
   for (auto i = 0; i < backingContext_->size; i++) {
     if (i == backingContext_->rank) {

gloo/rendezvous/file_store.cc

@@ -8,13 +8,13 @@
 
 #include "gloo/rendezvous/file_store.h"
 
-#include <errno.h>
 #include <fcntl.h>
-#include <limits.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
 #include <array>
+#include <cerrno>
+#include <climits>
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
 #include <fstream>
 #include <functional>
 #include <iostream>

gloo/rendezvous/file_store.h

@@ -19,18 +19,17 @@ namespace rendezvous {
 class FileStore : public Store {
  public:
   explicit FileStore(const std::string& path);
-  virtual ~FileStore() {}
+  ~FileStore() override = default;
 
-  virtual void set(const std::string& key, const std::vector<char>& data)
-      override;
+  void set(const std::string& key, const std::vector<char>& data) override;
 
-  virtual std::vector<char> get(const std::string& key) override;
+  std::vector<char> get(const std::string& key) override;
 
-  virtual void wait(const std::vector<std::string>& keys) override {
+  void wait(const std::vector<std::string>& keys) override {
     wait(keys, Store::kDefaultTimeout);
   }
 
-  virtual void wait(
+  void wait(
       const std::vector<std::string>& keys,
       const std::chrono::milliseconds& timeout) override;
 

gloo/rendezvous/hash_store.h

@@ -19,18 +19,17 @@ namespace rendezvous {
 
 class HashStore : public Store {
  public:
-  virtual ~HashStore() {}
+  ~HashStore() override = default;
 
-  virtual void set(const std::string& key, const std::vector<char>& data)
-      override;
+  void set(const std::string& key, const std::vector<char>& data) override;
 
-  virtual std::vector<char> get(const std::string& key) override;
+  std::vector<char> get(const std::string& key) override;
 
-  virtual void wait(const std::vector<std::string>& keys) override {
+  void wait(const std::vector<std::string>& keys) override {
     wait(keys, Store::kDefaultTimeout);
   }
 
-  virtual void wait(
+  void wait(
       const std::vector<std::string>& keys,
       const std::chrono::milliseconds& timeout) override;