From e825156add816c776eae1b7d279746a669cbf390 Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Tue, 8 Jul 2014 16:20:33 -0700 Subject: [PATCH] Move FormatParser to the internal namespace. --- format.cc | 278 +++++++++++++++++++++++--------------------- format.h | 76 +++++++----- test/format-test.cc | 8 +- test/util-test.cc | 3 +- 4 files changed, 194 insertions(+), 171 deletions(-) diff --git a/format.cc b/format.cc index 9faebf47..0c2946f7 100644 --- a/format.cc +++ b/format.cc @@ -154,6 +154,19 @@ int ParseNonnegativeInt(const Char *&s, const char *&error) FMT_NOEXCEPT(true) { } return value; } + +template +const Char *find_closing_brace(const Char *s, int num_open_braces = 1) { + for (int n = num_open_braces; *s; ++s) { + if (*s == '{') { + ++n; + } else if (*s == '}') { + if (--n == 0) + return s; + } + } + throw fmt::FormatError("unmatched '{' in format"); +} } // namespace int fmt::internal::SignBitNoInline(double value) { return SignBit(value); } @@ -357,15 +370,8 @@ void fmt::internal::FormatWinErrorMessage( template void fmt::internal::FormatErrorReporter::operator()( const Char *s, fmt::StringRef message) const { - for (int n = num_open_braces; *s; ++s) { - if (*s == '{') { - ++n; - } else if (*s == '}') { - if (--n == 0) - throw fmt::FormatError(message); - } - } - throw fmt::FormatError("unmatched '{' in format"); + if (find_closing_brace(s, num_open_braces)) + throw fmt::FormatError(message); } // Fills the padding around the content and returns the pointer to the @@ -553,7 +559,7 @@ void fmt::BasicWriter::write_str( template inline const Arg - &fmt::BasicWriter::FormatParser::ParseArgIndex(const Char *&s) { + &fmt::internal::FormatParser::ParseArgIndex(const Char *&s) { unsigned arg_index = 0; if (*s < '0' || *s > '9') { if (*s != '}' && *s != ':') @@ -580,7 +586,7 @@ inline const Arg } template -void fmt::BasicWriter::FormatParser::CheckSign( +void fmt::internal::FormatParser::CheckSign( const Char *&s, const Arg &arg) { char sign = static_cast(*s); if (arg.type > Arg::LAST_NUMERIC_TYPE) { @@ -851,7 +857,7 @@ void fmt::internal::PrintfParser::Format( case Arg::CUSTOM: if (spec.type_) internal::ReportUnknownType(spec.type_, "object"); - arg.custom.format(&writer, arg.custom.value, spec); + arg.custom.format(&writer, arg.custom.value, "s"); break; default: assert(false); @@ -862,39 +868,23 @@ void fmt::internal::PrintfParser::Format( } template -void fmt::BasicWriter::FormatParser::Format( - BasicWriter &writer, BasicStringRef format, - const ArgList &args) { +const Char *fmt::internal::FormatParser::format( + const Char *format_str, const internal::Arg &arg) { + const Char *s = format_str; const char *error = 0; - const Char *start = format.c_str(); - args_ = args; - next_arg_index_ = 0; - const Char *s = start; - while (*s) { - Char c = *s++; - if (c != '{' && c != '}') continue; - if (*s == c) { - writer.buffer_.append(start, s); - start = ++s; - continue; + FormatSpec spec; + if (*s == ':') { + if (arg.type == Arg::CUSTOM) { + arg.custom.format(this, arg.custom.value, s); + return find_closing_brace(s) + 1; } - if (c == '}') - throw FormatError("unmatched '}' in format"); - report_error_.num_open_braces = 1; - writer.buffer_.append(start, s - 1); - - const Arg &arg = ParseArgIndex(s); - - FormatSpec spec; - if (*s == ':') { - ++s; - - // Parse fill and alignment. - if (Char c = *s) { - const Char *p = s + 1; - spec.align_ = ALIGN_DEFAULT; - do { - switch (*p) { + ++s; + // Parse fill and alignment. + if (Char c = *s) { + const Char *p = s + 1; + spec.align_ = ALIGN_DEFAULT; + do { + switch (*p) { case '<': spec.align_ = ALIGN_LEFT; break; @@ -907,24 +897,24 @@ void fmt::BasicWriter::FormatParser::Format( case '^': spec.align_ = ALIGN_CENTER; break; - } - if (spec.align_ != ALIGN_DEFAULT) { - if (p != s) { - if (c == '}') break; - if (c == '{') - report_error_(s, "invalid fill character '{'"); - s += 2; - spec.fill_ = c; - } else ++s; - if (spec.align_ == ALIGN_NUMERIC && arg.type > Arg::LAST_NUMERIC_TYPE) - report_error_(s, "format specifier '=' requires numeric argument"); - break; - } - } while (--p >= s); - } + } + if (spec.align_ != ALIGN_DEFAULT) { + if (p != s) { + if (c == '}') break; + if (c == '{') + report_error_(s, "invalid fill character '{'"); + s += 2; + spec.fill_ = c; + } else ++s; + if (spec.align_ == ALIGN_NUMERIC && arg.type > Arg::LAST_NUMERIC_TYPE) + report_error_(s, "format specifier '=' requires numeric argument"); + break; + } + } while (--p >= s); + } - // Parse sign. - switch (*s) { + // Parse sign. + switch (*s) { case '+': CheckSign(s, arg); spec.flags_ |= SIGN_FLAG | PLUS_FLAG; @@ -936,44 +926,44 @@ void fmt::BasicWriter::FormatParser::Format( CheckSign(s, arg); spec.flags_ |= SIGN_FLAG; break; - } + } - if (*s == '#') { + if (*s == '#') { + if (arg.type > Arg::LAST_NUMERIC_TYPE) + report_error_(s, "format specifier '#' requires numeric argument"); + spec.flags_ |= HASH_FLAG; + ++s; + } + + // Parse width and zero flag. + if ('0' <= *s && *s <= '9') { + if (*s == '0') { if (arg.type > Arg::LAST_NUMERIC_TYPE) - report_error_(s, "format specifier '#' requires numeric argument"); - spec.flags_ |= HASH_FLAG; - ++s; + report_error_(s, "format specifier '0' requires numeric argument"); + spec.align_ = ALIGN_NUMERIC; + spec.fill_ = '0'; } + // Zero may be parsed again as a part of the width, but it is simpler + // and more efficient than checking if the next char is a digit. + spec.width_ = ParseNonnegativeInt(s, error); + if (error) + report_error_(s, error); + } - // Parse width and zero flag. + // Parse precision. + if (*s == '.') { + ++s; + spec.precision_ = 0; if ('0' <= *s && *s <= '9') { - if (*s == '0') { - if (arg.type > Arg::LAST_NUMERIC_TYPE) - report_error_(s, "format specifier '0' requires numeric argument"); - spec.align_ = ALIGN_NUMERIC; - spec.fill_ = '0'; - } - // Zero may be parsed again as a part of the width, but it is simpler - // and more efficient than checking if the next char is a digit. - spec.width_ = ParseNonnegativeInt(s, error); + spec.precision_ = ParseNonnegativeInt(s, error); if (error) report_error_(s, error); - } - - // Parse precision. - if (*s == '.') { + } else if (*s == '{') { ++s; - spec.precision_ = 0; - if ('0' <= *s && *s <= '9') { - spec.precision_ = ParseNonnegativeInt(s, error); - if (error) - report_error_(s, error); - } else if (*s == '{') { - ++s; - ++report_error_.num_open_braces; - const Arg &precision_arg = ParseArgIndex(s); - ULongLong value = 0; - switch (precision_arg.type) { + ++report_error_.num_open_braces; + const Arg &precision_arg = ParseArgIndex(s); + ULongLong value = 0; + switch (precision_arg.type) { case Arg::INT: if (precision_arg.int_value < 0) report_error_(s, "negative precision in format"); @@ -992,50 +982,50 @@ void fmt::BasicWriter::FormatParser::Format( break; default: report_error_(s, "precision is not integer"); - } - if (value > INT_MAX) - report_error_(s, "number is too big in format"); - spec.precision_ = static_cast(value); - if (*s++ != '}') - throw FormatError("unmatched '{' in format"); - --report_error_.num_open_braces; - } else { - report_error_(s, "missing precision in format"); - } - if (arg.type != Arg::DOUBLE && arg.type != Arg::LONG_DOUBLE) { - report_error_(s, - "precision specifier requires floating-point argument"); } + if (value > INT_MAX) + report_error_(s, "number is too big in format"); + spec.precision_ = static_cast(value); + if (*s++ != '}') + throw FormatError("unmatched '{' in format"); + --report_error_.num_open_braces; + } else { + report_error_(s, "missing precision in format"); + } + if (arg.type != Arg::DOUBLE && arg.type != Arg::LONG_DOUBLE) { + report_error_(s, + "precision specifier requires floating-point argument"); } - - // Parse type. - if (*s != '}' && *s) - spec.type_ = static_cast(*s++); } - if (*s++ != '}') - throw FormatError("unmatched '{' in format"); - start = s; + // Parse type. + if (*s != '}' && *s) + spec.type_ = static_cast(*s++); + } - // Format argument. - switch (arg.type) { + if (*s++ != '}') + throw FormatError("unmatched '{' in format"); + start_ = s; + + // Format argument. + switch (arg.type) { case Arg::INT: - writer.FormatInt(arg.int_value, spec); + writer_.FormatInt(arg.int_value, spec); break; case Arg::UINT: - writer.FormatInt(arg.uint_value, spec); + writer_.FormatInt(arg.uint_value, spec); break; case Arg::LONG_LONG: - writer.FormatInt(arg.long_long_value, spec); + writer_.FormatInt(arg.long_long_value, spec); break; case Arg::ULONG_LONG: - writer.FormatInt(arg.ulong_long_value, spec); + writer_.FormatInt(arg.ulong_long_value, spec); break; case Arg::DOUBLE: - writer.FormatDouble(arg.double_value, spec); + writer_.FormatDouble(arg.double_value, spec); break; case Arg::LONG_DOUBLE: - writer.FormatDouble(arg.long_double_value, spec); + writer_.FormatDouble(arg.long_double_value, spec); break; case Arg::CHAR: { if (spec.type_ && spec.type_ != 'c') @@ -1044,45 +1034,66 @@ void fmt::BasicWriter::FormatParser::Format( CharPtr out = CharPtr(); if (spec.width_ > 1) { Char fill = static_cast(spec.fill()); - out = writer.GrowBuffer(spec.width_); + out = writer_.GrowBuffer(spec.width_); if (spec.align_ == ALIGN_RIGHT) { std::fill_n(out, spec.width_ - 1, fill); out += spec.width_ - 1; } else if (spec.align_ == ALIGN_CENTER) { - out = writer.FillPadding(out, spec.width_, 1, fill); + out = writer_.FillPadding(out, spec.width_, 1, fill); } else { std::fill_n(out + 1, spec.width_ - 1, fill); } } else { - out = writer.GrowBuffer(1); + out = writer_.GrowBuffer(1); } *out = static_cast(arg.int_value); break; } case Arg::STRING: - writer.write_str(arg.string, spec); + writer_.write_str(arg.string, spec); break; case Arg::WSTRING: - writer.write_str(internal::CharTraits::convert(arg.wstring), spec); + writer_.write_str(internal::CharTraits::convert(arg.wstring), spec); break; case Arg::POINTER: if (spec.type_ && spec.type_ != 'p') internal::ReportUnknownType(spec.type_, "pointer"); spec.flags_= HASH_FLAG; spec.type_ = 'x'; - writer.FormatInt(reinterpret_cast(arg.pointer_value), spec); + writer_.FormatInt(reinterpret_cast(arg.pointer_value), spec); break; case Arg::CUSTOM: - if (spec.type_) - internal::ReportUnknownType(spec.type_, "object"); - arg.custom.format(&writer, arg.custom.value, spec); + arg.custom.format(this, arg.custom.value, s - 1); break; default: assert(false); break; - } } - writer.buffer_.append(start, s); + return s; +} + +template +void fmt::internal::FormatParser::Format( + BasicStringRef format_str, const ArgList &args) { + const Char *s = start_ = format_str.c_str(); + args_ = args; + next_arg_index_ = 0; + while (*s) { + Char c = *s++; + if (c != '{' && c != '}') continue; + if (*s == c) { + writer_.buffer_.append(start_, s); + start_ = ++s; + continue; + } + if (c == '}') + throw FormatError("unmatched '}' in format"); + report_error_.num_open_braces = 1; + writer_.buffer_.append(start_, s - 1); + Arg arg = ParseArgIndex(s); + s = format(s, arg); + } + writer_.buffer_.append(start_, s); } void fmt::ReportSystemError( @@ -1131,8 +1142,8 @@ template fmt::BasicWriter::CharPtr fmt::BasicWriter::FillPadding(CharPtr buffer, unsigned total_size, std::size_t content_size, wchar_t fill); -template void fmt::BasicWriter::FormatParser::Format( - BasicWriter &writer, BasicStringRef format, const ArgList &args); +template void fmt::internal::FormatParser::Format( + BasicStringRef format, const ArgList &args); template void fmt::internal::PrintfParser::Format( BasicWriter &writer, BasicStringRef format, const ArgList &args); @@ -1143,9 +1154,8 @@ template fmt::BasicWriter::CharPtr fmt::BasicWriter::FillPadding(CharPtr buffer, unsigned total_size, std::size_t content_size, wchar_t fill); -template void fmt::BasicWriter::FormatParser::Format( - BasicWriter &writer, BasicStringRef format, - const ArgList &args); +template void fmt::internal::FormatParser::Format( + BasicStringRef format, const ArgList &args); template void fmt::internal::PrintfParser::Format( BasicWriter &writer, BasicStringRef format, diff --git a/format.h b/format.h index b5af5135..0d7a6ff9 100644 --- a/format.h +++ b/format.h @@ -130,8 +130,13 @@ typedef BasicWriter WWriter; struct FormatSpec; +namespace internal { +template +class FormatParser; +} + template -void format(BasicWriter &w, const FormatSpec &spec, const T &value); +void format(internal::FormatParser &f, const Char *format_str, const T &value); /** \rst @@ -598,7 +603,7 @@ struct Arg { Type type; typedef void (*FormatFunc)( - void *writer, const void *arg, const FormatSpec &spec); + void *formatter, const void *arg, const void *format_str); struct CustomValue { const void *value; @@ -648,9 +653,9 @@ class MakeArg : public Arg { // Formats an argument of a custom type, such as a user-defined class. template static void format_custom_arg( - void *writer, const void *arg, const FormatSpec &spec) { - format(*static_cast*>(writer), - spec, *static_cast(arg)); + void *formatter, const void *arg, const void *format_str) { + format(*static_cast*>(formatter), + static_cast(format_str), *static_cast(arg)); } public: @@ -747,6 +752,32 @@ class ArgList { }; namespace internal { +// Format string parser. +// TODO: rename to Formatter +template +class FormatParser { +private: + BasicWriter &writer_; + ArgList args_; + int next_arg_index_; + const Char *start_; + fmt::internal::FormatErrorReporter report_error_; + + // Parses argument index and returns an argument with this index. + const internal::Arg &ParseArgIndex(const Char *&s); + + void CheckSign(const Char *&s, const internal::Arg &arg); + +public: + explicit FormatParser(BasicWriter &w) : writer_(w) {} + + BasicWriter &writer() { return writer_; } + + void Format(BasicStringRef format_str, const ArgList &args); + + const Char *format(const Char *format_str, const internal::Arg &arg); +}; + // Printf format string parser. template class PrintfParser { @@ -1225,23 +1256,7 @@ class BasicWriter { // Do not implement! void operator<<(typename internal::CharTraits::UnsupportedStrType); - // Format string parser. - class FormatParser { - private: - ArgList args_; - int next_arg_index_; - fmt::internal::FormatErrorReporter report_error_; - - // Parses argument index and returns an argument with this index. - const internal::Arg &ParseArgIndex(const Char *&s); - - void CheckSign(const Char *&s, const internal::Arg &arg); - - public: - void Format(BasicWriter &writer, - BasicStringRef format, const ArgList &args); - }; - + friend class internal::FormatParser; friend class internal::PrintfParser; public: @@ -1321,8 +1336,8 @@ class BasicWriter { See also `Format String Syntax`_. \endrst */ - inline void write(BasicStringRef format, const ArgList &args) { - FormatParser().Format(*this, format, args); + void write(BasicStringRef format, const ArgList &args) { + internal::FormatParser(*this).Format(format, args); } FMT_VARIADIC_VOID(write, fmt::BasicStringRef) @@ -1396,14 +1411,11 @@ class BasicWriter { template BasicWriter &operator<<(const StrFormatSpec &spec) { const StringChar *s = spec.str(); + // TODO: error if fill is not convertible to Char write_str(s, std::char_traits::length(s), spec); return *this; } - void write_str(const std::basic_string &s, const FormatSpec &spec) { - write_str(s.data(), s.size(), spec); - } - void clear() { buffer_.clear(); } }; @@ -1469,7 +1481,6 @@ typename fmt::BasicWriter::CharPtr } CharPtr p = GrowBuffer(width); CharPtr end = p + width; - // TODO: error if fill is not convertible to Char if (align == ALIGN_LEFT) { std::copy(prefix, prefix + prefix_size, p); p += size; @@ -1575,12 +1586,13 @@ void BasicWriter::FormatInt(T value, const Spec &spec) { } } -// The default formatting function. +// Formats a value. template -void format(BasicWriter &w, const FormatSpec &spec, const T &value) { +void format(internal::FormatParser &f, + const Char *format_str, const T &value) { std::basic_ostringstream os; os << value; - w.write_str(os.str(), spec); + f.format(format_str, internal::MakeArg(os.str())); } // Reports a system error without throwing an exception. diff --git a/test/format-test.cc b/test/format-test.cc index b8ec32e8..8318d973 100644 --- a/test/format-test.cc +++ b/test/format-test.cc @@ -519,7 +519,7 @@ TEST(WriterTest, pad) { EXPECT_EQ(" 33", (Writer() << pad(33ll, 7)).str()); EXPECT_EQ(" 44", (Writer() << pad(44ull, 7)).str()); - BasicWriter w; + Writer w; w.clear(); w << pad(42, 5, '0'); EXPECT_EQ("00042", w.str()); @@ -1312,14 +1312,14 @@ TEST(FormatterTest, FormatUsingIOStreams) { std::string s = format("The date is {0}", Date(2012, 12, 9)); EXPECT_EQ("The date is 2012-12-9", s); Date date(2012, 12, 9); - CheckUnknownTypes(date, "", "object"); + CheckUnknownTypes(date, "s", "string"); } class Answer {}; template -void format(BasicWriter &w, const fmt::FormatSpec &spec, Answer) { - w.write_str("42", spec); +void format(fmt::internal::FormatParser &f, const Char *, Answer) { + f.writer() << "42"; } TEST(FormatterTest, CustomFormat) { diff --git a/test/util-test.cc b/test/util-test.cc index 39fac2a9..d07fd192 100644 --- a/test/util-test.cc +++ b/test/util-test.cc @@ -185,7 +185,8 @@ TEST(UtilTest, MakeArg) { EXPECT_EQ(fmt::internal::Arg::CUSTOM, arg.type); arg.custom.value = &t; fmt::Writer w; - arg.custom.format(&w, &t, fmt::FormatSpec()); + fmt::internal::FormatParser formatter(w); + arg.custom.format(&formatter, &t, "}"); EXPECT_EQ("test", w.str()); }