From 5e87504b87d4c66b05210d8d64aaad7bbd45929b Mon Sep 17 00:00:00 2001
From: Oliver Hamlet <oliver@ortham.io>
Date: Mon, 7 Feb 2022 00:15:05 +0000
Subject: [PATCH] Add cppcoreguidelines-special-member-functions clang-tidy
 check

Fix the warnings it emits.

The GameCache copy constructor and assignment operator has also had its
use of locks fixed.
---
 CMakeLists.txt                      |  3 ++-
 src/api/game/game_cache.cpp         | 26 +++++++++++++++++++++-----
 src/api/game/game_cache.h           |  5 ++++-
 src/api/game/load_order_handler.cpp | 15 +++++++++++++++
 src/api/game/load_order_handler.h   |  5 +++++
 5 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 3cf9d109..471e654e 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -509,7 +509,8 @@ if(RUN_CLANG_TIDY)
         ${CLANG_TIDY_COMMON_CHECKS}
         "cppcoreguidelines-avoid-goto"
         "cppcoreguidelines-avoid-magic-numbers"
-        "cppcoreguidelines-non-private-member-variables-in-classes")
+        "cppcoreguidelines-non-private-member-variables-in-classes"
+        "cppcoreguidelines-special-member-functions")
 
     # Skip some checks for tests because they're not worth the noise (e.g. GTest
     # happens to use goto).
diff --git a/src/api/game/game_cache.cpp b/src/api/game/game_cache.cpp
index 02034349..5fde7ec8 100644
--- a/src/api/game/game_cache.cpp
+++ b/src/api/game/game_cache.cpp
@@ -37,18 +37,23 @@ namespace loot {
 GameCache::GameCache() {}
 
 GameCache::GameCache(const GameCache& cache) {
-  lock_guard<mutex> lock(mutex_);
-  lock_guard<mutex> otherLock(cache.mutex_);
+  lock_guard<mutex> lock(cache.mutex_);
 
   plugins_ = cache.plugins_;
   archivePaths_ = cache.archivePaths_;
 }
 
-GameCache& GameCache::operator=(const GameCache& cache) {
-  lock_guard<mutex> lock(mutex_);
-  lock_guard<mutex> otherLock(cache.mutex_);
+GameCache::GameCache(GameCache&& cache) {
+  lock_guard<mutex> lock(cache.mutex_);
 
+  plugins_ = std::move(cache.plugins_);
+  archivePaths_ = std::move(cache.archivePaths_);
+}
+
+GameCache& GameCache::operator=(const GameCache& cache) {
   if (&cache != this) {
+    std::scoped_lock lock(mutex_, cache.mutex_);
+
     plugins_ = cache.plugins_;
     archivePaths_ = cache.archivePaths_;
   }
@@ -56,6 +61,17 @@ GameCache& GameCache::operator=(const GameCache& cache) {
   return *this;
 }
 
+GameCache& GameCache::operator=(GameCache&& cache) {
+  if (&cache != this) {
+    std::scoped_lock lock(mutex_, cache.mutex_);
+
+    plugins_ = std::move(cache.plugins_);
+    archivePaths_ = std::move(cache.archivePaths_);
+  }
+
+  return *this;
+}
+
 std::vector<std::shared_ptr<const Plugin>> GameCache::GetPlugins() const {
   lock_guard<mutex> lock(mutex_);
 
diff --git a/src/api/game/game_cache.h b/src/api/game/game_cache.h
index 9e7985bd..7f072fcb 100644
--- a/src/api/game/game_cache.h
+++ b/src/api/game/game_cache.h
@@ -35,9 +35,12 @@ namespace loot {
 class GameCache {
 public:
   explicit GameCache();
-  explicit GameCache(const GameCache& cache);
+  GameCache(const GameCache& cache);
+  GameCache(GameCache&& cache);
+  ~GameCache() = default;
 
   GameCache& operator=(const GameCache& cache);
+  GameCache& operator=(GameCache&& cache);
 
   std::vector<std::shared_ptr<const Plugin>> GetPlugins() const;
   std::shared_ptr<const Plugin> GetPlugin(const std::string& pluginName) const;
diff --git a/src/api/game/load_order_handler.cpp b/src/api/game/load_order_handler.cpp
index d00b29ab..d4a5f049 100644
--- a/src/api/game/load_order_handler.cpp
+++ b/src/api/game/load_order_handler.cpp
@@ -57,8 +57,23 @@ unsigned int mapGameId(GameType gameType) {
 
 LoadOrderHandler::LoadOrderHandler() : gh_(nullptr) {}
 
+LoadOrderHandler::LoadOrderHandler(LoadOrderHandler&& other) : gh_(other.gh_) {
+  other.gh_ = nullptr;
+}
+
 LoadOrderHandler::~LoadOrderHandler() { lo_destroy_handle(gh_); }
 
+LoadOrderHandler& LoadOrderHandler::operator=(LoadOrderHandler&& other) {
+  if (&other != this) {
+    lo_destroy_handle(gh_);
+
+    gh_ = other.gh_;
+    other.gh_ = nullptr;
+  }
+
+  return *this;
+}
+
 void LoadOrderHandler::Init(const GameType& gameType,
                             const std::filesystem::path& gamePath,
                             const std::filesystem::path& gameLocalAppData) {
diff --git a/src/api/game/load_order_handler.h b/src/api/game/load_order_handler.h
index 8283c046..7885e0aa 100644
--- a/src/api/game/load_order_handler.h
+++ b/src/api/game/load_order_handler.h
@@ -38,8 +38,13 @@ namespace loot {
 class LoadOrderHandler {
 public:
   explicit LoadOrderHandler();
+  LoadOrderHandler(const LoadOrderHandler&) = delete;
+  LoadOrderHandler(LoadOrderHandler&& other);
   ~LoadOrderHandler();
 
+  LoadOrderHandler& operator=(const LoadOrderHandler&) = delete;
+  LoadOrderHandler& operator=(LoadOrderHandler&& other);
+
   void Init(const GameType& game,
             const std::filesystem::path& gamePath,
             const std::filesystem::path& gameLocalAppData = "");