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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/PDB/Raw/LinearIntegerMap.h:8
+//
+//===----------------------------------------------------------------------===//
+
----------------
It doesn't have to be detailed, but this needs a file comment to describe what this is.


================
Comment at: llvm/include/llvm/DebugInfo/PDB/Raw/LinearIntegerMap.h:69-70
+
+  std::vector<uint32_t> Keys;
+  std::vector<uint32_t> Values;
+};
----------------
You can initialize them by `= {0};`, no?


================
Comment at: llvm/lib/DebugInfo/PDB/Raw/LinearIntegerMap.cpp:22
+Error LinearIntegerMap::load(msf::StreamReader &Stream) {
+  const Header *HAH;
+  if (auto EC = Stream.readObject(HAH))
----------------
What does HAH stand for?


================
Comment at: llvm/lib/DebugInfo/PDB/Raw/LinearIntegerMap.cpp:32-33
+
+  Keys.resize(HAH->Capacity);
+  Values.resize(HAH->Capacity);
+
----------------
You are extending these vectors unconditionally. What was the point of `resize(1)` in the ctor?


================
Comment at: llvm/lib/DebugInfo/PDB/Raw/LinearIntegerMap.cpp:67-70
+  for (uint32_t P : Present)
+    LargestP = P;
+  for (uint32_t D : Deleted)
+    LargestD = D;
----------------
Does this actually find largest numbers?


================
Comment at: llvm/lib/DebugInfo/PDB/Raw/LinearIntegerMap.cpp:153-154
+  growIfAtCapacity();
+
+  return;
+}
----------------
Remove this.


================
Comment at: llvm/lib/DebugInfo/PDB/Raw/LinearIntegerMap.cpp:188
+uint32_t LinearIntegerMap::maxSize(uint32_t capacity) {
+  return capacity * 2 / 3 + 1;
+}
----------------
This needs a comment.


https://reviews.llvm.org/D28715