[PATCH] D28715: Raise the PDB Hash Map out of the NameMap class

David Majnemer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 13 17:30:03 PST 2017


majnemer added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/PDB/Raw/LinearIntegerMap.h:59
+  std::pair<uint32_t, LinearIntegerMapIterator> findInternal(uint32_t K);
+  static uint32_t maxSize(uint32_t capacity);
+  void growIfAtCapacity();
----------------
`Capacity` ?


================
Comment at: llvm/include/llvm/DebugInfo/PDB/Raw/LinearIntegerMap.h:79
+  LinearIntegerMapIterator(const LinearIntegerMap &Map);
+  LinearIntegerMapIterator() {}
+
----------------
`= default;` ?


================
Comment at: llvm/lib/DebugInfo/PDB/Raw/LinearIntegerMap.cpp:187-188
+
+uint32_t LinearIntegerMap::maxSize(uint32_t capacity) {
+  return capacity * 2 / 3 + 1;
+}
----------------
`Capacity` ? Also, I'd add parens.


================
Comment at: llvm/lib/DebugInfo/PDB/Raw/LinearIntegerMap.cpp:194
+    return;
+  uint32_t NewCapacity = capacity() * 3 / 2 + 1;
+  Keys.resize(NewCapacity);
----------------
`capacity() * 3` can overflow, should there be a std::max with capacity()? Maybe `std::max(capacity() * 3 / 2 + 1, capacity() + 1)` ?


https://reviews.llvm.org/D28715





More information about the llvm-commits mailing list