Review fixes.

coding/coding_tests/compressed_bit_vector_test.cpp

@@ -415,7 +415,7 @@ UNIT_TEST(CompressedBitVector_DenseLeaveFirstNBits)
 
   {
     vector<uint64_t> setBits;
-    for (int i = 0; i < 100; ++i)
+    for (uint64_t i = 0; i < 100; ++i)
       setBits.push_back(2 * i);
     auto cbv = coding::CompressedBitVectorBuilder::FromBitPositions(setBits);
     TEST_EQUAL(cbv->PopCount(), 100, ());
@@ -425,7 +425,7 @@ UNIT_TEST(CompressedBitVector_DenseLeaveFirstNBits)
     TEST_EQUAL(cbv->PopCount(), 50, ());
     TEST_EQUAL(cbv->GetStorageStrategy(), coding::CompressedBitVector::StorageStrategy::Dense, ());
 
-    for (int i = 0; i < 50; ++i)
+    for (uint64_t i = 0; i < 50; ++i)
     {
       TEST(cbv->GetBit(2 * i), ());
       TEST(!cbv->GetBit(2 * i + 1), ());
@@ -436,15 +436,15 @@ UNIT_TEST(CompressedBitVector_DenseLeaveFirstNBits)
 UNIT_TEST(CompressedBitVector_SparseLeaveFirstNBits)
 {
   vector<uint64_t> setBits;
-  for (int p = 0; p < 20; ++p)
+  for (int p = 0; p < 10; ++p)
     setBits.push_back(static_cast<uint64_t>(1) << p);
   auto cbv = coding::CompressedBitVectorBuilder::FromBitPositions(setBits);
-  TEST_EQUAL(cbv->PopCount(), 20, ());
+  TEST_EQUAL(cbv->PopCount(), 10, ());
   TEST_EQUAL(cbv->GetStorageStrategy(), coding::CompressedBitVector::StorageStrategy::Sparse, ());
 
   cbv = cbv->LeaveFirstSetNBits(100);
-  TEST_EQUAL(cbv->PopCount(), 20, ());
-  for (uint64_t bit = 0; bit < (1 << 20); ++bit)
+  TEST_EQUAL(cbv->PopCount(), 10, ());
+  for (uint64_t bit = 0; bit < (1 << 10); ++bit)
   {
     if (bit != 0 && (bit & (bit - 1)) == 0)
       TEST(cbv->GetBit(bit), (bit));
@@ -452,11 +452,11 @@ UNIT_TEST(CompressedBitVector_SparseLeaveFirstNBits)
       TEST(!cbv->GetBit(bit), (bit));
   }
 
-  cbv = cbv->LeaveFirstSetNBits(10);
-  TEST_EQUAL(cbv->PopCount(), 10, ());
-  for (uint64_t bit = 0; bit < (1 << 20); ++bit)
+  cbv = cbv->LeaveFirstSetNBits(8);
+  TEST_EQUAL(cbv->PopCount(), 8, ());
+  for (uint64_t bit = 0; bit < (1 << 10); ++bit)
   {
-    if (bit != 0 && (bit & (bit - 1)) == 0 && bit < (1 << 10))
+    if (bit != 0 && (bit & (bit - 1)) == 0 && bit < (1 << 8))
       TEST(cbv->GetBit(bit), (bit));
     else
       TEST(!cbv->GetBit(bit), (bit));
@@ -464,6 +464,6 @@ UNIT_TEST(CompressedBitVector_SparseLeaveFirstNBits)
 
   cbv = cbv->LeaveFirstSetNBits(0);
   TEST_EQUAL(cbv->PopCount(), 0, ());
-  for (uint64_t bit = 0; bit < (1 << 20); ++bit)
+  for (uint64_t bit = 0; bit < (1 << 10); ++bit)
     TEST(!cbv->GetBit(bit), (bit));
 }

coding/compressed_bit_vector.hpp

@@ -58,6 +58,8 @@ public:
   // Would operator[] look better?
   virtual bool GetBit(uint64_t pos) const = 0;
 
+  // Returns a subset of the current bit vector with first
+  // min(PopCount(), |n|) set bits.
   virtual unique_ptr<CompressedBitVector> LeaveFirstSetNBits(uint64_t n) const = 0;
 
   // Returns the strategy used when storing this bit vector.

search/v2/features_filter.cpp

@@ -43,15 +43,7 @@ unique_ptr<coding::CompressedBitVector> ViewportFilter::Filter(
   auto result = coding::CompressedBitVector::Intersect(m_filter, cbv);
   if (!coding::CompressedBitVector::IsEmpty(result))
     return result;
-
-  uint64_t limit = std::min(static_cast<uint64_t>(m_threshold), cbv.PopCount());
-  vector<uint64_t> positions;
-  for (uint64_t pos = 0; positions.size() != limit; ++pos)
-  {
-    if (cbv.GetBit(pos))
-      positions.push_back(pos);
-  }
-  return coding::CompressedBitVectorBuilder::FromBitPositions(move(positions));
+  return cbv.LeaveFirstSetNBits(m_threshold);
 }
 }  // namespace v2
 }  // namespace search

search/v2/features_filter.hpp

@@ -13,7 +13,7 @@ namespace v2
 {
 // A lightweight filter of features.
 //
-// NOTE: this class and it's subclasses *ARE* thread-safe.
+// NOTE: this class and its subclasses *ARE* thread-safe.
 class FeaturesFilter
 {
 public:
@@ -31,8 +31,8 @@ protected:
   uint32_t const m_threshold;
 };
 
-// Exact filter - leaves only features belonging to the set it had
-// been constructed from.
+// Exact filter - leaves only features belonging to the set it was
+// constructed from.
 class LocalityFilter : public FeaturesFilter
 {
 public:
@@ -44,10 +44,10 @@ public:
 };
 
 // Fuzzy filter - tries to leave only features belonging to the set it
-// had been constructed from, but if the result is empty, leaves at
-// most first |threshold| features. This property is quite useful when
-// there are no matching features in viewport but it's ok to process a
-// limited number of features outside the viewport.
+// was constructed from, but if the result is empty, leaves at most
+// first |threshold| features instead. This property is quite useful
+// when there are no matching features in viewport but it's ok to
+// process a limited number of features outside the viewport.
 class ViewportFilter : public FeaturesFilter
 {
 public:

search/v2/geocoder.cpp

@@ -1095,7 +1095,7 @@ void Geocoder::MatchPOIsAndBuildings(size_t curToken)
 
     bool const looksLikeHouseNumber = feature::IsHouseNumber(m_layers.back().m_subQuery);
 
-    if (coding::CompressedBitVector::IsEmpty(features.Get()) && !looksLikeHouseNumber)
+    if (features.IsEmpty() && !looksLikeHouseNumber)
       break;
 
     if (n == 1)

search/v2/geocoder.hpp

@@ -188,7 +188,7 @@ private:
   void MatchViewportAndPosition();
 
   // Tries to do geocoding in a limited scope, assuming that knowledge
-  // about high-level features, like cities or countries, is somehow
+  // about high-level features, like cities or countries, is
   // incorporated into |filter|.
   void LimitedSearch(FeaturesFilter const & filter);