Review fixes.

6 files changed, 34 insertions, 45 deletions

diff --git a/coding/coding_tests/trie_test.cpp b/coding/coding_tests/trie_test.cpp
index 83ccbd3528..8ef2f3cc3e 100644
--- a/coding/coding_tests/trie_test.cpp
+++ b/coding/coding_tests/trie_test.cpp
@@ -113,7 +113,7 @@ class SingleValueSerializerChar
 {
 public:
   template <typename TWriter>
-  void Serialize(TWriter & writer, char & v) const
+  void Serialize(TWriter & writer, char const & v) const
   {
     WriteToSink(writer, v);
   }
@@ -123,7 +123,7 @@ class SingleValueSerializerUint32
 {
 public:
   template <typename TWriter>
-  void Serialize(TWriter & writer, uint32_t & v) const
+  void Serialize(TWriter & writer, uint32_t const & v) const
   {
     WriteToSink(writer, v);
   }
@@ -134,10 +134,10 @@ class ValueListChar
 public:
   using TValue = char;
 
-  void Init(vector<TValue> const &) {}
-
   ValueListChar(const string & s) : m_string(s) {}
 
+  void Init(vector<TValue> const &) {}
+
   size_t Size() const { return m_string.size(); }
 
   bool IsEmpty() const { return m_string.empty(); }
@@ -291,7 +291,7 @@ UNIT_TEST(TrieBuilder_Build)
     auto const root = trie::ReadTrie<MemReader, ValueListUint32>(memReader, serializer);
     vector<KeyValuePair> res;
     KeyValuePairBackInserter f;
-    trie::ForEachRefWithValues(*root, f, vector<trie::TrieChar>());
+    trie::ForEachRef(*root, f, vector<trie::TrieChar>());
     sort(f.m_v.begin(), f.m_v.end());
     TEST_EQUAL(v, f.m_v, ());
   }
diff --git a/coding/compressed_bit_vector.cpp b/coding/compressed_bit_vector.cpp
index 9f81e7a3a6..8ae2157763 100644
--- a/coding/compressed_bit_vector.cpp
+++ b/coding/compressed_bit_vector.cpp
@@ -185,7 +185,6 @@ DenseCBV::DenseCBV(vector<uint64_t> const & setBits)
 {
   if (setBits.empty())
   {
-    m_popCount = 0;
     return;
   }
   uint64_t const maxBit = *max_element(setBits.begin(), setBits.end());
@@ -331,6 +330,7 @@ unique_ptr<CompressedBitVector> CompressedBitVectorBuilder::FromCBV(CompressedBi
     return CompressedBitVectorBuilder::FromBitPositions(sparse.m_positions);
   }
   }
+  CHECK(false, ("Unknown strategy when building a compressed bit vector."));
   return unique_ptr<CompressedBitVector>();
 }
 
diff --git a/coding/compressed_bit_vector.hpp b/coding/compressed_bit_vector.hpp
index 93329a1423..18fd8f6982 100644
--- a/coding/compressed_bit_vector.hpp
+++ b/coding/compressed_bit_vector.hpp
@@ -1,3 +1,5 @@
+#pragma once
+
 #include "coding/read_write_utils.hpp"
 #include "coding/reader.hpp"
 #include "coding/writer.hpp"
@@ -74,7 +76,7 @@ public:
   DenseCBV() = default;
 
   // Builds a dense CBV from a list of positions of set bits.
-  DenseCBV(vector<uint64_t> const & setBits);
+  explicit DenseCBV(vector<uint64_t> const & setBits);
 
   // Not to be confused with the constructor: the semantics
   // of the array of integers is completely different.
@@ -115,9 +117,9 @@ public:
   friend class CompressedBitVectorBuilder;
   using TIterator = vector<uint64_t>::const_iterator;
 
-  SparseCBV(vector<uint64_t> const & setBits);
+  explicit SparseCBV(vector<uint64_t> const & setBits);
 
-  SparseCBV(vector<uint64_t> && setBits);
+  explicit SparseCBV(vector<uint64_t> && setBits);
 
   // Returns the position of the i'th set bit.
   uint64_t Select(size_t i) const;
diff --git a/coding/trie.hpp b/coding/trie.hpp
index d8de8d1e68..4c1b397b08 100644
--- a/coding/trie.hpp
+++ b/coding/trie.hpp
@@ -26,8 +26,8 @@ public:
 
   struct Edge
   {
-    typedef buffer_vector<TrieChar, 8> EdgeStrT;
-    EdgeStrT m_str;
+    using TEdgeLabel = buffer_vector<TrieChar, 8>;
+    TEdgeLabel m_label;
   };
 
   buffer_vector<Edge, 8> m_edge;
@@ -68,34 +68,18 @@ struct FixedSizeValueReader
 };
 
 template <typename TValueList, typename TF, typename TString>
