[Support] Proposed alternative to llvm::format()

[kledzik]

Summary

In using llvm::format() to dump columnar tables of data in my recent enhancements to llvm-objdump, I ran into some issues uncovered by the built bots that show how unsafe llvm::format() is. The issues I hit where that the compiler did not check that %x match the integer size and that %s can match a StringRef but produce the wrong output. To use a StringRef with format() is also ugly because you have to convert to a std::string then call c_str().

All I really needed was a way to generate fixed with hexadecimal (e.g. 0x00001234) and fixed with strings (e.g. right or left justified strings in a column). I whipped up an alternative that is easy to use with the stream operator and generates fixed with output.

[silvas]

This is the approach I've always wanted ever since I experienced YAMLIO's
Hex* classes. I can never remember the right printf format specifiers for
width and stuff (not to mention the iostream manipulators), and the PRI*
macros are ugly and error-prone.

Also you're not the only one running into these issues: llvm::format, such
as r185766 and PR20715. I also seem to remember one more, but I can't find
it.

The argument could be made that we should improve clang's printf checking
to handle things like the snprintf in the function template like what
llvm::format uses, but I think that's orthogonal.

• Sean Silva

[• rafael]

This looks really nice!

[kledzik]

Add doxygen comments and unit tests.

[rnk]

I also like this, and I think that's enough +1 power to go ahead and land it.

[kledzik]

committed in r218463

include/llvm/Support/Format.h

 #ifndef LLVM_SUPPORT_FORMAT_H
 #define LLVM_SUPPORT_FORMAT_H
 
+#include "llvm/ADT/StringRef.h"
+
 #include <cassert>
 #include <cstdio>
 #ifdef _MSC_VER

                                                 Val5, Val6);
 }
 
+/// This is a helper class used for left_justify() and right_justify().
+class FormattedString {
+  StringRef Str;
+  unsigned Width;
+  bool RightJustify;
+  friend class raw_ostream;
+public:
+    FormattedString(StringRef S, unsigned W, bool R)
+      : Str(S), Width(W), RightJustify(R) { }
+};
+
+/// left_justify - append spaces after string so total output is
+/// \p Width characters.  If \p Str is larger that \p Width, full string
+/// is written with no padding.
+inline FormattedString left_justify(StringRef Str, unsigned Width) {
+  return FormattedString(Str, Width, false);
+}
+
+/// right_justify - add spaces before string so total output is
+/// \p Width characters.  If \p Str is larger that \p Width, full string
+/// is written with no padding.
+inline FormattedString right_justify(StringRef Str, unsigned Width) {
+  return FormattedString(Str, Width, true);
+}
+
+/// This is a helper class used for format_hex() and format_decimal().
+class FormattedNumber {
+  uint64_t HexValue;
+  int64_t DecValue;
+  unsigned Width;
+  bool Hex;
+  bool Upper;
+  friend class raw_ostream;
+public:
+    FormattedNumber(uint64_t HV, int64_t DV, unsigned W, bool H, bool U)
+      : HexValue(HV), DecValue(DV), Width(W), Hex(H), Upper(U) { }
+};
+
+/// format_hex - Output \p N as a fixed width hexadecimal. If number will not
+/// fit in width, full number is still printed.  Examples:
+///   OS << format_hex(255, 4)        => 0xff
+///   OS << format_hex(255, 4, true)  => 0xFF
+///   OS << format_hex(255, 6)        => 0x00ff
+///   OS << format_hex(255, 2)        => 0xff
+inline FormattedNumber format_hex(uint64_t N, unsigned Width, bool Upper=false) {
+  assert(Width <= 18 && "hex width must be <= 18");
+  return FormattedNumber(N, 0, Width, true, Upper);
+}
+
+/// format_decimal - Output \p N as a right justified, fixed-width decimal. If 
+/// number will not fit in width, full number is still printed.  Examples:
+///   OS << format_decimal(0, 5)     => "    0"
+///   OS << format_decimal(255, 5)   => "  255"
+///   OS << format_decimal(-1, 3)    => " -1"
+///   OS << format_decimal(12345, 3) => "12345"
+inline FormattedNumber format_decimal(int64_t N, unsigned Width) {
+  return FormattedNumber(0, N, Width, false, false);
+}
+
+
 } // end namespace llvm
 
 #endif

include/llvm/Support/raw_ostream.h

 
 namespace llvm {
   class format_object_base;
+  class FormattedString;
+  class FormattedNumber;
   template <typename T>
   class SmallVectorImpl;
 

   // Formatted output, see the format() function in Support/Format.h.
   raw_ostream &operator<<(const format_object_base &Fmt);
 
+  // Formatted output, see the leftJustify() function in Support/Format.h.
+  raw_ostream &operator<<(const FormattedString &);
+  
+  // Formatted output, see the formatHex() function in Support/Format.h.
+  raw_ostream &operator<<(const FormattedNumber &);
+  
   /// indent - Insert 'NumSpaces' spaces.
   raw_ostream &indent(unsigned NumSpaces);
 

lib/Support/raw_ostream.cpp

 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/MathExtras.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
 #include <cctype>

   }
 }
 
