From 095b43a8f03d447ddb3bba2db10703aa19173835 Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Sun, 9 Dec 2012 11:32:39 -0800 Subject: [PATCH] Test and correct handling of the width specifier. --- format.cc | 55 +++++++++++++++------------------- format.h | 27 ++++++++++------- format_test.cc | 80 +++++++++++++++++++++++++++++++++++--------------- 3 files changed, 97 insertions(+), 65 deletions(-) diff --git a/format.cc b/format.cc index a3374ef5..4548c0d0 100644 --- a/format.cc +++ b/format.cc @@ -39,20 +39,6 @@ unsigned ParseUInt(const char *&s) { } while ('0' <= *s && *s <= '9'); return value; } - -// Flags. -enum { - PLUS_FLAG = 1, - ZERO_FLAG = 2 -}; - -void CheckFlags(unsigned flags) { - if (flags == 0) return; - if ((flags & PLUS_FLAG) != 0) - throw fmt::FormatError("format specifier '+' used with non-numeric type"); - if ((flags & ZERO_FLAG) != 0) - throw fmt::FormatError("format specifier '0' used with non-numeric type"); -} } template @@ -94,33 +80,42 @@ void fmt::Formatter::Format() { unsigned arg_index = ParseUInt(s); if (arg_index >= args_.size()) Throw(s, "argument index is out of range in format"); + Arg &arg = args_[arg_index]; - enum { MAX_FORMAT_SIZE = 9}; // longest format: %+0*.*ld + enum { MAX_FORMAT_SIZE = 10}; // longest format: %+0-*.*ld char arg_format[MAX_FORMAT_SIZE]; char *arg_format_ptr = arg_format; *arg_format_ptr++ = '%'; - unsigned flags = 0; int width = -1; int precision = -1; char type = 0; if (*s == ':') { ++s; if (*s == '+') { - flags |= PLUS_FLAG; + if (arg.type > LAST_NUMERIC_TYPE) { + Throw(s, + "format specifier '+' used with non-numeric type"); + } *arg_format_ptr++ = *s++; } if (*s == '0') { - flags |= ZERO_FLAG; + if (arg.type > LAST_NUMERIC_TYPE) { + Throw(s, + "format specifier '0' used with non-numeric type"); + } *arg_format_ptr++ = *s++; } // Parse width. if ('0' <= *s && *s <= '9') { + if (arg.type > LAST_NUMERIC_TYPE) + *arg_format_ptr++ = '-'; *arg_format_ptr++ = '*'; - unsigned number = ParseUInt(s); - if (number > INT_MAX) ; // TODO: error - width = number; + unsigned value = ParseUInt(s); + if (value > INT_MAX) + Throw(s, "number is too big in format"); + width = value; } // Parse precision. @@ -130,10 +125,10 @@ void fmt::Formatter::Format() { ++s; precision = 0; if ('0' <= *s && *s <= '9') { - do { - // TODO: check overflow - precision = precision * 10 + (*s++ - '0'); - } while ('0' <= *s && *s <= '9'); + unsigned value = ParseUInt(s); + if (value > INT_MAX) + Throw(s, "number is too big in format"); // TODO: test + precision = value; } else { // TODO: error } @@ -149,10 +144,8 @@ void fmt::Formatter::Format() { start = s; // Format argument. - Arg &arg = args_[arg_index]; switch (arg.type) { case CHAR: - CheckFlags(flags); if (width == -1 && precision == -1) { buffer_.push_back(arg.int_value); break; @@ -195,12 +188,13 @@ void fmt::Formatter::Format() { FormatBuiltinArg(arg_format, arg.long_double_value, width, precision); break; case STRING: - CheckFlags(flags); + // TODO: align string left by default if (width == -1 && precision == -1) { const char *str = arg.string_value; std::size_t size = arg.size; if (size == 0 && *str) size = std::strlen(str); + buffer_.reserve(buffer_.size() + size + 1); buffer_.insert(buffer_.end(), str, str + size); break; } @@ -209,21 +203,18 @@ void fmt::Formatter::Format() { FormatBuiltinArg(arg_format, arg.string_value, width, precision); break; case WSTRING: - CheckFlags(flags); *arg_format_ptr++ = 'l'; *arg_format_ptr++ = 's'; *arg_format_ptr = '\0'; FormatBuiltinArg(arg_format, arg.wstring_value, width, precision); break; case POINTER: - CheckFlags(flags); *arg_format_ptr++ = 'p'; *arg_format_ptr = '\0'; FormatBuiltinArg(arg_format, arg.pointer_value, width, precision); break; case CUSTOM: - CheckFlags(flags); - (this->*arg.format)(arg.custom_value); + (this->*arg.format)(arg.custom_value, width); break; default: assert(false); diff --git a/format.h b/format.h index 0f9cfcb4..cfc21ea4 100644 --- a/format.h +++ b/format.h @@ -32,10 +32,13 @@ class Formatter { std::vector buffer_; // Output buffer. enum Type { - CHAR, INT, UINT, LONG, ULONG, DOUBLE, LONG_DOUBLE, - STRING, WSTRING, POINTER, CUSTOM + // Numeric types should go first. + INT, UINT, LONG, ULONG, DOUBLE, LONG_DOUBLE, POINTER, + LAST_NUMERIC_TYPE = POINTER, CHAR, STRING, WSTRING, CUSTOM }; + typedef void (Formatter::*FormatFunc)(const void *arg, int width); + // An argument. struct Arg { Type type; @@ -56,7 +59,7 @@ class Formatter { }; struct { const void *custom_value; - void (Formatter::*format)(const void *value); + FormatFunc format; }; }; @@ -72,8 +75,8 @@ class Formatter { : type(STRING), string_value(value), size(size) {} explicit Arg(const wchar_t *value) : type(WSTRING), wstring_value(value) {} explicit Arg(const void *value) : type(POINTER), pointer_value(value) {} - explicit Arg(const void *value, void (Formatter::*format)(const void *)) - : type(CUSTOM), custom_value(value), format(format) {} + explicit Arg(const void *value, FormatFunc f) + : type(CUSTOM), custom_value(value), format(f) {} }; std::vector args_; @@ -95,7 +98,7 @@ class Formatter { // Formats an argument of a custom type, such as a user-defined class. template - void FormatCustomArg(const void *arg); + void FormatCustomArg(const void *arg, int width); void Format(); @@ -244,14 +247,18 @@ class ArgFormatterWithCallback : public ArgFormatter { }; template -void Formatter::FormatCustomArg(const void *arg) { +void Formatter::FormatCustomArg(const void *arg, int width) { const T &value = *static_cast(arg); std::ostringstream os; os << value; std::string str(os.str()); - // Extra char is reserved for terminating '\0'. - buffer_.reserve(buffer_.size() + str.size() + 1); - buffer_.insert(buffer_.end(), str.begin(), str.end()); + if (width < 0) { + // Extra char is reserved for terminating '\0'. + buffer_.reserve(buffer_.size() + str.size() + 1); + buffer_.insert(buffer_.end(), str.begin(), str.end()); + return; + } + FormatBuiltinArg("%-*s", str.c_str(), width, -1); } inline ArgFormatter Formatter::operator()(const char *format) { diff --git a/format_test.cc b/format_test.cc index e8c57528..4652317c 100644 --- a/format_test.cc +++ b/format_test.cc @@ -66,12 +66,12 @@ class TestString { } }; -TEST(FormatterTest, FormatNoArgs) { +TEST(FormatterTest, NoArgs) { EXPECT_EQ("abracadabra", str(Format("{0}{1}{0}") << "abra" << "cad")); EXPECT_EQ("test", str(Format("test"))); } -TEST(FormatterTest, FormatArgs) { +TEST(FormatterTest, Args) { EXPECT_EQ("42", str(Format("{0}") << 42)); EXPECT_EQ("before 42", str(Format("before {0}") << 42)); EXPECT_EQ("42 after", str(Format("{0} after") << 42)); @@ -81,7 +81,7 @@ TEST(FormatterTest, FormatArgs) { EXPECT_EQ("abracadabra", str(Format("{0}{1}{0}") << "abra" << "cad")); } -TEST(FormatterTest, FormatArgErrors) { +TEST(FormatterTest, ArgErrors) { EXPECT_THROW_MSG(Format("{"), FormatError, "unmatched '{' in format"); EXPECT_THROW_MSG(Format("{}"), FormatError, "missing argument index in format string"); @@ -109,57 +109,87 @@ TEST(FormatterTest, FormatArgErrors) { } } -TEST(FormatterTest, FormatPlusFlag) { +TEST(FormatterTest, EmptySpecs) { + EXPECT_EQ("42", str(Format("{0:}") << 42)); +} + +TEST(FormatterTest, PlusFlag) { EXPECT_EQ("+42", str(Format("{0:+}") << 42)); EXPECT_EQ("-42", str(Format("{0:+}") << -42)); - EXPECT_THROW_MSG(Format("{0:+") << 'c', - FormatError, "unmatched '{' in format"); - EXPECT_THROW_MSG(Format("{0:+}") << 'c', - FormatError, "format specifier '+' used with non-numeric type"); EXPECT_EQ("+42", str(Format("{0:+}") << 42)); EXPECT_EQ("+42", str(Format("{0:+}") << 42u)); EXPECT_EQ("+42", str(Format("{0:+}") << 42l)); EXPECT_EQ("+42", str(Format("{0:+}") << 42ul)); EXPECT_EQ("+42", str(Format("{0:+}") << 42.0)); EXPECT_EQ("+42", str(Format("{0:+}") << 42.0l)); + EXPECT_EQ("+0x42", + str(Format("{0:+}") << reinterpret_cast(0x42))); + EXPECT_THROW_MSG(Format("{0:+") << 'c', + FormatError, "unmatched '{' in format"); + EXPECT_THROW_MSG(Format("{0:+}") << 'c', + FormatError, "format specifier '+' used with non-numeric type"); EXPECT_THROW_MSG(Format("{0:+}") << "abc", FormatError, "format specifier '+' used with non-numeric type"); EXPECT_THROW_MSG(Format("{0:+}") << L"abc", FormatError, "format specifier '+' used with non-numeric type"); - EXPECT_THROW_MSG(Format("{0:+}") << static_cast("abc"), - FormatError, "format specifier '+' used with non-numeric type"); EXPECT_THROW_MSG(Format("{0:+}") << TestString(), FormatError, "format specifier '+' used with non-numeric type"); } -TEST(FormatterTest, FormatZeroFlag) { +TEST(FormatterTest, ZeroFlag) { EXPECT_EQ("42", str(Format("{0:0}") << 42)); - EXPECT_THROW_MSG(Format("{0:0") << 'c', - FormatError, "unmatched '{' in format"); - EXPECT_THROW_MSG(Format("{0:05}") << 'c', - FormatError, "format specifier '0' used with non-numeric type"); EXPECT_EQ("-0042", str(Format("{0:05}") << -42)); EXPECT_EQ("00042", str(Format("{0:05}") << 42u)); EXPECT_EQ("-0042", str(Format("{0:05}") << -42l)); EXPECT_EQ("00042", str(Format("{0:05}") << 42ul)); EXPECT_EQ("-0042", str(Format("{0:05}") << -42.0)); EXPECT_EQ("-0042", str(Format("{0:05}") << -42.0l)); + EXPECT_EQ("0x0042", + str(Format("{0:06}") << reinterpret_cast(0x42))); + EXPECT_THROW_MSG(Format("{0:0") << 'c', + FormatError, "unmatched '{' in format"); + EXPECT_THROW_MSG(Format("{0:05}") << 'c', + FormatError, "format specifier '0' used with non-numeric type"); EXPECT_THROW_MSG(Format("{0:05}") << "abc", FormatError, "format specifier '0' used with non-numeric type"); EXPECT_THROW_MSG(Format("{0:05}") << L"abc", FormatError, "format specifier '0' used with non-numeric type"); - EXPECT_THROW_MSG(Format("{0:05}") << static_cast("abc"), - FormatError, "format specifier '0' used with non-numeric type"); EXPECT_THROW_MSG(Format("{0:05}") << TestString(), FormatError, "format specifier '0' used with non-numeric type"); } -TEST(FormatterTest, FormatWidth) { - // TODO -} - -TEST(FormatterTest, FormatChar) { - EXPECT_EQ("a*b", str(Format("{0}{1}{2}") << 'a' << '*' << 'b')); +TEST(FormatterTest, Width) { + char format[256]; + if (ULONG_MAX > UINT_MAX) { + std::sprintf(format, "{0:%lu", INT_MAX + 1l); + EXPECT_THROW_MSG(Format(format), FormatError, "unmatched '{' in format"); + std::sprintf(format, "{0:%lu}", UINT_MAX + 1l); + EXPECT_THROW_MSG(Format(format) << 0, + FormatError, "number is too big in format"); + } else { + std::sprintf(format, "{0:%u0", UINT_MAX); + EXPECT_THROW_MSG(Format(format), FormatError, "unmatched '{' in format"); + std::sprintf(format, "{0:%u0}", UINT_MAX); + EXPECT_THROW_MSG(Format(format) << 0, + FormatError, "number is too big in format"); + } + std::sprintf(format, "{0:%u", INT_MAX + 1u); + EXPECT_THROW_MSG(Format(format), FormatError, "unmatched '{' in format"); + std::sprintf(format, "{0:%u}", INT_MAX + 1u); + EXPECT_THROW_MSG(Format(format) << 0, + FormatError, "number is too big in format"); + EXPECT_EQ(" -42", str(Format("{0:4}") << -42)); + EXPECT_EQ(" 42", str(Format("{0:5}") << 42u)); + EXPECT_EQ(" -42", str(Format("{0:6}") << -42l)); + EXPECT_EQ(" 42", str(Format("{0:7}") << 42lu)); + EXPECT_EQ(" -1.23", str(Format("{0:8}") << -1.23)); + EXPECT_EQ(" -1.23", str(Format("{0:9}") << -1.23l)); + EXPECT_EQ(" 0xcafe", + str(Format("{0:10}") << reinterpret_cast(0xcafe))); + EXPECT_EQ("x ", str(Format("{0:11}") << 'x')); + EXPECT_EQ("narrow ", str(Format("{0:12}") << "narrow")); + EXPECT_EQ("wide ", str(Format("{0:13}") << L"wide")); + EXPECT_EQ("test ", str(Format("{0:14}") << TestString("test"))); } TEST(FormatterTest, FormatInt) { @@ -173,6 +203,10 @@ TEST(FormatterTest, FormatInt) { EXPECT_EQ(os.str(), str(Format("{0}") << INT_MAX)); } +TEST(FormatterTest, FormatChar) { + EXPECT_EQ("a*b", str(Format("{0}{1}{2}") << 'a' << '*' << 'b')); +} + TEST(FormatterTest, FormatString) { EXPECT_EQ("test", str(Format("{0}") << std::string("test"))); }