[PATCH] D116960: [ADT] Add an in-place version of toHex()

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

Mostly LGTM, just a couple of comments on the drive-by changes to `hexdigit()` (I might split that (and the new call in toHex()) into a separate commit, but if you keep them here please mention them in the commit message).



================
Comment at: llvm/include/llvm/ADT/StringExtras.h:37
 inline char hexdigit(unsigned X, bool LowerCase = false) {
-  const char HexChar = LowerCase ? 'a' : 'A';
-  return X < 10 ? '0' + X : HexChar + X - 10;
+  assert(X <= 16);
+  static const char *const LUT = "0123456789ABCDEF";
----------------
Should this be `X < 16`?


================
Comment at: llvm/include/llvm/ADT/StringExtras.h:38
+  assert(X <= 16);
+  static const char *const LUT = "0123456789ABCDEF";
+  const uint8_t Offset = LowerCase ? 32 : 0;
----------------
Can you use `constexpr StringLiteral` here?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116960/new/

https://reviews.llvm.org/D116960