+raw_ostream &raw_ostream::operator<<(const FormattedString &FS) {
+  unsigned Len = FS.Str.size(); 
+  int PadAmount = FS.Width - Len;
+  if (FS.RightJustify && (PadAmount > 0))
+    this->indent(PadAmount);
+  this->operator<<(FS.Str);
+  if (!FS.RightJustify && (PadAmount > 0))
+    this->indent(PadAmount);
+  return *this;
+}
+
+raw_ostream &raw_ostream::operator<<(const FormattedNumber &FN) {
+  if (FN.Hex) {
+    unsigned Nibbles = (64 - countLeadingZeros(FN.HexValue)+3)/4;
+    unsigned Width = (FN.Width > Nibbles+2) ? FN.Width : Nibbles+2;
+        
+    char NumberBuffer[20] = "0x0000000000000000";
+    char *EndPtr = NumberBuffer+Width;
+    char *CurPtr = EndPtr;
+    const char A = FN.Upper ? 'A' : 'a';
+    unsigned long long N = FN.HexValue;
+    while (N) {
+      uintptr_t x = N % 16;
+      *--CurPtr = (x < 10 ? '0' + x : A + x - 10);
+      N /= 16;
+    }
+
+    return write(NumberBuffer, Width);
+  } else {
+    // Zero is a special case.
+    if (FN.DecValue == 0) {
+      this->indent(FN.Width-1);
+      return *this << '0';
+    }
+    char NumberBuffer[32];
+    char *EndPtr = NumberBuffer+sizeof(NumberBuffer);
+    char *CurPtr = EndPtr;
+    bool Neg = (FN.DecValue < 0);
+    uint64_t N = Neg ? -FN.DecValue : FN.DecValue;
+    while (N) {
+      *--CurPtr = '0' + char(N % 10);
+      N /= 10;
+    }
+    int Len = EndPtr - CurPtr;
+    int Pad = FN.Width - Len;
+    if (Neg) 
+      --Pad;
+    if (Pad > 0)
+      this->indent(Pad);
+    if (Neg)
+      *this << '-';
+    return write(CurPtr, Len);
+  }
+}
+
+
 /// indent - Insert 'NumSpaces' spaces.
 raw_ostream &raw_ostream::indent(unsigned NumSpaces) {
   static const char Spaces[] = "                                "

unittests/Support/raw_ostream_test.cpp

   EXPECT_EQ("\\001\\010\\200", Str);
 }
 
+TEST(raw_ostreamTest, Justify) {  
+  EXPECT_EQ("xyz   ", printToString(left_justify("xyz", 6), 6));
+  EXPECT_EQ("abc",    printToString(left_justify("abc", 3), 3));
+  EXPECT_EQ("big",    printToString(left_justify("big", 1), 3));
+  EXPECT_EQ("   xyz", printToString(right_justify("xyz", 6), 6));
+  EXPECT_EQ("abc",    printToString(right_justify("abc", 3), 3));
+  EXPECT_EQ("big",    printToString(right_justify("big", 1), 3));
 }
+
+TEST(raw_ostreamTest, FormatHex) {  
+  EXPECT_EQ("0x1234",     printToString(format_hex(0x1234, 6), 6));
+  EXPECT_EQ("0x001234",   printToString(format_hex(0x1234, 8), 8));
+  EXPECT_EQ("0x00001234", printToString(format_hex(0x1234, 10), 10));
+  EXPECT_EQ("0x1234",     printToString(format_hex(0x1234, 4), 6));
+  EXPECT_EQ("0xff",       printToString(format_hex(255, 4), 4));
+  EXPECT_EQ("0xFF",       printToString(format_hex(255, 4, true), 4));
+  EXPECT_EQ("0x1",        printToString(format_hex(1, 3), 3));
+  EXPECT_EQ("0x12",       printToString(format_hex(0x12, 3), 4));
+  EXPECT_EQ("0x123",      printToString(format_hex(0x123, 3), 5));
+  EXPECT_EQ("0xffffffffffffffff",     
+                          printToString(format_hex(UINT64_MAX, 18), 18));
+  EXPECT_EQ("0x8000000000000000",     
+                          printToString(format_hex((INT64_MIN), 18), 18));
+}
+
+TEST(raw_ostreamTest, FormatDecimal) {  
+  EXPECT_EQ("   0",        printToString(format_decimal(0, 4), 4));
+  EXPECT_EQ("  -1",        printToString(format_decimal(-1, 4), 4));
+  EXPECT_EQ("    -1",      printToString(format_decimal(-1, 6), 6));
+  EXPECT_EQ("1234567890",  printToString(format_decimal(1234567890, 10), 10));
+  EXPECT_EQ("  9223372036854775807", 
+                          printToString(format_decimal(INT64_MAX, 21), 21));
+  EXPECT_EQ(" -9223372036854775808", 
+                          printToString(format_decimal(INT64_MIN, 21), 21));
+}
+
+
+}