[llvm] 6871657 - [ADT] refactor MoveOnly type in ADT unittest (#94421)

via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 4 21:58:49 PDT 2024


Author: c8ef
Date: 2024-06-04T21:58:45-07:00
New Revision: 68716573b627b86af50356edf63ba86856edd859

URL: https://github.com/llvm/llvm-project/commit/68716573b627b86af50356edf63ba86856edd859
DIFF: https://github.com/llvm/llvm-project/commit/68716573b627b86af50356edf63ba86856edd859.diff

LOG: [ADT] refactor MoveOnly type in ADT unittest (#94421)

context:
https://github.com/llvm/llvm-project/pull/94151#pullrequestreview-2097098530

Added: 
    llvm/unittests/ADT/CountCopyAndMove.cpp
    llvm/unittests/ADT/CountCopyAndMove.h

Modified: 
    llvm/unittests/ADT/CMakeLists.txt
    llvm/unittests/ADT/DenseMapTest.cpp
    llvm/unittests/ADT/STLForwardCompatTest.cpp

Removed: 
    llvm/unittests/ADT/MoveOnly.cpp
    llvm/unittests/ADT/MoveOnly.h


################################################################################
diff  --git a/llvm/unittests/ADT/CMakeLists.txt b/llvm/unittests/ADT/CMakeLists.txt
index 17c5c9d1c59ce..85c140e63fecd 100644
--- a/llvm/unittests/ADT/CMakeLists.txt
+++ b/llvm/unittests/ADT/CMakeLists.txt
@@ -18,6 +18,7 @@ add_llvm_unittest(ADTTests
   CoalescingBitVectorTest.cpp
   CombinationGeneratorTest.cpp
   ConcurrentHashtableTest.cpp
+  CountCopyAndMove.cpp
   DAGDeltaAlgorithmTest.cpp
   DeltaAlgorithmTest.cpp
   DenseMapTest.cpp
@@ -52,7 +53,6 @@ add_llvm_unittest(ADTTests
   LazyAtomicPointerTest.cpp
   MappedIteratorTest.cpp
   MapVectorTest.cpp
-  MoveOnly.cpp
   PackedVectorTest.cpp
   PagedVectorTest.cpp
   PointerEmbeddedIntTest.cpp

diff  --git a/llvm/unittests/ADT/CountCopyAndMove.cpp b/llvm/unittests/ADT/CountCopyAndMove.cpp
new file mode 100644
index 0000000000000..fe1e2f4a5b89f
--- /dev/null
+++ b/llvm/unittests/ADT/CountCopyAndMove.cpp
@@ -0,0 +1,17 @@
+//===- llvm/unittest/ADT/CountCopyAndMove.cpp - Optional unit tests -------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "CountCopyAndMove.h"
+
+using namespace llvm;
+
+int CountCopyAndMove::CopyConstructions = 0;
+int CountCopyAndMove::CopyAssignments = 0;
+int CountCopyAndMove::MoveConstructions = 0;
+int CountCopyAndMove::MoveAssignments = 0;
+int CountCopyAndMove::Destructions = 0;

diff  --git a/llvm/unittests/ADT/CountCopyAndMove.h b/llvm/unittests/ADT/CountCopyAndMove.h
new file mode 100644
index 0000000000000..126054427b81a
--- /dev/null
+++ b/llvm/unittests/ADT/CountCopyAndMove.h
@@ -0,0 +1,57 @@
+//===- llvm/unittest/ADT/CountCopyAndMove.h - Optional unit tests ---------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_UNITTESTS_ADT_COUNTCOPYANDMOVE_H
+#define LLVM_UNITTESTS_ADT_COUNTCOPYANDMOVE_H
+
+namespace llvm {
+
+struct CountCopyAndMove {
+  static int CopyConstructions;
+  static int CopyAssignments;
+  static int MoveConstructions;
+  static int MoveAssignments;
+  static int Destructions;
+  int val;
+
+  CountCopyAndMove() = default;
+  explicit CountCopyAndMove(int val) : val(val) {}
+  CountCopyAndMove(const CountCopyAndMove &other) : val(other.val) {
+    ++CopyConstructions;
+  }
+  CountCopyAndMove &operator=(const CountCopyAndMove &other) {
+    val = other.val;
+    ++CopyAssignments;
+    return *this;
+  }
+  CountCopyAndMove(CountCopyAndMove &&other) : val(other.val) {
+    ++MoveConstructions;
+  }
+  CountCopyAndMove &operator=(CountCopyAndMove &&other) {
+    val = other.val;
+    ++MoveAssignments;
+    return *this;
+  }
+  ~CountCopyAndMove() { ++Destructions; }
+
+  static void ResetCounts() {
+    CopyConstructions = 0;
+    CopyAssignments = 0;
+    MoveConstructions = 0;
+    MoveAssignments = 0;
+    Destructions = 0;
+  }
+
+  static int TotalCopies() { return CopyConstructions + CopyAssignments; }
+
+  static int TotalMoves() { return MoveConstructions + MoveAssignments; }
+};
+
+} // end namespace llvm
+
+#endif // LLVM_UNITTESTS_ADT_COUNTCOPYANDMOVE_H

diff  --git a/llvm/unittests/ADT/DenseMapTest.cpp b/llvm/unittests/ADT/DenseMapTest.cpp
index cc3244528f27e..09f9a57647709 100644
--- a/llvm/unittests/ADT/DenseMapTest.cpp
+++ b/llvm/unittests/ADT/DenseMapTest.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/DenseMap.h"
+#include "CountCopyAndMove.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/ADT/DenseMapInfoVariant.h"
 #include "llvm/ADT/StringRef.h"
@@ -346,29 +347,6 @@ TYPED_TEST(DenseMapTest, ConstIteratorTest) {
   EXPECT_TRUE(cit == cit2);
 }
 
-namespace {
-// Simple class that counts how many moves and copy happens when growing a map
-struct CountCopyAndMove {
-  static int Move;
-  static int Copy;
-  CountCopyAndMove() {}
-
-  CountCopyAndMove(const CountCopyAndMove &) { Copy++; }
-  CountCopyAndMove &operator=(const CountCopyAndMove &) {
-    Copy++;
-    return *this;
-  }
-  CountCopyAndMove(CountCopyAndMove &&) { Move++; }
-  CountCopyAndMove &operator=(const CountCopyAndMove &&) {
-    Move++;
-    return *this;
-  }
-};
-int CountCopyAndMove::Copy = 0;
-int CountCopyAndMove::Move = 0;
-
-} // anonymous namespace
-
 // Test initializer list construction.
 TEST(DenseMapCustomTest, InitializerList) {
   DenseMap<int, int> M({{0, 0}, {0, 1}, {1, 2}});
@@ -401,8 +379,8 @@ TEST(DenseMapCustomTest, DefaultMinReservedSizeTest) {
   // Will allocate 64 buckets
   Map.reserve(1);
   unsigned MemorySize = Map.getMemorySize();
-  CountCopyAndMove::Copy = 0;
-  CountCopyAndMove::Move = 0;
+  CountCopyAndMove::ResetCounts();
+
   for (int i = 0; i < ExpectedMaxInitialEntries; ++i)
     Map.insert(std::pair<int, CountCopyAndMove>(std::piecewise_construct,
                                                 std::forward_as_tuple(i),
@@ -410,9 +388,9 @@ TEST(DenseMapCustomTest, DefaultMinReservedSizeTest) {
   // Check that we didn't grow
   EXPECT_EQ(MemorySize, Map.getMemorySize());
   // Check that move was called the expected number of times
-  EXPECT_EQ(ExpectedMaxInitialEntries, CountCopyAndMove::Move);
+  EXPECT_EQ(ExpectedMaxInitialEntries, CountCopyAndMove::TotalMoves());
   // Check that no copy occurred
-  EXPECT_EQ(0, CountCopyAndMove::Copy);
+  EXPECT_EQ(0, CountCopyAndMove::TotalCopies());
 
   // Adding one extra element should grow the map
   Map.insert(std::pair<int, CountCopyAndMove>(
@@ -425,7 +403,7 @@ TEST(DenseMapCustomTest, DefaultMinReservedSizeTest) {
   //  This relies on move-construction elision, and cannot be reliably tested.
   //   EXPECT_EQ(ExpectedMaxInitialEntries + 2, CountCopyAndMove::Move);
   // Check that no copy occurred
-  EXPECT_EQ(0, CountCopyAndMove::Copy);
+  EXPECT_EQ(0, CountCopyAndMove::TotalCopies());
 }
 
 // Make sure creating the map with an initial size of N actually gives us enough
@@ -439,8 +417,8 @@ TEST(DenseMapCustomTest, InitialSizeTest) {
   for (auto Size : {1, 2, 48, 66}) {
     DenseMap<int, CountCopyAndMove> Map(Size);
     unsigned MemorySize = Map.getMemorySize();
-    CountCopyAndMove::Copy = 0;
-    CountCopyAndMove::Move = 0;
+    CountCopyAndMove::ResetCounts();
+
     for (int i = 0; i < Size; ++i)
       Map.insert(std::pair<int, CountCopyAndMove>(std::piecewise_construct,
                                                   std::forward_as_tuple(i),
@@ -448,9 +426,9 @@ TEST(DenseMapCustomTest, InitialSizeTest) {
     // Check that we didn't grow
     EXPECT_EQ(MemorySize, Map.getMemorySize());
     // Check that move was called the expected number of times
-    EXPECT_EQ(Size, CountCopyAndMove::Move);
+    EXPECT_EQ(Size, CountCopyAndMove::TotalMoves());
     // Check that no copy occurred
-    EXPECT_EQ(0, CountCopyAndMove::Copy);
+    EXPECT_EQ(0, CountCopyAndMove::TotalCopies());
   }
 }
 
@@ -461,15 +439,14 @@ TEST(DenseMapCustomTest, InitFromIterator) {
   const int Count = 65;
   Values.reserve(Count);
   for (int i = 0; i < Count; i++)
-    Values.emplace_back(i, CountCopyAndMove());
+    Values.emplace_back(i, CountCopyAndMove(i));
 
-  CountCopyAndMove::Move = 0;
-  CountCopyAndMove::Copy = 0;
+  CountCopyAndMove::ResetCounts();
   DenseMap<int, CountCopyAndMove> Map(Values.begin(), Values.end());
   // Check that no move occurred
-  EXPECT_EQ(0, CountCopyAndMove::Move);
+  EXPECT_EQ(0, CountCopyAndMove::TotalMoves());
   // Check that copy was called the expected number of times
-  EXPECT_EQ(Count, CountCopyAndMove::Copy);
+  EXPECT_EQ(Count, CountCopyAndMove::TotalCopies());
 }
 
 // Make sure reserve actually gives us enough buckets to insert N items
@@ -484,8 +461,7 @@ TEST(DenseMapCustomTest, ReserveTest) {
     DenseMap<int, CountCopyAndMove> Map;
     Map.reserve(Size);
     unsigned MemorySize = Map.getMemorySize();
-    CountCopyAndMove::Copy = 0;
-    CountCopyAndMove::Move = 0;
+    CountCopyAndMove::ResetCounts();
     for (int i = 0; i < Size; ++i)
       Map.insert(std::pair<int, CountCopyAndMove>(std::piecewise_construct,
                                                   std::forward_as_tuple(i),
@@ -493,9 +469,9 @@ TEST(DenseMapCustomTest, ReserveTest) {
     // Check that we didn't grow
     EXPECT_EQ(MemorySize, Map.getMemorySize());
     // Check that move was called the expected number of times
-    EXPECT_EQ(Size, CountCopyAndMove::Move);
+    EXPECT_EQ(Size, CountCopyAndMove::TotalMoves());
     // Check that no copy occurred
-    EXPECT_EQ(0, CountCopyAndMove::Copy);
+    EXPECT_EQ(0, CountCopyAndMove::TotalCopies());
   }
 }
 

diff  --git a/llvm/unittests/ADT/MoveOnly.cpp b/llvm/unittests/ADT/MoveOnly.cpp
deleted file mode 100644
index 541903bbe9467..0000000000000
--- a/llvm/unittests/ADT/MoveOnly.cpp
+++ /dev/null
@@ -1,15 +0,0 @@
-//===- llvm/unittest/ADT/MoveOnly.cpp - Optional unit tests ---------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "MoveOnly.h"
-
-using namespace llvm;
-
-unsigned MoveOnly::MoveConstructions = 0;
-unsigned MoveOnly::Destructions = 0;
-unsigned MoveOnly::MoveAssignments = 0;

diff  --git a/llvm/unittests/ADT/MoveOnly.h b/llvm/unittests/ADT/MoveOnly.h
deleted file mode 100644
index ad64deae771ba..0000000000000
--- a/llvm/unittests/ADT/MoveOnly.h
+++ /dev/null
@@ -1,42 +0,0 @@
-//===- llvm/unittest/ADT/MoveOnly.h - Optional unit tests -----------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_UNITTESTS_ADT_MOVEONLY_H
-#define LLVM_UNITTESTS_ADT_MOVEONLY_H
-
-namespace llvm {
-
-struct MoveOnly {
-  static unsigned MoveConstructions;
-  static unsigned Destructions;
-  static unsigned MoveAssignments;
-  int val;
-  explicit MoveOnly(int val) : val(val) {
-  }
-  MoveOnly(MoveOnly&& other) {
-    val = other.val;
-    ++MoveConstructions;
-  }
-  MoveOnly &operator=(MoveOnly&& other) {
-    val = other.val;
-    ++MoveAssignments;
-    return *this;
-  }
-  ~MoveOnly() {
-    ++Destructions;
-  }
-  static void ResetCounts() {
-    MoveConstructions = 0;
-    Destructions = 0;
-    MoveAssignments = 0;
-  }
-};
-
-}  // end namespace llvm
-
-#endif // LLVM_UNITTESTS_ADT_MOVEONLY_H

diff  --git a/llvm/unittests/ADT/STLForwardCompatTest.cpp b/llvm/unittests/ADT/STLForwardCompatTest.cpp
index b0c95d09ba2c6..b856386aa3e45 100644
--- a/llvm/unittests/ADT/STLForwardCompatTest.cpp
+++ b/llvm/unittests/ADT/STLForwardCompatTest.cpp
@@ -7,7 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/ADT/STLForwardCompat.h"
-#include "MoveOnly.h"
+#include "CountCopyAndMove.h"
 #include "gtest/gtest.h"
 
 namespace {
@@ -58,27 +58,29 @@ TEST(TransformTest, TransformStd) {
 }
 
 TEST(TransformTest, MoveTransformStd) {
-  using llvm::MoveOnly;
+  using llvm::CountCopyAndMove;
 
-  std::optional<MoveOnly> A;
+  std::optional<CountCopyAndMove> A;
 
-  MoveOnly::ResetCounts();
+  CountCopyAndMove::ResetCounts();
   std::optional<int> B = llvm::transformOptional(
-      std::move(A), [&](const MoveOnly &M) { return M.val + 2; });
+      std::move(A), [&](const CountCopyAndMove &M) { return M.val + 2; });
   EXPECT_FALSE(B.has_value());
-  EXPECT_EQ(0u, MoveOnly::MoveConstructions);
-  EXPECT_EQ(0u, MoveOnly::MoveAssignments);
-  EXPECT_EQ(0u, MoveOnly::Destructions);
+  EXPECT_EQ(0, CountCopyAndMove::TotalCopies());
+  EXPECT_EQ(0, CountCopyAndMove::MoveConstructions);
+  EXPECT_EQ(0, CountCopyAndMove::MoveAssignments);
+  EXPECT_EQ(0, CountCopyAndMove::Destructions);
 
-  A = MoveOnly(5);
-  MoveOnly::ResetCounts();
+  A = CountCopyAndMove(5);
+  CountCopyAndMove::ResetCounts();
   std::optional<int> C = llvm::transformOptional(
-      std::move(A), [&](const MoveOnly &M) { return M.val + 2; });
+      std::move(A), [&](const CountCopyAndMove &M) { return M.val + 2; });
   EXPECT_TRUE(C.has_value());
   EXPECT_EQ(7, *C);
-  EXPECT_EQ(0u, MoveOnly::MoveConstructions);
-  EXPECT_EQ(0u, MoveOnly::MoveAssignments);
-  EXPECT_EQ(0u, MoveOnly::Destructions);
+  EXPECT_EQ(0, CountCopyAndMove::TotalCopies());
+  EXPECT_EQ(0, CountCopyAndMove::MoveConstructions);
+  EXPECT_EQ(0, CountCopyAndMove::MoveAssignments);
+  EXPECT_EQ(0, CountCopyAndMove::Destructions);
 }
 
 TEST(TransformTest, TransformLlvm) {
@@ -96,27 +98,29 @@ TEST(TransformTest, TransformLlvm) {
 }
 
 TEST(TransformTest, MoveTransformLlvm) {
-  using llvm::MoveOnly;
+  using llvm::CountCopyAndMove;
 
-  std::optional<MoveOnly> A;
+  std::optional<CountCopyAndMove> A;
 
-  MoveOnly::ResetCounts();
+  CountCopyAndMove::ResetCounts();
   std::optional<int> B = llvm::transformOptional(
-      std::move(A), [&](const MoveOnly &M) { return M.val + 2; });
+      std::move(A), [&](const CountCopyAndMove &M) { return M.val + 2; });
   EXPECT_FALSE(B.has_value());
-  EXPECT_EQ(0u, MoveOnly::MoveConstructions);
-  EXPECT_EQ(0u, MoveOnly::MoveAssignments);
-  EXPECT_EQ(0u, MoveOnly::Destructions);
+  EXPECT_EQ(0, CountCopyAndMove::TotalCopies());
+  EXPECT_EQ(0, CountCopyAndMove::MoveConstructions);
+  EXPECT_EQ(0, CountCopyAndMove::MoveAssignments);
+  EXPECT_EQ(0, CountCopyAndMove::Destructions);
 
-  A = MoveOnly(5);
-  MoveOnly::ResetCounts();
+  A = CountCopyAndMove(5);
+  CountCopyAndMove::ResetCounts();
   std::optional<int> C = llvm::transformOptional(
-      std::move(A), [&](const MoveOnly &M) { return M.val + 2; });
+      std::move(A), [&](const CountCopyAndMove &M) { return M.val + 2; });
   EXPECT_TRUE(C.has_value());
   EXPECT_EQ(7, *C);
-  EXPECT_EQ(0u, MoveOnly::MoveConstructions);
-  EXPECT_EQ(0u, MoveOnly::MoveAssignments);
-  EXPECT_EQ(0u, MoveOnly::Destructions);
+  EXPECT_EQ(0, CountCopyAndMove::TotalCopies());
+  EXPECT_EQ(0, CountCopyAndMove::MoveConstructions);
+  EXPECT_EQ(0, CountCopyAndMove::MoveAssignments);
+  EXPECT_EQ(0, CountCopyAndMove::Destructions);
 }
 
 TEST(TransformTest, ToUnderlying) {


        


More information about the llvm-commits mailing list