[PATCH] D35515: [PDB] Finish and simplify TPI hashing

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/TpiHashing.cpp:31-33
+  bool ForwardRef = bool(Opts & ClassOptions::ForwardReference);
+  bool Scoped = bool(Opts & ClassOptions::Scoped);
+  bool HasUniqueName = bool(Opts & ClassOptions::HasUniqueName);
----------------
ruiu wrote:
> Do you need these casts to bool?
Apparently I do. The old code was implicitly converting uint16_t to bool, but you can't implicitly convert a ClassOptions strongly typed enum to bool.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/TpiHashing.cpp:83-88
+  // Run CRC32 over the bytes. This corresponds to `hashBufv8`.
+  JamCRC JC(/*Init=*/0U);
+  ArrayRef<char> Bytes(reinterpret_cast<const char *>(Rec.data().data()),
+                       Rec.data().size());
+  JC.update(Bytes);
+  return JC.getCRC();
----------------
ruiu wrote:
> You can make this as `default:` for the above switch.
I declare a variable, though, so I'd need more braces, and I'm trying to pacify any compilers that might warn about falling off the end of the function.


https://reviews.llvm.org/D35515