[PATCH] D43043: [DebugInfo] Unify ChecksumKind and Checksum value in DIFile

[Scott Linder via Phabricator via llvm-commits]

scott.linder added inline comments.


================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:1555
+  if (N->getRawChecksum()) {
+    Record.push_back(N->getRawChecksum()->Kind);
+    Record.push_back(VE.getMetadataOrNullID(N->getRawChecksum()->Value));
----------------
aprantl wrote:
> The comment in MetadataLoader claims that the None kind uses 0 as a representation.  Why is this condition necessary?
I'm not sure I understand what you are asking; the old code would have implicitly done the same here, as `CSK_None` was encoded as 0.  In the new code the `None` variant of the `Optional` does not have an "encoding", so we have to explicitly encode each part of the struct, even if the `Optional` is `None`. In order to retain backwards compatibility, we still reserve null bytes for this purpose, and I documented this in comments on each end and in the `ChecksumKind` enum.


================
Comment at: test/Bitcode/upgrade-dbg-checksum.ll:16
+; Function Attrs: noinline nounwind optnone uwtable
+define i32 @f() #0 !dbg !15 {
+  ret i32 42, !dbg !18
----------------
aprantl wrote:
> I don't think you need a function or a global at all. An empty module with just two DICompileUnits should be sufficient.
You are right, I will minimize the test case.


https://reviews.llvm.org/D43043