[PATCH] D43326: [PDB] Fix emission of PDB string table

[Adrian McCarthy via Phabricator via llvm-commits]

amccarth added a comment.

Excellent patch description!  It's reassuring to see all the nagging questions explained.  I like the test, too.



================
Comment at: llvm/include/llvm/DebugInfo/PDB/Native/HashTable.h:180
+    Buckets.swap(NewMap.Buckets);
+    std::swap(Present, NewMap.Present);
+    std::swap(Deleted, NewMap.Deleted);
----------------
The usual idiom is:

```
using std::swap;
swap(Present, NewMap.Present);
```

That makes it possible for ADL to find if there's a specialization for swap and to fall back to std::swap if there isn't.  It doesn't matter much here, since SparseBitVector doesn't seem to have a swap specialization.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/NamedStreamMap.cpp:35
+    // HASH Hasher<ULONG*, USHORT*>::hashPbCb(PB pb, size_t cb, ULONG ulMod).
+    // Here, the type HASH is a typedef of unsigned short.
+    // ** It is not a bug that we truncate the result of hashStringV1, in fact
----------------
This sentence ("Here, ... unsigned short.") seems unnecessary.  The type is actually std::uint16_t, and it's just above.  Also, it detracts from the important comment that follows.


https://reviews.llvm.org/D43326