fix sorted_vector_{set,map} insert with bad hint
authorNathan Bronson <ngbronson@fb.com>
Tue, 21 Mar 2017 21:28:38 +0000 (14:28 -0700)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Tue, 21 Mar 2017 21:36:20 +0000 (14:36 -0700)
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

folly/sorted_vector_types.h
folly/test/sorted_vector_test.cpp

index fe586a8..41d26fc 100644 (file)
@@ -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;
       }
     }
 
index 9207fa0..257b5df 100644 (file)
@@ -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) {