[llvm] r278603 - Revert "[ADT] Add a reserve method to DenseSet as well as an insert() for R-value"

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Sat Aug 13 13:14:40 PDT 2016


Author: mehdi_amini
Date: Sat Aug 13 15:14:39 2016
New Revision: 278603

URL: http://llvm.org/viewvc/llvm-project?rev=278603&view=rev
Log:
Revert "[ADT] Add a reserve method to DenseSet as well as an insert() for R-value"

This reverts commit r278600. The unittest does not pass on MSVC, there is
an extra move. Investigating how to make it more robust.

Modified:
    llvm/trunk/include/llvm/ADT/DenseSet.h
    llvm/trunk/unittests/ADT/DenseSetTest.cpp

Modified: llvm/trunk/include/llvm/ADT/DenseSet.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/DenseSet.h?rev=278603&r1=278602&r2=278603&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/DenseSet.h (original)
+++ llvm/trunk/include/llvm/ADT/DenseSet.h Sat Aug 13 15:14:39 2016
@@ -58,10 +58,6 @@ public:
   /// the Size of the set.
   void resize(size_t Size) { TheMap.resize(Size); }
 
-  /// Grow the DenseSet so that it can contain at least \p NumEntries items
-  /// before resizing again.
-  void reserve(size_t Size) { TheMap.reserve(Size); }
-
   void clear() {
     TheMap.clear();
   }
@@ -158,11 +154,6 @@ public:
     return TheMap.insert(std::make_pair(V, Empty));
   }
 
-  std::pair<iterator, bool> insert(ValueT &&V) {
-    detail::DenseSetEmpty Empty;
-    return TheMap.insert(std::make_pair(std::move(V), Empty));
-  }
-
   /// Alternative version of insert that uses a different (and possibly less
   /// expensive) key type.
   template <typename LookupKeyT>

Modified: llvm/trunk/unittests/ADT/DenseSetTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/DenseSetTest.cpp?rev=278603&r1=278602&r2=278603&view=diff
==============================================================================
--- llvm/trunk/unittests/ADT/DenseSetTest.cpp (original)
+++ llvm/trunk/unittests/ADT/DenseSetTest.cpp Sat Aug 13 15:14:39 2016
@@ -65,67 +65,4 @@ TEST(DenseSetCustomTest, FindAsTest) {
   EXPECT_TRUE(set.find_as("d") == set.end());
 }
 
-// Simple class that counts how many moves and copy happens when growing a map
-struct CountCopyAndMove {
-  static int Move;
-  static int Copy;
-  int Value;
-  CountCopyAndMove(int Value) : Value(Value) {}
-
-  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
-
-namespace llvm {
-// Specialization required to insert a CountCopyAndMove into a DenseSet.
-template <> struct DenseMapInfo<CountCopyAndMove> {
-  static inline CountCopyAndMove getEmptyKey() { return CountCopyAndMove(-1); };
-  static inline CountCopyAndMove getTombstoneKey() {
-    return CountCopyAndMove(-2);
-  };
-  static unsigned getHashValue(const CountCopyAndMove &Val) {
-    return Val.Value;
-  }
-  static bool isEqual(const CountCopyAndMove &LHS,
-                      const CountCopyAndMove &RHS) {
-    return LHS.Value == RHS.Value;
-  }
-};
-}
-
-namespace {
-// Make sure reserve actually gives us enough buckets to insert N items
-// without increasing allocation size.
-TEST(DenseSetCustomTest, ReserveTest) {
-  // Test a few different size, 48 is *not* a random choice: we need a value
-  // that is 2/3 of a power of two to stress the grow() condition, and the power
-  // of two has to be at least 64 because of minimum size allocation in the
-  // DenseMa. 66 is a value just above the 64 default init.
-  for (auto Size : {1, 2, 48, 66}) {
-    DenseSet<CountCopyAndMove> Set;
-    Set.reserve(Size);
-    unsigned MemorySize = Set.getMemorySize();
-    CountCopyAndMove::Copy = 0;
-    CountCopyAndMove::Move = 0;
-    for (int i = 0; i < Size; ++i)
-      Set.insert(CountCopyAndMove(i));
-    // Check that we didn't grow
-    EXPECT_EQ(MemorySize, Set.getMemorySize());
-    // Check that move was called the expected number of times
-    EXPECT_EQ(Size, CountCopyAndMove::Move);
-    // Check that no copy occured
-    EXPECT_EQ(0, CountCopyAndMove::Copy);
-  }
-}
 }




More information about the llvm-commits mailing list