[PATCH] D90320: [llvm][StringExtras] Use a lookup table for `hexDigitValue`

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

dexonsmith accepted this revision.
dexonsmith added a comment.

LGTM, thanks for splitting this out.



================
Comment at: llvm/include/llvm/ADT/StringExtras.h:72
+    /// 0-47: Non hexadecimal digits
+    -1U, -1U, -1U, -1U, -1U, -1U, -1U, -1U, -1U, -1U, -1U, -1U, -1U, -1U, -1U,
+    -1U, -1U, -1U, -1U, -1U, -1U, -1U, -1U, -1U, -1U, -1U, -1U, -1U, -1U, -1U,
----------------
I wonder if this would feel more ordered / be easier to read if each line of `-1U` was a (possibly incomplete) group of 8.
- 0-47 would have 6 groups of 8;
- 58-64 would have a group of 7;
- 71-96 would have a group of 1, three groups of 8, and a group of 1;
- 103-255 would have a group of 1, and 19 groups of 8.

I chose 8 because 16 will be too long for 80-column.

If you try it out and it looks worse, then what you have now is probably best.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90320