Merge pull request swiftlang#15661 from atrick/blotvec

Fix usability issues with BlotVector.

Author: gottesmm

include/swift/Basic/BlotMapVector.h

@@ -51,6 +51,9 @@ class BlotMapVector {
     iterator_range<iterator> getItems() {
       return swift::make_range(begin(), end());
     }
+    iterator_range<const_iterator> getItems() const {
+      return swift::make_range(begin(), end());
+    }
 
     ValueT &operator[](const KeyT &Arg) {
       auto Pair = Map.insert(std::make_pair(Arg, size_t(0)));
@@ -85,27 +88,34 @@ class BlotMapVector {
     }
 
     const_iterator find(const KeyT &Key) const {
-      return const_cast<BlotMapVector &>(*this)->find(Key);
+      return const_cast<BlotMapVector &>(*this).find(Key);
     }
 
-    /// This is similar to erase, but instead of removing the element from the
-    /// vector, it just zeros out the key in the vector. This leaves iterators
-    /// intact, but clients must be prepared for zeroed-out keys when iterating.
-    void erase(const KeyT &Key) { blot(Key); }
-
-    /// This is similar to erase, but instead of removing the element from the
-    /// vector, it just zeros out the key in the vector. This leaves iterators
+    /// Eliminate the element at `Key`. Instead of removing the element from the
+    /// vector, just zero out the key in the vector. This leaves iterators
     /// intact, but clients must be prepared for zeroed-out keys when iterating.
+    ///
+    /// Return true if the element was erased.
+    bool erase(const KeyT &Key) { return blot(Key); }
+
+    /// Eliminate the element at the given iterator. Instead of removing the
+    /// element from the vector, just zero out the key in the vector. This
+    /// leaves iterators intact, but clients must be prepared for zeroed-out
+    /// keys when iterating.
     void erase(iterator I) { erase((*I)->first); }
 
-    /// This is similar to erase, but instead of removing the element from the
+    /// Eliminate the element at `Key`. Instead of removing the element from the
     /// vector, it just zeros out the key in the vector. This leaves iterators
     /// intact, but clients must be prepared for zeroed-out keys when iterating.
-    void blot(const KeyT &Key) {
+    ///
+    /// Return true if the element was found and erased.
+    bool blot(const KeyT &Key) {
       typename MapT::iterator It = Map.find(Key);
-      if (It == Map.end()) return;
+      if (It == Map.end())
+        return false;
       Vector[It->second] = None;
       Map.erase(It);
+      return true;
     }
 
     void clear() {

include/swift/Basic/BlotSetVector.h

@@ -120,7 +120,7 @@ class BlotSetVector {
     if (Iter == Map.end())
       return false;
     unsigned Index = Iter->second;
-    Map.erase(V);
+    Map.erase(Iter);
     Vector[Index] = None;
     return true;
   }

unittests/Basic/BlotMapVectorTest.cpp

@@ -446,7 +446,7 @@ TYPED_TEST(BlotMapVectorTest, EraseTest) {
 // Test erase(value) method
 TYPED_TEST(BlotMapVectorTest, EraseTest2) {
   this->Map[this->getKey()] = this->getValue();
-  this->Map.erase(this->getKey());
+  EXPECT_TRUE(this->Map.erase(this->getKey()));
 
   EXPECT_EQ(0u, this->Map.size());
   EXPECT_TRUE(this->Map.empty());
@@ -639,9 +639,11 @@ TEST(BlotMapVectorCustomTest, SmallBlotMapVectorGrowTest) {
   for (unsigned i = 0; i < 20; ++i)
     map[i] = i + 1;
   for (unsigned i = 0; i < 10; ++i)
-    map.erase(i);
+    EXPECT_TRUE(map.erase(i));
   for (unsigned i = 20; i < 32; ++i)
     map[i] = i + 1;
+  for (unsigned i = 0; i < 10; ++i)
+    EXPECT_FALSE(map.erase(i));
 
   // Size tests
   EXPECT_EQ(22u, map.size());