Merge topic 'cmFileLockPool-remove-dynamic-memory-usage'

8dd284bf19 cmFileLockPool: enhance items management

Acked-by: Kitware Robot <[email protected]>
Merge-request: !4041

Source/cmFileLock.cxx

@@ -3,11 +3,23 @@
 #include "cmFileLock.h"
 
 #include <cassert>
+#include <utility>
 
 #include "cmFileLockResult.h"
 
 // Common implementation
 
+cmFileLock::cmFileLock(cmFileLock&& other) noexcept
+{
+  this->File = other.File;
+#if defined(_WIN32)
+  other.File = INVALID_HANDLE_VALUE;
+#else
+  other.File = -1;
+#endif
+  this->Filename = std::move(other.Filename);
+}
+
 cmFileLock::~cmFileLock()
 {
   if (!this->Filename.empty()) {
@@ -17,6 +29,19 @@ cmFileLock::~cmFileLock()
   }
 }
 
+cmFileLock& cmFileLock::operator=(cmFileLock&& other) noexcept
+{
+  this->File = other.File;
+#if defined(_WIN32)
+  other.File = INVALID_HANDLE_VALUE;
+#else
+  other.File = -1;
+#endif
+  this->Filename = std::move(other.Filename);
+
+  return *this;
+}
+
 cmFileLockResult cmFileLock::Lock(const std::string& filename,
                                   unsigned long timeout)
 {

Source/cmFileLock.h

@@ -26,7 +26,9 @@ public:
   ~cmFileLock();
 
   cmFileLock(cmFileLock const&) = delete;
+  cmFileLock(cmFileLock&&) noexcept;
   cmFileLock& operator=(cmFileLock const&) = delete;
+  cmFileLock& operator=(cmFileLock&&) noexcept;
 
   /**
    * @brief Lock the file.

Source/cmFileLockPool.cxx

@@ -3,40 +3,34 @@
 #include "cmFileLockPool.h"
 
 #include <cassert>
+#include <utility>
 
-#include "cmAlgorithms.h"
 #include "cmFileLock.h"
 #include "cmFileLockResult.h"
 
 cmFileLockPool::cmFileLockPool() = default;
 
-cmFileLockPool::~cmFileLockPool()
-{
-  cmDeleteAll(this->FunctionScopes);
-  cmDeleteAll(this->FileScopes);
-}
+cmFileLockPool::~cmFileLockPool() = default;
 
 void cmFileLockPool::PushFunctionScope()
 {
-  this->FunctionScopes.push_back(new ScopePool());
+  this->FunctionScopes.push_back(ScopePool());
 }
 
 void cmFileLockPool::PopFunctionScope()
 {
   assert(!this->FunctionScopes.empty());
-  delete this->FunctionScopes.back();
   this->FunctionScopes.pop_back();
 }
 
 void cmFileLockPool::PushFileScope()
 {
-  this->FileScopes.push_back(new ScopePool());
+  this->FileScopes.push_back(ScopePool());
 }
 
 void cmFileLockPool::PopFileScope()
 {
   assert(!this->FileScopes.empty());
-  delete this->FileScopes.back();
   this->FileScopes.pop_back();
 }
 
@@ -49,7 +43,7 @@ cmFileLockResult cmFileLockPool::LockFunctionScope(const std::string& filename,
   if (this->FunctionScopes.empty()) {
     return cmFileLockResult::MakeNoFunction();
   }
-  return this->FunctionScopes.back()->Lock(filename, timeoutSec);
+  return this->FunctionScopes.back().Lock(filename, timeoutSec);
 }
 
 cmFileLockResult cmFileLockPool::LockFileScope(const std::string& filename,
@@ -59,7 +53,7 @@ cmFileLockResult cmFileLockPool::LockFileScope(const std::string& filename,
     return cmFileLockResult::MakeAlreadyLocked();
   }
   assert(!this->FileScopes.empty());
-  return this->FileScopes.back()->Lock(filename, timeoutSec);
+  return this->FileScopes.back().Lock(filename, timeoutSec);
 }
 
 cmFileLockResult cmFileLockPool::LockProcessScope(const std::string& filename,
@@ -74,14 +68,14 @@ cmFileLockResult cmFileLockPool::LockProcessScope(const std::string& filename,
 cmFileLockResult cmFileLockPool::Release(const std::string& filename)
 {
   for (auto& funcScope : this->FunctionScopes) {
-    const cmFileLockResult result = funcScope->Release(filename);
+    const cmFileLockResult result = funcScope.Release(filename);
     if (!result.IsOk()) {
       return result;
     }
   }
 
   for (auto& fileScope : this->FileScopes) {
-    const cmFileLockResult result = fileScope->Release(filename);
+    const cmFileLockResult result = fileScope.Release(filename);
     if (!result.IsOk()) {
       return result;
     }
@@ -93,14 +87,14 @@ cmFileLockResult cmFileLockPool::Release(const std::string& filename)
 bool cmFileLockPool::IsAlreadyLocked(const std::string& filename) const
 {
   for (auto const& funcScope : this->FunctionScopes) {
-    const bool result = funcScope->IsAlreadyLocked(filename);
+    const bool result = funcScope.IsAlreadyLocked(filename);
     if (result) {
       return true;
     }
   }
 
   for (auto const& fileScope : this->FileScopes) {
-    const bool result = fileScope->IsAlreadyLocked(filename);
+    const bool result = fileScope.IsAlreadyLocked(filename);
     if (result) {
       return true;
     }
@@ -111,21 +105,29 @@ bool cmFileLockPool::IsAlreadyLocked(const std::string& filename) const
 
 cmFileLockPool::ScopePool::ScopePool() = default;
 
-cmFileLockPool::ScopePool::~ScopePool()
+cmFileLockPool::ScopePool::~ScopePool() = default;
+
+cmFileLockPool::ScopePool::ScopePool(ScopePool&&) noexcept = default;
+
+cmFileLockPool::ScopePool& cmFileLockPool::ScopePool::operator=(
+  ScopePool&& other) noexcept
 {
-  cmDeleteAll(this->Locks);
+  if (this != &other) {
+    this->Locks = std::move(other.Locks);
+  }
+
+  return *this;
 }
 
 cmFileLockResult cmFileLockPool::ScopePool::Lock(const std::string& filename,
                                                  unsigned long timeoutSec)
 {
-  cmFileLock* lock = new cmFileLock();
-  const cmFileLockResult result = lock->Lock(filename, timeoutSec);
+  cmFileLock lock;
+  const cmFileLockResult result = lock.Lock(filename, timeoutSec);
   if (result.IsOk()) {
-    this->Locks.push_back(lock);
+    this->Locks.push_back(std::move(lock));
     return cmFileLockResult::MakeOk();
   }
-  delete lock;
   return result;
 }
 
@@ -133,8 +135,8 @@ cmFileLockResult cmFileLockPool::ScopePool::Release(
   const std::string& filename)
 {
   for (auto& lock : this->Locks) {
-    if (lock->IsLocked(filename)) {
-      return lock->Release();
+    if (lock.IsLocked(filename)) {
+      return lock.Release();
     }
   }
   return cmFileLockResult::MakeOk();
@@ -144,7 +146,7 @@ bool cmFileLockPool::ScopePool::IsAlreadyLocked(
   const std::string& filename) const
 {
   for (auto const& lock : this->Locks) {
-    if (lock->IsLocked(filename)) {
+    if (lock.IsLocked(filename)) {
       return true;
     }
   }

Source/cmFileLockPool.h

@@ -8,7 +8,8 @@
 #include <string>
 #include <vector>
 
-class cmFileLock;
+#include "cmFileLock.h"
+
 class cmFileLockResult;
 
 class cmFileLockPool
@@ -64,7 +65,9 @@ private:
     ~ScopePool();
 
     ScopePool(ScopePool const&) = delete;
+    ScopePool(ScopePool&&) noexcept;
     ScopePool& operator=(ScopePool const&) = delete;
+    ScopePool& operator=(ScopePool&&) noexcept;
 
     cmFileLockResult Lock(const std::string& filename,
                           unsigned long timeoutSec);
@@ -72,17 +75,12 @@ private:
     bool IsAlreadyLocked(const std::string& filename) const;
 
   private:
-    using List = std::vector<cmFileLock*>;
-    using It = List::iterator;
-    using CIt = List::const_iterator;
+    using List = std::vector<cmFileLock>;
 
     List Locks;
   };
 
-  using List = std::vector<ScopePool*>;
-
-  using It = List::iterator;
-  using CIt = List::const_iterator;
+  using List = std::vector<ScopePool>;
 
   List FunctionScopes;
   List FileScopes;

Utilities/IWYU/mapping.imp

@@ -47,6 +47,8 @@
   # HACK: iwyu suggests <ext/alloc_traits.h> and <memory> each time vector[] is used.
   # https://github.com/include-what-you-use/include-what-you-use/issues/166
   { include: [ "<ext/alloc_traits.h>", private, "<vector>", public ] },
+  { symbol: [ "std::allocator_traits<std::allocator<cmFileLock> >::value_type", private, "<vector>", public ] },
+  { symbol: [ "std::allocator_traits<std::allocator<cmFileLockPool::ScopePool> >::value_type", private, "<vector>", public ] },
   { symbol: [ "std::allocator_traits<std::allocator<cmComputeComponentGraph::TarjanEntry> >::value_type", private, "<vector>", public ] },
   { symbol: [ "std::allocator_traits<std::allocator<cmFortranFile> >::value_type", private, "<vector>", public ] },
   { symbol: [ "std::allocator_traits<std::allocator<cmGraphEdgeList> >::value_type", private, "<vector>", public ] },