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

[Scott Linder via Phabricator via llvm-commits]

scott.linder marked 3 inline comments as done.
scott.linder 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
----------------
JDevlieghere wrote:
> Why did you not choose to make this part of the `ChecksumInfo` struct?
They do not depend on the template parameter, so this saves having two instances of `ChecksumKind` in the typesystem, namely `ChecksumInfo<MDString *>::ChecksumKind` and `ChecksumInfo<StringRef>::ChecksumKind`.


================
Comment at: include/llvm/IR/DebugInfoMetadata.h:585
 
-  static ChecksumKind getChecksumKind(StringRef CSKindStr);
+  static StringRef getChecksumKindAsString(ChecksumKind CSKind);
+  static Optional<ChecksumKind> getChecksumKind(StringRef CSKindStr);
----------------
JDevlieghere wrote:
> Would it make sense to move these into `ChecksumInfo` too?
Similar to above, these are static methods which do not depend on the templated string type (and never will), so I chose to leave them outside. Moving them inside just creates multiple, equivalent names for them.


https://reviews.llvm.org/D43043