[PATCH] D19500: Parse and dump PDB DBI Stream Header Information

[David Majnemer via llvm-commits]

majnemer added a subscriber: majnemer.

================
Comment at: lib/DebugInfo/PDB/Raw/PDBDbiStream.cpp:24-31
@@ +23,10 @@
+//};
+static const uint16_t FLAG_INCREMENTAL_MASK = 0x0001;
+static const uint16_t FLAG_INCREMENTAL_SHIFT = 0;
+
+static const uint16_t FLAG_STRIPPED_MASK = 0x0002;
+static const uint16_t FLAG_STRIPPED_SHIFT = 1;
+
+static const uint16_t FLAG_HAS_CTYPES_MASK = 0x0004;
+static const uint16_t FLAG_HAS_CTYPES_SHIFT = 2;
+
----------------
You are in an anonymous namespace, no need for static.  Also, these don't seem to correspond to the LLVM naming style.

================
Comment at: lib/DebugInfo/PDB/Raw/PDBDbiStream.cpp:73
@@ +72,3 @@
+PDBDbiStream::PDBDbiStream(PDBFile &File) : Pdb(File), Stream(3, File) {
+  static_assert(sizeof(HeaderInfo) == 64, "Invalid HeaderInfo size!");
+}
----------------
No need to stick this assertion in the constructor, I'd leave it with the type definition.

================
Comment at: lib/DebugInfo/PDB/Raw/PDBDbiStream.cpp:86-87
@@ +85,4 @@
+
+  if (Header->VersionSignature != -1)
+    return std::make_error_code(std::errc::illegal_byte_sequence);
+
----------------
All signatures != -1 are ok?

================
Comment at: lib/DebugInfo/PDB/Raw/PDBDbiStream.cpp:114
@@ +113,3 @@
+bool PDBDbiStream::isIncrementallyLinked() const {
+  return !!((Header->Flags & FLAG_INCREMENTAL_MASK) >> FLAG_INCREMENTAL_SHIFT);
+}
----------------
This is a little confusing to read, why not just write `return (Header->Flags & FLAG_INCREMENTAL_MASK) != 0;`


http://reviews.llvm.org/D19500