[llvm] r264475 - StringMap/DenseMap unittests: use piecewise_construct and ensure no copy occurs.

[Mehdi Amini via llvm-commits]

Author: mehdi_amini
Date: Fri Mar 25 18:25:06 2016
New Revision: 264475

URL: http://llvm.org/viewvc/llvm-project?rev=264475&view=rev
Log:
StringMap/DenseMap unittests: use piecewise_construct and ensure no copy occurs.

This makes us no longer relying on move-construction elision by the compiler.
Suggested by D. Blaikie.

From: Mehdi Amini <mehdi.amini at apple.com>

Modified:
    llvm/trunk/unittests/ADT/DenseMapTest.cpp
    llvm/trunk/unittests/ADT/StringMapTest.cpp

Modified: llvm/trunk/unittests/ADT/DenseMapTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/DenseMapTest.cpp?rev=264475&r1=264474&r2=264475&view=diff
==============================================================================
--- llvm/trunk/unittests/ADT/DenseMapTest.cpp (original)
+++ llvm/trunk/unittests/ADT/DenseMapTest.cpp Fri Mar 25 18:25:06 2016
@@ -377,18 +377,21 @@ TEST(DenseMapCustomTest, DefaultMinReser
   CountCopyAndMove::Copy = 0;
   CountCopyAndMove::Move = 0;
   for (int i = 0; i < ExpectedMaxInitialEntries; ++i)
-    Map.insert(std::make_pair(i, CountCopyAndMove()));
+    Map.insert(std::pair<int, CountCopyAndMove>(std::piecewise_construct,
+                                                std::forward_as_tuple(i),
+                                                std::forward_as_tuple()));
   // Check that we didn't grow
   EXPECT_EQ(MemorySize, Map.getMemorySize());
   // Check that move was called the expected number of times
-  EXPECT_EQ(ExpectedMaxInitialEntries * 2, CountCopyAndMove::Move);
+  EXPECT_EQ(ExpectedMaxInitialEntries, CountCopyAndMove::Move);
   // Check that no copy occured
   EXPECT_EQ(0, CountCopyAndMove::Copy);
 
   // Adding one extra element should grow the map
-  CountCopyAndMove::Copy = 0;
-  CountCopyAndMove::Move = 0;
-  Map.insert(std::make_pair(ExpectedMaxInitialEntries, CountCopyAndMove()));
+  Map.insert(std::pair<int, CountCopyAndMove>(
+      std::piecewise_construct,
+      std::forward_as_tuple(ExpectedMaxInitialEntries),
+      std::forward_as_tuple()));
   // Check that we grew
   EXPECT_NE(MemorySize, Map.getMemorySize());
   // Check that move was called the expected number of times
@@ -412,12 +415,13 @@ TEST(DenseMapCustomTest, InitialSizeTest
     CountCopyAndMove::Copy = 0;
     CountCopyAndMove::Move = 0;
     for (int i = 0; i < Size; ++i)
-      Map.insert(std::make_pair(i, CountCopyAndMove()));
+      Map.insert(std::pair<int, CountCopyAndMove>(std::piecewise_construct,
+                                                  std::forward_as_tuple(i),
+                                                  std::forward_as_tuple()));
     // Check that we didn't grow
     EXPECT_EQ(MemorySize, Map.getMemorySize());
     // Check that move was called the expected number of times
-    //  This relies on move-construction elision, and cannot be reliably tested.
-    //   EXPECT_EQ(Size * 2, CountCopyAndMove::Move);
+    EXPECT_EQ(Size, CountCopyAndMove::Move);
     // Check that no copy occured
     EXPECT_EQ(0, CountCopyAndMove::Copy);
   }
@@ -455,12 +459,13 @@ TEST(DenseMapCustomTest, ReserveTest) {
     CountCopyAndMove::Copy = 0;
     CountCopyAndMove::Move = 0;
     for (int i = 0; i < Size; ++i)
-      Map.insert(std::make_pair(i, CountCopyAndMove()));
+      Map.insert(std::pair<int, CountCopyAndMove>(std::piecewise_construct,
+                                                  std::forward_as_tuple(i),
+                                                  std::forward_as_tuple()));
     // Check that we didn't grow
     EXPECT_EQ(MemorySize, Map.getMemorySize());
     // Check that move was called the expected number of times
-    //  This relies on move-construction elision, and cannot be reliably tested.
-    //   EXPECT_EQ(Size * 2, CountCopyAndMove::Move);
+    EXPECT_EQ(Size, CountCopyAndMove::Move);
     // Check that no copy occured
     EXPECT_EQ(0, CountCopyAndMove::Copy);
   }

Modified: llvm/trunk/unittests/ADT/StringMapTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/StringMapTest.cpp?rev=264475&r1=264474&r2=264475&view=diff
==============================================================================
--- llvm/trunk/unittests/ADT/StringMapTest.cpp (original)
+++ llvm/trunk/unittests/ADT/StringMapTest.cpp Fri Mar 25 18:25:06 2016
@@ -391,21 +391,16 @@ TEST(StringMapCustomTest, InitialSizeTes
   for (auto Size : {1, 32, 67}) {
     StringMap<CountCtorCopyAndMove> Map(Size);
     auto NumBuckets = Map.getNumBuckets();
-
-    // Prepare the elts in a vector. We do this as a pre-step to shield us
-    // against the internals of std::pair which can introduce spurious move/copy
-    std::vector<std::pair<std::string, CountCtorCopyAndMove>> Elts;
-    for (int i = 0; i < Size; ++i)
-      Elts.emplace_back(Twine(i).str(), CountCtorCopyAndMove());
-
     CountCtorCopyAndMove::Move = 0;
     CountCtorCopyAndMove::Copy = 0;
     for (int i = 0; i < Size; ++i)
-      Map.insert(Elts[i]);
-    // After the inital copy, the map will move the Elts in the Entry.
-    EXPECT_EQ((unsigned)Size, CountCtorCopyAndMove::Move);
+      Map.insert(std::pair<std::string, CountCtorCopyAndMove>(
+          std::piecewise_construct, std::forward_as_tuple(Twine(i).str()),
+          std::forward_as_tuple(i)));
+    // After the inital move, the map will move the Elts in the Entry.
+    EXPECT_EQ((unsigned)Size * 2, CountCtorCopyAndMove::Move);
     // We copy once the pair from the Elts vector
-    EXPECT_EQ((unsigned)Size, CountCtorCopyAndMove::Copy);
+    EXPECT_EQ(0u, CountCtorCopyAndMove::Copy);
     // Check that the map didn't grow
     EXPECT_EQ(Map.getNumBuckets(), NumBuckets);
   }