diff --git a/gloo/algorithm.cc b/gloo/algorithm.cc index 9488ff59d..a0be5b859 100644 --- a/gloo/algorithm.cc +++ b/gloo/algorithm.cc @@ -21,7 +21,7 @@ Algorithm::Algorithm(const std::shared_ptr& context) contextSize_(context_->size) {} // Have to provide implementation for pure virtual destructor. -Algorithm::~Algorithm() noexcept(false) {} +Algorithm::~Algorithm() noexcept(false) = default; std::unique_ptr& Algorithm::getPair(int i) { return context_->getPair(i); diff --git a/gloo/algorithm.h b/gloo/algorithm.h index 07abd1464..7b8dc5bc4 100644 --- a/gloo/algorithm.h +++ b/gloo/algorithm.h @@ -100,7 +100,7 @@ const ReductionFunction* ReductionFunction::max = template class LocalOp { public: - virtual ~LocalOp() noexcept(false) {} + virtual ~LocalOp() noexcept(false) = default; virtual void runAsync() = 0; virtual void wait() = 0; diff --git a/gloo/allgather_ring.h b/gloo/allgather_ring.h index c0fee2e5c..7acd0d223 100644 --- a/gloo/allgather_ring.h +++ b/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; diff --git a/gloo/allreduce.cc b/gloo/allreduce.cc index 080f7f302..88774f735 100644 --- a/gloo/allreduce.cc +++ b/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; } diff --git a/gloo/allreduce.h b/gloo/allreduce.h index 904eb8b32..ff3e94879 100644 --- a/gloo/allreduce.h +++ b/gloo/allreduce.h @@ -42,9 +42,7 @@ struct AllreduceOptionsImpl { }; explicit AllreduceOptionsImpl(const std::shared_ptr& context) - : context(context), - timeout(context->getTimeout()), - algorithm(UNSPECIFIED) {} + : context(context), timeout(context->getTimeout()) {} std::shared_ptr context; @@ -52,7 +50,7 @@ struct AllreduceOptionsImpl { 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. diff --git a/gloo/allreduce_bcube.h b/gloo/allreduce_bcube.h index c91fa130d..f5508e5c3 100644 --- a/gloo/allreduce_bcube.h +++ b/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; } diff --git a/gloo/allreduce_halving_doubling.h b/gloo/allreduce_halving_doubling.h index 256fd8a92..733d7ab84 100644 --- a/gloo/allreduce_halving_doubling.h +++ b/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 sendCounts_; std::vector recvCounts_; - size_t sendCountToLargerBlock_; + size_t sendCountToLargerBlock_{0}; int dummy_; std::vector> 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_; }; diff --git a/gloo/allreduce_local.cc b/gloo/allreduce_local.cc index 2d1109f08..c4000bdb5 100644 --- a/gloo/allreduce_local.cc +++ b/gloo/allreduce_local.cc @@ -8,7 +8,7 @@ #include "gloo/allreduce_local.h" -#include +#include namespace gloo { diff --git a/gloo/allreduce_local.h b/gloo/allreduce_local.h index ed2285da4..b1e513997 100644 --- a/gloo/allreduce_local.h +++ b/gloo/allreduce_local.h @@ -22,9 +22,9 @@ class AllreduceLocal : public Algorithm { const int count, const ReductionFunction* fn = ReductionFunction::sum); - virtual ~AllreduceLocal() = default; + ~AllreduceLocal() override = default; - virtual void run() override; + void run() override; protected: std::vector ptrs_; diff --git a/gloo/allreduce_ring.h b/gloo/allreduce_ring.h index c1e5765cb..ce44ec396 100644 --- a/gloo/allreduce_ring.h +++ b/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; } diff --git a/gloo/allreduce_ring_chunked.h b/gloo/allreduce_ring_chunked.h index 5ac12aa44..c03dd5ebf 100644 --- a/gloo/allreduce_ring_chunked.h +++ b/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(malloc(bytes_)); + for (auto& i : inbox_) { + i = static_cast(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; } diff --git a/gloo/barrier.h b/gloo/barrier.h index f6a9da5a1..13bbac76c 100644 --- a/gloo/barrier.h +++ b/gloo/barrier.h @@ -19,7 +19,7 @@ class Barrier : public Algorithm { explicit Barrier(const std::shared_ptr& context) : Algorithm(context) {} - virtual ~Barrier() {} + ~Barrier() override = default; }; class BarrierOptions { diff --git a/gloo/barrier_all_to_all.h b/gloo/barrier_all_to_all.h index 88235bc29..3b53c6590 100644 --- a/gloo/barrier_all_to_all.h +++ b/gloo/barrier_all_to_all.h @@ -17,7 +17,7 @@ class BarrierAllToAll : public Barrier { explicit BarrierAllToAll(const std::shared_ptr& context) : Barrier(context) {} - void run() { + void run() override { // Create send/recv buffers for every peer auto slot = this->context_->nextSlot(); diff --git a/gloo/barrier_all_to_one.h b/gloo/barrier_all_to_one.h index 6fba8597e..e3bf803aa 100644 --- a/gloo/barrier_all_to_one.h +++ b/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(); diff --git a/gloo/broadcast_one_to_all.h b/gloo/broadcast_one_to_all.h index e599e102f..4373054d7 100644 --- a/gloo/broadcast_one_to_all.h +++ b/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; diff --git a/gloo/common/logging.h b/gloo/common/logging.h index b1fa59de4..068f20045 100644 --- a/gloo/common/logging.h +++ b/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 msg_stack_; diff --git a/gloo/common/memory.h b/gloo/common/memory.h index e7780c58c..aa51260c3 100644 --- a/gloo/common/memory.h +++ b/gloo/common/memory.h @@ -74,7 +74,7 @@ class ShareableNonOwningPtr; template class NonOwningPtr final { public: - NonOwningPtr() {} + NonOwningPtr() = default; explicit NonOwningPtr(const WeakNonOwningPtr& ptr) : ptr_(ptr.ptr_.lock()) {} @@ -99,7 +99,7 @@ class NonOwningPtr final { template class WeakNonOwningPtr final { public: - WeakNonOwningPtr() {} + WeakNonOwningPtr() = default; explicit WeakNonOwningPtr(const ShareableNonOwningPtr& ref) : ptr_(ref.ptr_) {} diff --git a/gloo/context.cc b/gloo/context.cc index fd9b83c7b..392114733 100644 --- a/gloo/context.cc +++ b/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& Context::getDevice() { GLOO_ENFORCE(device_, "Device not set!"); diff --git a/gloo/context.h b/gloo/context.h index ddd6221b7..d7d7a330b 100644 --- a/gloo/context.h +++ b/gloo/context.h @@ -60,7 +60,7 @@ class Context { protected: std::shared_ptr device_; std::shared_ptr transportContext_; - int slot_; + int slot_{0}; std::chrono::milliseconds timeout_; }; diff --git a/gloo/rendezvous/context.cc b/gloo/rendezvous/context.cc index f71739d3a..28814c477 100644 --- a/gloo/rendezvous/context.cc +++ b/gloo/rendezvous/context.cc @@ -9,6 +9,7 @@ #include "gloo/rendezvous/context.h" #include +#include #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 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) { diff --git a/gloo/rendezvous/file_store.cc b/gloo/rendezvous/file_store.cc index 1202e80f0..dba96e2c7 100644 --- a/gloo/rendezvous/file_store.cc +++ b/gloo/rendezvous/file_store.cc @@ -8,13 +8,13 @@ #include "gloo/rendezvous/file_store.h" -#include #include -#include -#include -#include -#include #include +#include +#include +#include +#include +#include #include #include #include diff --git a/gloo/rendezvous/file_store.h b/gloo/rendezvous/file_store.h index 480ce4acd..b9172edf6 100644 --- a/gloo/rendezvous/file_store.h +++ b/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& data) - override; + void set(const std::string& key, const std::vector& data) override; - virtual std::vector get(const std::string& key) override; + std::vector get(const std::string& key) override; - virtual void wait(const std::vector& keys) override { + void wait(const std::vector& keys) override { wait(keys, Store::kDefaultTimeout); } - virtual void wait( + void wait( const std::vector& keys, const std::chrono::milliseconds& timeout) override; diff --git a/gloo/rendezvous/hash_store.h b/gloo/rendezvous/hash_store.h index c015ace17..58b18e2b5 100644 --- a/gloo/rendezvous/hash_store.h +++ b/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& data) - override; + void set(const std::string& key, const std::vector& data) override; - virtual std::vector get(const std::string& key) override; + std::vector get(const std::string& key) override; - virtual void wait(const std::vector& keys) override { + void wait(const std::vector& keys) override { wait(keys, Store::kDefaultTimeout); } - virtual void wait( + void wait( const std::vector& keys, const std::chrono::milliseconds& timeout) override; diff --git a/gloo/rendezvous/prefix_store.cc b/gloo/rendezvous/prefix_store.cc index 5fccd565f..fdd7591b0 100644 --- a/gloo/rendezvous/prefix_store.cc +++ b/gloo/rendezvous/prefix_store.cc @@ -9,14 +9,13 @@ #include "prefix_store.h" #include +#include namespace gloo { namespace rendezvous { -PrefixStore::PrefixStore( - const std::string& prefix, - std::shared_ptr store) - : prefix_(prefix), store_(std::move(store)) {} +PrefixStore::PrefixStore(std::string prefix, std::shared_ptr store) + : prefix_(std::move(prefix)), store_(std::move(store)) {} std::string PrefixStore::joinKey(const std::string& key) { std::stringstream ss; diff --git a/gloo/rendezvous/prefix_store.h b/gloo/rendezvous/prefix_store.h index b547c2c6a..a41806eb8 100644 --- a/gloo/rendezvous/prefix_store.h +++ b/gloo/rendezvous/prefix_store.h @@ -17,32 +17,30 @@ namespace rendezvous { class PrefixStore : public Store { public: - PrefixStore(const std::string& prefix, std::shared_ptr store); + PrefixStore(std::string prefix, std::shared_ptr store); - virtual ~PrefixStore() {} + ~PrefixStore() override = default; - virtual void set(const std::string& key, const std::vector& data) - override; + void set(const std::string& key, const std::vector& data) override; - virtual std::vector get(const std::string& key) override; + std::vector get(const std::string& key) override; - virtual void wait(const std::vector& keys) override { + void wait(const std::vector& keys) override { wait(keys, Store::kDefaultTimeout); } - virtual void wait( + void wait( const std::vector& keys, const std::chrono::milliseconds& timeout) override; - virtual bool has_v2_support() override; - virtual std::vector> multi_get( + bool has_v2_support() override; + std::vector> multi_get( const std::vector& keys) override; - virtual void multi_set( + void multi_set( const std::vector& keys, const std::vector>& values) override; - virtual void append(const std::string& key, const std::vector& data) - override; - virtual int64_t add(const std::string& key, int64_t value) override; + void append(const std::string& key, const std::vector& data) override; + int64_t add(const std::string& key, int64_t value) override; protected: const std::string prefix_; diff --git a/gloo/rendezvous/store.h b/gloo/rendezvous/store.h index e2569d0ef..657c59b18 100644 --- a/gloo/rendezvous/store.h +++ b/gloo/rendezvous/store.h @@ -27,48 +27,47 @@ class Store : public IStore { static constexpr std::chrono::milliseconds kDefaultTimeout = std::chrono::seconds(30); - virtual ~Store() = default; + ~Store() override = default; - virtual void set(const std::string& key, const std::vector& data) = 0; + void set(const std::string& key, const std::vector& data) override = 0; - virtual std::vector get(const std::string& key) = 0; + std::vector get(const std::string& key) override = 0; virtual void wait(const std::vector& keys) = 0; - virtual void wait( + void wait( const std::vector& keys, - const std::chrono::milliseconds& /*timeout*/) { + const std::chrono::milliseconds& /*timeout*/) override { // Base implementation ignores the timeout for backward compatibility. // Derived Store implementations should override this function. wait(keys); } - virtual bool has_v2_support() { + bool has_v2_support() override { // If True, the following operations are guaranteed to be efficiently and // correclty implemented. return false; } - virtual std::vector> multi_get( - const std::vector& /*keys*/) { + std::vector> multi_get( + const std::vector& /*keys*/) override { GLOO_THROW_INVALID_OPERATION_EXCEPTION( "this store doesn't support multi_get"); } - virtual void multi_set( + void multi_set( const std::vector& /*keys*/, - const std::vector>& /*values*/) { + const std::vector>& /*values*/) override { GLOO_THROW_INVALID_OPERATION_EXCEPTION( "this store doesn't support multi_set"); } - virtual void append( - const std::string& key, - const std::vector& /*data*/) { + void append(const std::string& key, const std::vector& /*data*/) + override { GLOO_THROW_INVALID_OPERATION_EXCEPTION("this store doesn't support append"); } - virtual int64_t add(const std::string& key, int64_t value) { + int64_t add(const std::string& key, int64_t value) override { GLOO_THROW_INVALID_OPERATION_EXCEPTION("this store doesn't support add"); } }; diff --git a/gloo/scatter.cc b/gloo/scatter.cc index c1810ca47..74da01e68 100644 --- a/gloo/scatter.cc +++ b/gloo/scatter.cc @@ -30,8 +30,8 @@ void scatter(ScatterOptions& opts) { // Assert there are as many inputs as ranks to send to. GLOO_ENFORCE_EQ(in.size(), context->size); // Assert the size of all inputs is identical to the output. - for (size_t i = 0; i < in.size(); i++) { - GLOO_ENFORCE_EQ(in[i]->size, out->size); + for (const auto& i : in) { + GLOO_ENFORCE_EQ(i->size, out->size); } } diff --git a/gloo/test/allreduce_test.cc b/gloo/test/allreduce_test.cc index 7c25aee5f..a2c58bf29 100644 --- a/gloo/test/allreduce_test.cc +++ b/gloo/test/allreduce_test.cc @@ -6,7 +6,7 @@ * LICENSE file in the root directory of this source tree. */ -#include +#include #include #include @@ -53,8 +53,8 @@ using ParamHP = std::tuple>; template class AllreduceConstructorTest : public BaseTest {}; -typedef ::testing::Types, AllreduceRingChunked> - AllreduceTypes; +using AllreduceTypes = + ::testing::Types, AllreduceRingChunked>; TYPED_TEST_CASE(AllreduceConstructorTest, AllreduceTypes); TYPED_TEST(AllreduceConstructorTest, InlinePointers) { diff --git a/gloo/test/base_test.h b/gloo/test/base_test.h index 307b4a8a6..77b6fb5cd 100644 --- a/gloo/test/base_test.h +++ b/gloo/test/base_test.h @@ -84,14 +84,14 @@ class BaseTest : public ::testing::Test { std::mutex mutex; std::vector errors; for (int rank = 0; rank < size; rank++) { - threads.push_back(std::thread([&, rank]() { + threads.emplace_back([&, rank]() { try { fn(rank); } catch (const std::exception& e) { std::lock_guard lock(mutex); - errors.push_back(e.what()); + errors.emplace_back(e.what()); } - })); + }); } // Wait for threads to complete diff --git a/gloo/test/main.cc b/gloo/test/main.cc index cbc662a90..0781a46f6 100644 --- a/gloo/test/main.cc +++ b/gloo/test/main.cc @@ -10,6 +10,7 @@ // One-time init to use EPIPE errors instead of SIGPIPE #ifndef _WIN32 +#include namespace { struct Initializer { Initializer() { diff --git a/gloo/transport/address.cc b/gloo/transport/address.cc index a59fb494a..bb4f7ee6a 100644 --- a/gloo/transport/address.cc +++ b/gloo/transport/address.cc @@ -12,7 +12,7 @@ namespace gloo { namespace transport { // Have to provide implementation for pure virtual destructor. -Address::~Address() {} +Address::~Address() = default; } // namespace transport } // namespace gloo diff --git a/gloo/transport/buffer.cc b/gloo/transport/buffer.cc index 549f93a03..58eebb5fa 100644 --- a/gloo/transport/buffer.cc +++ b/gloo/transport/buffer.cc @@ -12,7 +12,7 @@ namespace gloo { namespace transport { // Have to provide implementation for pure virtual destructor. -Buffer::~Buffer() {} +Buffer::~Buffer() = default; } // namespace transport } // namespace gloo diff --git a/gloo/transport/buffer.h b/gloo/transport/buffer.h index 382061a87..330c75d37 100644 --- a/gloo/transport/buffer.h +++ b/gloo/transport/buffer.h @@ -16,7 +16,7 @@ namespace transport { class Buffer { public: explicit Buffer(int slot, void* ptr, size_t size) - : slot_(slot), ptr_(ptr), size_(size), debug_(false) {} + : slot_(slot), ptr_(ptr), size_(size) {} virtual ~Buffer() = 0; virtual void setDebug(bool debug) { @@ -37,7 +37,7 @@ class Buffer { int slot_; void* ptr_; size_t size_; - bool debug_; + bool debug_{false}; }; } // namespace transport diff --git a/gloo/transport/context.cc b/gloo/transport/context.cc index 4b4cfadbf..e333989f6 100644 --- a/gloo/transport/context.cc +++ b/gloo/transport/context.cc @@ -17,7 +17,7 @@ Context::Context(int rank, int size) : rank(rank), size(size) { } // Have to provide implementation for pure virtual destructor. -Context::~Context() {} +Context::~Context() = default; std::unique_ptr& Context::getPair(int rank_2) { return pairs_.at(rank_2); @@ -101,7 +101,7 @@ std::vector Context::extractAddress( } Context::LazyTally::LazyTally(std::vector& vec, slot_t slot) - : vec_(vec), slot_(slot), initialized_(false) {} + : vec_(vec), slot_(slot) {} Context::LazyTally::~LazyTally() { // Remove empty tally from vector. diff --git a/gloo/transport/context.h b/gloo/transport/context.h index 1733b7979..adc190c8f 100644 --- a/gloo/transport/context.h +++ b/gloo/transport/context.h @@ -224,7 +224,7 @@ class Context { std::vector::iterator it_; // If the iterator has been initialized. - bool initialized_; + bool initialized_{false}; // Initialize iterator to Tally instance for this slot. void initialize_iterator(); diff --git a/gloo/transport/device.cc b/gloo/transport/device.cc index 4b30d3808..50e4e5b06 100644 --- a/gloo/transport/device.cc +++ b/gloo/transport/device.cc @@ -12,7 +12,7 @@ namespace gloo { namespace transport { // Have to provide implementation for pure virtual destructor. -Device::~Device() {} +Device::~Device() = default; } // namespace transport } // namespace gloo diff --git a/gloo/transport/pair.cc b/gloo/transport/pair.cc index a1af2d1fa..efcbcb2d4 100644 --- a/gloo/transport/pair.cc +++ b/gloo/transport/pair.cc @@ -12,7 +12,7 @@ namespace gloo { namespace transport { // Have to provide implementation for pure virtual destructor. -Pair::~Pair() {} +Pair::~Pair() = default; } // namespace transport } // namespace gloo diff --git a/gloo/transport/unbound_buffer.cc b/gloo/transport/unbound_buffer.cc index cdd9aa0ad..049bda7ee 100644 --- a/gloo/transport/unbound_buffer.cc +++ b/gloo/transport/unbound_buffer.cc @@ -12,7 +12,7 @@ namespace gloo { namespace transport { // Have to provide implementation for pure virtual destructor. -UnboundBuffer::~UnboundBuffer() {} +UnboundBuffer::~UnboundBuffer() = default; } // namespace transport } // namespace gloo diff --git a/gloo/transport/uv/address.cc b/gloo/transport/uv/address.cc index 9b69b9ece..a33a8cbfe 100644 --- a/gloo/transport/uv/address.cc +++ b/gloo/transport/uv/address.cc @@ -8,7 +8,7 @@ #include -#include +#include #include diff --git a/gloo/transport/uv/address.h b/gloo/transport/uv/address.h index 00822e474..96f3b9d11 100644 --- a/gloo/transport/uv/address.h +++ b/gloo/transport/uv/address.h @@ -26,7 +26,7 @@ class Address : public ::gloo::transport::Address { public: using sequence_type = int; - Address() {} + Address() = default; Address(struct sockaddr_storage ss, sequence_type seq = -1); @@ -36,9 +36,9 @@ class Address : public ::gloo::transport::Address { Address& operator=(const Address& other); Address(const Address& other); - virtual std::vector bytes() const override; + std::vector bytes() const override; - virtual std::string str() const override; + std::string str() const override; const struct sockaddr_storage& getSockaddr() const { return impl_.ss; @@ -66,7 +66,7 @@ class Address : public ::gloo::transport::Address { sequence_type seq = -1; }; - static_assert(std::is_trivially_copyable::value, "!"); + static_assert(std::is_trivially_copyable_v, "!"); Impl impl_; mutable std::mutex m_; diff --git a/gloo/transport/uv/context.h b/gloo/transport/uv/context.h index 61ff7aa36..aaaf430f3 100644 --- a/gloo/transport/uv/context.h +++ b/gloo/transport/uv/context.h @@ -45,7 +45,7 @@ class Context final : public ::gloo::transport::Context, public: Context(std::shared_ptr device, int rank, int size); - virtual ~Context(); + ~Context() override; std::unique_ptr& createPair(int rank) override; diff --git a/gloo/transport/uv/device.cc b/gloo/transport/uv/device.cc index 1dc986433..62a795d4c 100644 --- a/gloo/transport/uv/device.cc +++ b/gloo/transport/uv/device.cc @@ -12,7 +12,9 @@ #include #include #include +#include #include +#include #include // @manual @@ -179,7 +181,7 @@ std::shared_ptr CreateDevice(struct attr attr) { return std::make_shared(attr); } -Device::Device(const struct attr& attr) : attr_(attr) { +Device::Device(struct attr attr) : attr_(std::move(attr)) { loop_ = libuv::Loop::create(); // Use async handle to trigger the event loop to @@ -208,7 +210,7 @@ Device::Device(const struct attr& attr) : attr_(attr) { addr_ = Address(listener_->sockname()); // Run uv_run on private thread. - thread_.reset(new std::thread([this] { loop_->run(); })); + thread_ = std::make_unique([this] { loop_->run(); }); } Device::~Device() { diff --git a/gloo/transport/uv/device.h b/gloo/transport/uv/device.h index 5dc2d6524..e18350459 100644 --- a/gloo/transport/uv/device.h +++ b/gloo/transport/uv/device.h @@ -33,7 +33,7 @@ namespace uv { // network interface. Whatever is used will be resolved to a // sockaddr_storage struct to finally bind a socket to. struct attr { - attr() {} + attr() = default; /* implicit */ attr(const char* ptr) : hostname(ptr) {} std::string hostname; @@ -75,17 +75,16 @@ std::shared_ptr<::gloo::transport::Device> CreateDevice(struct attr); class Device : public ::gloo::transport::Device, public std::enable_shared_from_this { public: - explicit Device(const struct attr& attr); + explicit Device(struct attr attr); - virtual ~Device(); + ~Device() override; - virtual std::string str() const override; + std::string str() const override; - virtual const std::string& getPCIBusID() const override; + const std::string& getPCIBusID() const override; - virtual std::shared_ptr<::gloo::transport::Context> createContext( - int rank, - int size) override; + std::shared_ptr<::gloo::transport::Context> createContext(int rank, int size) + override; protected: using ConnectCallback = std::function< diff --git a/gloo/transport/uv/libuv.h b/gloo/transport/uv/libuv.h index ddc2a807e..86f92a567 100644 --- a/gloo/transport/uv/libuv.h +++ b/gloo/transport/uv/libuv.h @@ -220,7 +220,7 @@ class Loop : public std::enable_shared_from_this { explicit Loop(std::unique_ptr ptr) : loop_(std::move(ptr)) {} static std::shared_ptr create() { - auto ptr = std::unique_ptr(new uv_loop_t); + auto ptr = std::make_unique(); auto loop = std::make_shared(std::move(ptr)); auto rv = uv_loop_init(loop->loop_.get()); UV_ASSERT(rv, "uv_loop_init"); @@ -228,18 +228,15 @@ class Loop : public std::enable_shared_from_this { } template - typename std:: - enable_if::value, std::shared_ptr>::type - resource(Args&&... args) { + std::enable_if_t, std::shared_ptr> + resource(Args&&... args) { auto handle = T::create(shared_from_this(), std::forward(args)...); handle->init(); return handle; } template - typename std::enable_if< - std::is_base_of::value, - std::shared_ptr>::type + std::enable_if_t, std::shared_ptr> resource(Args&&... args) { return T::create(shared_from_this(), std::forward(args)...); } @@ -281,13 +278,13 @@ class Resource : public Emitter, public std::enable_shared_from_this { template const R* get() const noexcept { - static_assert(!std::is_same::value, "!"); + static_assert(!std::is_same_v, "!"); return reinterpret_cast(&resource_); } template R* get() noexcept { - static_assert(!std::is_same::value, "!"); + static_assert(!std::is_same_v, "!"); return reinterpret_cast(&resource_); } @@ -346,7 +343,7 @@ class Handle : public Resource, public BaseHandle { } template - typename std::invoke_result::type invoke(F&& f, Args&&... args) { + std::invoke_result_t invoke(F&& f, Args&&... args) { return std::forward(f)(std::forward(args)...); } @@ -382,9 +379,9 @@ class Request : public Resource, public BaseRequest { // The request is leaked if the call is successful, under the // assumption that it is unleaked when the callback gets called. template - typename std::enable_if< - !std::is_void::type>::value, - typename std::invoke_result::type>::type + std::enable_if_t< + !std::is_void_v>, + std::invoke_result_t> invoke(F&& f, Args&&... args) { auto err = std::forward(f)(std::forward(args)...); if (err) { @@ -457,8 +454,7 @@ class ReadEvent { size_t length; template - typename std::enable_if::value, T>::type as() - const { + std::enable_if_t, T> as() const { if (length != sizeof(T)) { abort(); } @@ -631,7 +627,7 @@ class TCP final : public Handle { template void write(T t) { static_assert( - std::is_trivially_copyable::value, + std::is_trivially_copyable_v, "Only trivially copyable types can be written directly."); auto data = std::unique_ptr( new char[sizeof(T)], [](char* ptr) { delete[] ptr; }); diff --git a/gloo/transport/uv/pair.cc b/gloo/transport/uv/pair.cc index 4db8bf32c..e75dc3e3e 100644 --- a/gloo/transport/uv/pair.cc +++ b/gloo/transport/uv/pair.cc @@ -31,9 +31,8 @@ Pair::Pair( device_(device), rank_(rank), timeout_(timeout), - addr_(device_->nextAddress()), - state_(INITIALIZED), - errno_(0) {} + addr_(device_->nextAddress()) + {} Pair::~Pair() { std::unique_lock lock(mutex_); @@ -82,11 +81,7 @@ void Pair::connect(const std::vector& bytes) { addr_, peer, timeout_, - std::bind( - &Pair::connectCallback, - this, - std::placeholders::_1, - std::placeholders::_2)); + [this](auto && PH1, auto && PH2) { connectCallback(std::forward(PH1), std::forward(PH2)); }); // Wait for callback to fire. // @@ -122,16 +117,11 @@ void Pair::connectCallback( state_ = CONNECTED; // Setup event listeners. - handle_->on(std::bind( - &Pair::onClose, this, std::placeholders::_1, std::placeholders::_2)); - handle_->on(std::bind( - &Pair::onEnd, this, std::placeholders::_1, std::placeholders::_2)); - handle_->on(std::bind( - &Pair::onError, this, std::placeholders::_1, std::placeholders::_2)); - handle_->on(std::bind( - &Pair::onRead, this, std::placeholders::_1, std::placeholders::_2)); - handle_->on(std::bind( - &Pair::onWrite, this, std::placeholders::_1, std::placeholders::_2)); + handle_->on([this](auto && PH1, auto && PH2) { onClose(std::forward(PH1), std::forward(PH2)); }); + handle_->on([this](auto && PH1, auto && PH2) { onEnd(std::forward(PH1), std::forward(PH2)); }); + handle_->on([this](auto && PH1, auto && PH2) { onError(std::forward(PH1), std::forward(PH2)); }); + handle_->on([this](auto && PH1, auto && PH2) { onRead(std::forward(PH1), std::forward(PH2)); }); + handle_->on([this](auto && PH1, auto && PH2) { onWrite(std::forward(PH1), std::forward(PH2)); }); // Prepare to read next preamble. readNextOp(); diff --git a/gloo/transport/uv/pair.h b/gloo/transport/uv/pair.h index 510c9619b..a9f3f9dd7 100644 --- a/gloo/transport/uv/pair.h +++ b/gloo/transport/uv/pair.h @@ -97,28 +97,28 @@ class Pair : public ::gloo::transport::Pair { int rank, std::chrono::milliseconds timeout); - virtual ~Pair(); + ~Pair() override; Pair(const Pair& that) = delete; Pair& operator=(const Pair& that) = delete; - virtual const Address& address() const override; + const Address& address() const override; - virtual void connect(const std::vector& bytes) override; + void connect(const std::vector& bytes) override; - virtual void setSync(bool sync, bool busyPoll) override { + void setSync(bool sync, bool busyPoll) override { abort(); } - virtual std::unique_ptr<::gloo::transport::Buffer> createSendBuffer( + std::unique_ptr<::gloo::transport::Buffer> createSendBuffer( int slot, void* ptr, size_t size) override { abort(); } - virtual std::unique_ptr<::gloo::transport::Buffer> createRecvBuffer( + std::unique_ptr<::gloo::transport::Buffer> createRecvBuffer( int slot, void* ptr, size_t size) override { @@ -199,10 +199,10 @@ class Pair : public ::gloo::transport::Pair { // State of the pair. This is used so that we can ensure the // underlying connection is closed before we destruct. - State state_; + State state_{INITIALIZED}; // Error state of the handle, if set. - int errno_; + int errno_{0}; // Handle of the connection. // This is set only if state_ == CONNECTED || state_ == CLOSING. diff --git a/gloo/transport/uv/unbound_buffer.cc b/gloo/transport/uv/unbound_buffer.cc index 593020b3c..123818b55 100644 --- a/gloo/transport/uv/unbound_buffer.cc +++ b/gloo/transport/uv/unbound_buffer.cc @@ -12,6 +12,8 @@ #include #include +#include + namespace gloo { namespace transport { namespace uv { @@ -21,14 +23,11 @@ UnboundBuffer::UnboundBuffer( void* ptr, size_t size) : ::gloo::transport::UnboundBuffer(ptr, size), - context_(context), - recvCompletions_(0), - recvRank_(-1), - sendCompletions_(0), - sendRank_(-1), + context_(std::move(context)), + shareableNonOwningPtr_(this) {} -UnboundBuffer::~UnboundBuffer() {} +UnboundBuffer::~UnboundBuffer() = default; void UnboundBuffer::handleRecvCompletion(int rank) { std::lock_guard lock(mutex_); diff --git a/gloo/transport/uv/unbound_buffer.h b/gloo/transport/uv/unbound_buffer.h index 8f0fdb348..78dee38f1 100644 --- a/gloo/transport/uv/unbound_buffer.h +++ b/gloo/transport/uv/unbound_buffer.h @@ -28,7 +28,7 @@ class UnboundBuffer : public ::gloo::transport::UnboundBuffer { public: UnboundBuffer(std::shared_ptr context, void* ptr, size_t size); - virtual ~UnboundBuffer(); + ~UnboundBuffer() override; // If specified, the source of this recv is stored in the rank pointer. // Returns true if it completed, false if it was aborted. @@ -68,10 +68,10 @@ class UnboundBuffer : public ::gloo::transport::UnboundBuffer { bool abortWaitRecv_{false}; bool abortWaitSend_{false}; - int recvCompletions_; - int recvRank_; - int sendCompletions_; - int sendRank_; + int recvCompletions_{0}; + int recvRank_{-1}; + int sendCompletions_{0}; + int sendRank_{-1}; // Allows for sharing weak (non owning) references to "this" without // affecting the lifetime of this instance. diff --git a/gloo/types.h b/gloo/types.h index b9cb94680..b887b4990 100644 --- a/gloo/types.h +++ b/gloo/types.h @@ -91,8 +91,8 @@ class Slot { }; struct float16; -float16 cpu_float2half_rn(float f); -float cpu_half2float(float16 h); +float16 cpu_float2half_rn(float f) noexcept; +float cpu_half2float(float16 h) noexcept; struct alignas(2) float16 { uint16_t x; @@ -127,12 +127,7 @@ struct alignas(2) float16 { return *this; } - float16& operator=(const float16& rhs) { - if (rhs != *this) { - x = rhs.x; - } - return *this; - } + float16& operator=(const float16& rhs) = default; bool operator==(const float16& rhs) const { return x == rhs.x; @@ -249,7 +244,7 @@ inline bool operator>=(const float16& lhs, const float16& rhs) { return cpu_half2float(lhs) >= cpu_half2float(rhs); } -inline float16 cpu_float2half_rn(float f) { +inline float16 cpu_float2half_rn(float f) noexcept { float16 ret; static_assert( @@ -310,7 +305,7 @@ inline float16 cpu_float2half_rn(float f) { return ret; } -inline float cpu_half2float(float16 h) { +inline float cpu_half2float(float16 h) noexcept { unsigned sign = ((h.x >> 15) & 1); unsigned exponent = ((h.x >> 10) & 0x1f); unsigned mantissa = ((h.x & 0x3ff) << 13);