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

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added inline comments.


================
Comment at: include/llvm/IR/DebugInfoMetadata.h:504
   // file.
   enum ChecksumKind {
+    // The first variant was originally CSK_None, encoded as 0. The new
----------------
Why did you not choose to make this part of the `ChecksumInfo` struct?


================
Comment at: include/llvm/IR/DebugInfoMetadata.h:519
+    ChecksumKind Kind;
+    /// The string value of the checksum.
+    T Value;
----------------
aprantl wrote:
> If this is a string, why does this need to be a template?
It's used for both `MDString`s and `StringRef`s.


================
Comment at: include/llvm/IR/DebugInfoMetadata.h:585
 
-  static ChecksumKind getChecksumKind(StringRef CSKindStr);
+  static StringRef getChecksumKindAsString(ChecksumKind CSKind);
+  static Optional<ChecksumKind> getChecksumKind(StringRef CSKindStr);
----------------
Would it make sense to move these into `ChecksumInfo` too?


================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:177
+      switch (F->getChecksum()->Kind) {
+      case DIFile::CSK_MD5:  CSKind = FileChecksumKind::MD5; break;
+      case DIFile::CSK_SHA1: CSKind = FileChecksumKind::SHA1; break;
----------------
Why do we have/need two enums to express the same thing? Are the values different?


Repository:
  rL LLVM

https://reviews.llvm.org/D43043