[PATCH] D61277: [PDB] Fix hash function used to write /src/headerblock

[Zachary Turner via Phabricator via llvm-commits]

zturner accepted this revision.
zturner added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/PDBStringTableBuilder.cpp:29-35
+  // The reference implementation doesn't include code for /src/headerblock
+  // handling, but it can only read natvis entries lld's PDB files if
+  // this hash function truncates the hash to 16 bit.
+  // PDB/include/misc.h in the reference implementation has a hashSz() function
+  // that returns an unsigned short, that seems what's being used for
+  // /src/headerblock.
+  return static_cast<uint16_t>(Table->getIdForString(S));
----------------
IIUC, this shouldn't be specific to `/src/headerblock` right?  Because this `StringTableHashTraits` is used for the entire string table.  This means the comment is overly specific as it's referring to `/src/headerblock` from code that ultimately doesn't really have anything to do with `/src/headerblock`.  

It also means that changing this will probably affect lots of other things that are subtle and hard to test.  So be on the lookout for other weird failures.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61277/new/

https://reviews.llvm.org/D61277