-void ForEachRef(Iterator<TValueList> const & iter, TF && f, TString const & s)
+void ForEachRef(Iterator<TValueList> const & it, TF && f, TString const & s)
 {
-  iter.m_valueList.ForEach([&f, &s](typename TValueList::TValue const & /* value */)
-                           {
-                             f(s);
-                           });
-  for (size_t i = 0; i < iter.m_edge.size(); ++i)
+  it.m_valueList.ForEach([&f, &s](typename TValueList::TValue const & value)
+                         {
+                           f(s, value);
+                         });
+  for (size_t i = 0; i < it.m_edge.size(); ++i)
   {
     TString s1(s);
-    s1.insert(s1.end(), iter.m_edge[i].m_str.begin(), iter.m_edge[i].m_str.end());
-    auto it = iter.GoToEdge(i);
-    ForEachRef(*it, f, s1);
-  }
-}
-
-template <typename TValueList, typename TF, typename TString>
-void ForEachRefWithValues(Iterator<TValueList> const & iter, TF && f, TString const & s)
-{
-  iter.m_valueList.ForEach([&f, &s](typename TValueList::TValue const & value)
-                           {
-                             f(s, value);
-                           });
-  for (size_t i = 0; i < iter.m_edge.size(); ++i)
-  {
-    TString s1(s);
-    s1.insert(s1.end(), iter.m_edge[i].m_str.begin(), iter.m_edge[i].m_str.end());
-    auto it = iter.GoToEdge(i);
-    ForEachRefWithValues(*it, f, s1);
+    s1.insert(s1.end(), it.m_edge[i].m_label.begin(), it.m_edge[i].m_label.end());
+    auto nextIt = it.GoToEdge(i);
+    ForEachRef(*nextIt, f, s1);
   }
 }
 }  // namespace trie
diff --git a/coding/trie_builder.hpp b/coding/trie_builder.hpp
index f640b44ccc..714c8f02f6 100644
--- a/coding/trie_builder.hpp
+++ b/coding/trie_builder.hpp
@@ -4,6 +4,7 @@
 #include "coding/varint.hpp"
 
 #include "base/buffer_vector.hpp"
+#include "base/logging.hpp"
 
 #include "std/algorithm.hpp"
 #include "std/vector.hpp"
@@ -210,8 +211,10 @@ void AppendValue(TNodeInfo & node, TValue const & value)
   ASSERT(node.m_temporaryValueList.empty() || node.m_temporaryValueList.back() <= value, ());
   if (!node.m_temporaryValueList.empty() && node.m_temporaryValueList.back() == value)
     return;
-  ASSERT(node.m_mayAppend, ());
-  node.m_temporaryValueList.push_back(value);
+  if (node.m_mayAppend)
+    node.m_temporaryValueList.push_back(value);
+  else
+    LOG(LERROR, ("Cannot append to a finalized value list."));
 }
 
 template <typename TSink, typename TIter, typename TValueList, typename TSerializer>
@@ -241,8 +244,8 @@ void Build(TSink & sink, TSerializer const & serializer, TIter const beg, TIter
     while (nCommon < min(key.size(), prevKey.size()) && prevKey[nCommon] == key[nCommon])
       ++nCommon;
 
-    PopNodes(sink, serializer, nodes, nodes.size() - nCommon - 1);  // Root is also a common node.
-
+    // Root is also a common node.
+    PopNodes(sink, serializer, nodes, nodes.size() - nCommon - 1);
     uint64_t const pos = sink.Pos();
     for (size_t i = nCommon; i < key.size(); ++i)
       nodes.emplace_back(pos, key[i]);
diff --git a/coding/trie_reader.hpp b/coding/trie_reader.hpp
index b84aeaf7cf..4546e916fe 100644
--- a/coding/trie_reader.hpp
+++ b/coding/trie_reader.hpp
@@ -72,7 +72,7 @@ public:
     }
 
     return make_unique<Iterator0<TReader, TValueList, TSerializer>>(
-        m_reader.SubReader(offset, size), this->m_edge[i].m_str.back(), m_serializer);
+        m_reader.SubReader(offset, size), this->m_edge[i].m_label.back(), m_serializer);
   }
 
 private:
@@ -108,7 +108,7 @@ private:
       uint8_t const header = ReadPrimitiveFromSource<uint8_t>(src);
       m_edgeInfo[i].m_isLeaf = ((header & 128) != 0);
       if (header & 64)
-        e.m_str.push_back(baseChar + bits::ZigZagDecode(header & 63U));
+        e.m_label.push_back(baseChar + bits::ZigZagDecode(header & 63U));
       else
       {
         // [vu edgeLen-1]: if edgeLen-1 in header == 63
@@ -118,9 +118,9 @@ private:
         edgeLen += 1;
 
         // [vi edgeChar0 - baseChar] [vi edgeChar1 - edgeChar0] ... [vi edgeCharN - edgeCharN-1]
-        e.m_str.reserve(edgeLen);
+        e.m_label.reserve(edgeLen);
         for (uint32_t i = 0; i < edgeLen; ++i)
-          e.m_str.push_back(baseChar += ReadVarInt<int32_t>(src));
+          e.m_label.push_back(baseChar += ReadVarInt<int32_t>(src));
       }
 
       // [child size]: if the child is not the last one
@@ -128,7 +128,7 @@ private:
       if (i != childCount - 1)
         m_edgeInfo[i + 1].m_offset += ReadVarUint<uint32_t>(src);
 
-      baseChar = e.m_str[0];
+      baseChar = e.m_label[0];
     }
 
     uint32_t const currentOffset = static_cast<uint32_t>(src.Pos());