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

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: llvm/lib/DebugInfo/CodeView/ModuleDebugFragmentRecord.cpp:40
+  default:
+    __debugbreak();
+  }
----------------
inglorion wrote:
> 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?
Yea, this is supposed to be an unreachable.  My bad.


================
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)
----------------
inglorion wrote:
> Do you still need the TODO or is this done now?
Nope, I can remove.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.cpp:76
 
+uint32_t DbiModuleDescriptorBuilder::calculateC13DebugInfoSize() const {
+  uint32_t Result = 0;
----------------
inglorion wrote:
> We're calling this method from both finalize() and finalizeMsfLayout(). Can we avoid having to call it twice?
Unfortunately no, at least not with the way the classes are currently designed.    It's been something I've been thinking about for a while but I don't have a good solution yet.


https://reviews.llvm.org/D32716