[PATCH] D21393: [pdbdump] Verify LF_{CLASS, ENUM, INTERFACE, STRUCTURE, UNION} records with different hash function.

[Zachary Turner via llvm-commits]

zturner added inline comments.

================
Comment at: lib/DebugInfo/PDB/Raw/TpiStream.cpp:69
@@ -69,1 +68,3 @@
+template <typename T>
+static uint32_t getTpiHash(T &Rec, const CVRecord<TypeLeafKind> &RawRec) {
   auto Opts = static_cast<uint16_t>(Rec.getOptions());
----------------
I was imagining that we wouldn't even have this `TpiHashVerifier` class, and that everything would just go in `TypeDumper`.  Does this not work?  For example, imagine you delete all of this code, and delete the `TpiHashVerifier` class, and delete lines 205-209.  Then, in `TypeDumper.cpp`, you change `CVTypeDumper::visitClass` to look like this:

```
Error CVTypeDumper::visitClass(ClassRecord &Class) {
  uint16_t Props = static_cast<uint16_t>(Class.getOptions());
  W->printNumber("MemberCount", Class.getMemberCount());
  W->printFlags("Properties", Props, makeArrayRef(ClassOptionNames));
  printTypeIndex("FieldList", Class.getFieldList());
  printTypeIndex("DerivedFrom", Class.getDerivationList());
  printTypeIndex("VShape", Class.getVTableShape());
  W->printNumber("SizeOf", Class.getSize());
  W->printString("Name", Class.getName());
  if (Props & uint16_t(ClassOptions::HasUniqueName))
    W->printString("LinkageName", Class.getUniqueName());
  Name = Class.getName();

  /* NEW CODE HERE */
  if (auto EC = verifyHash(Class))
    return EC;
  return Error::success();
}

```

With the current implementation, we walk the entire type array twice.  Once to verify hashes, and once to dump them.  So if we could do it all in one pass, it seems better.


http://reviews.llvm.org/D21393