[PATCH] D32716: Write CodeView file checksum and line information to PDB

[Bob Haarman via Phabricator via llvm-commits]

inglorion added inline comments.


================
Comment at: llvm/lib/DebugInfo/CodeView/ModuleDebugFragmentRecord.cpp:40
+  default:
+    __debugbreak();
+  }
----------------
This is essentially an assert, right? Do you want this check to be performed even in noassert builds? If not, please rewrite using assert. If you do want this evaluated in noassert builds, can you replace __debugbreak with llvm_unreachable?


================
Comment at: llvm/lib/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.cpp:44
+  Size += 0;                        // TODO: Layout.C11Bytes
+  Size += C13Size;                  // TODO: Layout.C13Bytes
   Size += sizeof(uint32_t);         // GlobalRefs substream size (always 0)
----------------
Do you still need the TODO or is this done now?


================
Comment at: llvm/lib/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.cpp:76
 
+uint32_t DbiModuleDescriptorBuilder::calculateC13DebugInfoSize() const {
+  uint32_t Result = 0;
----------------
We're calling this method from both finalize() and finalizeMsfLayout(). Can we avoid having to call it twice?


https://reviews.llvm.org/D32716