fix sorted_vector_{set,map} insert with bad hint

Summary:
sorted_vector_{set,map} can silently drop insert if the hinted
insertion position is too small.  This diff fixes it.

Reviewed By: yfeldblum

Differential Revision: D4747319

fbshipit-source-id: 31e399d07a0b77b700edf034dc723cb997dc8e16

diff --git a/folly/sorted_vector_types.h b/folly/sorted_vector_types.h
index fe586a861fb7c31c2138ef9fcd7b3e20b0c90abb..41d26fc033dcb769fa2fabc954ec9bb2256a061d 100644 (file)
--- a/folly/sorted_vector_types.h
+++ b/folly/sorted_vector_types.h
@@ -122,22 +122,20 @@ namespace detail {
   {
     const typename OurContainer::value_compare& cmp(sorted.value_comp());
     if (hint == cont.end() || cmp(value, *hint)) {
-      if (hint == cont.begin()) {
-        po.increase_capacity(cont, cont.begin());
-        return cont.insert(cont.begin(), std::move(value));
-      }
-      if (cmp(*(hint - 1), value)) {
+      if (hint == cont.begin() || cmp(*(hint - 1), value)) {
         hint = po.increase_capacity(cont, hint);
         return cont.insert(hint, std::move(value));
+      } else {
+        return sorted.insert(std::move(value)).first;
       }
-      return sorted.insert(std::move(value)).first;
     }
 
     if (cmp(*hint, value)) {
       if (hint + 1 == cont.end() || cmp(value, *(hint + 1))) {
-        typename OurContainer::iterator it =
-          po.increase_capacity(cont, hint + 1);
-        return cont.insert(it, std::move(value));
+        hint = po.increase_capacity(cont, hint + 1);
+        return cont.insert(hint, std::move(value));
+      } else {
+        return sorted.insert(std::move(value)).first;
       }
     }
 

diff --git a/folly/test/sorted_vector_test.cpp b/folly/test/sorted_vector_test.cpp
index 9207fa03ca0441422b6e432b580610dfeb48b068..257b5dff56cc349a2a7df0408665229635cf40a9 100644 (file)
--- a/folly/test/sorted_vector_test.cpp
+++ b/folly/test/sorted_vector_test.cpp
@@ -144,6 +144,21 @@ TEST(SortedVectorTypes, SimpleSetTest) {
   EXPECT_TRUE(cpy2 == cpy);
 }
 
+TEST(SortedVectorTypes, BadHints) {
+  for (int toInsert = -1; toInsert <= 7; ++toInsert) {
+    for (int hintPos = 0; hintPos <= 4; ++hintPos) {
+      sorted_vector_set<int> s;
+      for (int i = 0; i <= 3; ++i) {
+        s.insert(i * 2);
+      }
+      s.insert(s.begin() + hintPos, toInsert);
+      size_t expectedSize = (toInsert % 2) == 0 ? 4 : 5;
+      EXPECT_EQ(s.size(), expectedSize);
+      check_invariant(s);
+    }
+  }
+}
+
 TEST(SortedVectorTypes, SimpleMapTest) {
   sorted_vector_map<int,float> m;
   for (int i = 0; i < 1000; ++i) {