fix semantics of QMS::Iterator::skipTo

Summary: Passing large `key` doesn't correctly advance position to the end.

Reviewed By: ot

Differential Revision: D29712973

fbshipit-source-id: 7da7c49250753c12f3703ccf49107e56bf841131

folly/experimental/QuotientMultiSet-inl.h

@@ -31,7 +31,7 @@ namespace folly {
 
 namespace qms_detail {
 
-/*
+/**
  * Reference: Faster Remainder by Direct Computation: Applications to Compilers
  * and Software Libraries, Software: Practice and Experience 49 (6), 2019.
  */
@@ -287,13 +287,7 @@ uint64_t QuotientMultiSet<Instructions>::getBlockPayload(
 template <class Instructions>
 QuotientMultiSet<Instructions>::Iterator::Iterator(
     const QuotientMultiSet<Instructions>* qms)
-    : qms_(qms),
-      key_(0),
-      occBlockIndex_(-1),
-      occOffsetInBlock_(0),
-      occWord_(0),
-      occBlock_(nullptr),
-      pos_(-1) {}
+    : qms_(qms) {}
 
 template <class Instructions>
 bool QuotientMultiSet<Instructions>::Iterator::next() {
@@ -326,7 +320,7 @@ bool QuotientMultiSet<Instructions>::Iterator::next() {
 template <class Instructions>
 bool QuotientMultiSet<Instructions>::Iterator::skipTo(uint64_t key) {
   if (key > qms_->maxKey_) {
-    return false;
+    return setEnd();
   }
   const auto qr =
       qms_detail::getQuotientAndRemainder(key, qms_->divisor_, qms_->fraction_);

folly/experimental/QuotientMultiSet.h

@@ -203,24 +203,24 @@ class QuotientMultiSet<Instructions>::Iterator {
   bool nextOccupied();
 
   const QuotientMultiSet<Instructions>* qms_;
-  uint64_t key_;
+  uint64_t key_ = 0;
 
   // State members for the quotient occupied position.
   // Block index of key_'s occupied slot.
-  size_t occBlockIndex_;
+  size_t occBlockIndex_ = -1;
   // Block offset of key_'s occupied slot.
-  uint64_t occOffsetInBlock_;
+  uint64_t occOffsetInBlock_ = 0;
   // Occupied words of the occupiedBlock_ after quotientBlockOffset_.
-  uint64_t occWord_;
+  uint64_t occWord_ = 0;
   // Block of the current occupied slot.
-  const Block* occBlock_;
+  const Block* occBlock_ = nullptr;
 
   // State member for the actual key position.
   // Position of the current key_.
-  size_t pos_;
+  size_t pos_ = -1;
 };
 
-/*
+/**
  * Class to build a QuotientMultiSet.
  *
  * The builder requires inserting elements in non-decreasing order.

folly/experimental/test/QuotientMultiSetTest.cpp

@@ -72,7 +72,7 @@ class QuotientMultiSetTest : public ::testing::Test {
       pos = reader.getBlockPayload((range.end - 1) / kBlockSize);
       EXPECT_EQ(index - 1, pos + (range.end - 1) % kBlockSize) << debugInfo();
 
-      iter.skipTo(key);
+      EXPECT_TRUE(iter.skipTo(key));
       EXPECT_EQ(key, iter.key()) << debugInfo();
       EXPECT_EQ(range.begin, iter.pos()) << debugInfo();
       EXPECT_EQ(
@@ -109,6 +109,7 @@ class QuotientMultiSetTest : public ::testing::Test {
     if (keys.back() < maxKey) {
       uint64_t key = folly::Random::rand64(keys.back(), maxKey, rng) + 1;
       EXPECT_FALSE(reader.equalRange(key)) << keys.back() << " " << key;
+      EXPECT_FALSE(iter.done());
       EXPECT_FALSE(iter.skipTo(key));
       EXPECT_TRUE(iter.done());
     }
@@ -120,6 +121,13 @@ class QuotientMultiSetTest : public ::testing::Test {
     }
     EXPECT_FALSE(nextIter.next());
     EXPECT_TRUE(nextIter.done());
+
+    if (maxKey + 1 != 0) {
+      folly::QuotientMultiSet<>::Iterator skipToEndIter(&reader);
+      EXPECT_FALSE(skipToEndIter.done());
+      EXPECT_FALSE(skipToEndIter.skipTo(maxKey + 1));
+      EXPECT_TRUE(skipToEndIter.done());
+    }
   }
 
   std::mt19937 rng;