hans marked 2 inline comments as done.
hans added a comment.
In D116960#3232461 <https://reviews.llvm.org/D116960#3232461>, @dexonsmith
wrote:
> 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).
Thanks! I'll commit the hexdigit() change separately.
================
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";
----------------
dexonsmith wrote:
> Should this be `X < 16`?
D'oh, yes of course. Thanks for spotting that!
================
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;
----------------
dexonsmith wrote:
> Can you use `constexpr StringLiteral` here?
Yes that would work, but I think it's simpler to just use the built-in types
here.
Actually, there's no need for this to be a pointer, it should just be an array.
I'll fix that.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116960/new/
https://reviews.llvm.org/D116960
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
