[PATCH] Teach llvm::format_hex() to support formatting without a prefix '0x'

Zachary Turner zturner at google.com
Fri Jan 23 13:43:42 PST 2015


================
Comment at: include/llvm/Support/Format.h:278-279
@@ -275,1 +277,4 @@
+///   OS << format_hex(255, 2, true, false) => FF
+inline FormattedNumber format_hex(uint64_t N, unsigned Width,
+                                  bool Upper = false, bool HexPrefix = true) {
   assert(Width <= 18 && "hex width must be <= 18");
----------------
rnk wrote:
> I'd rather have a special purpose helper than a second default value bool. format_hex_noprefix? format_noprefix_hex? format_hex_bytes?
I agree in principle that default values suck, but on the other hand I don't think HexPrefix is any more or less special than Upper, aside from having been implemented later.  So it's inconsistent that one of the options is specified via a default argument, while another option is specified by using a different function even though there's nothing fundamentally different about the two options.  Something like this would be a good time for a flags enum, but that would require updating thousands of lines of code.

Another thing is that it makes the pre-function comment showing sample inputs and outputs a little redundant now.  I guess I would just copy over all 4 samples but remove the 0x from each of them?

Either way, I don't feel too strongly, so I don't mind changing it.

http://reviews.llvm.org/D7151

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list