From 217e7c76f10218e643b5b232a56f9caaf5ea0014 Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Sun, 14 Jan 2018 07:19:23 -0800 Subject: [PATCH] Pass ranges by value --- include/fmt/core.h | 73 +++++++++++++++++++--------- include/fmt/format.cc | 6 +-- include/fmt/format.h | 91 +++++++++++++++-------------------- include/fmt/printf.h | 58 ++++++++++++---------- include/fmt/string.h | 2 +- include/fmt/time.h | 2 +- test/custom-formatter-test.cc | 16 +++--- test/format-test.cc | 41 ++++++++-------- test/ostream-test.cc | 6 ++- test/util-test.cc | 10 ++-- 10 files changed, 169 insertions(+), 136 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index c203ea76..3cc4b8a3 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -488,9 +488,9 @@ class value { value(char val) { set(int_value, val); } #if !defined(_MSC_VER) || defined(_NATIVE_WCHAR_T_DEFINED) - value(wchar_t value) { + value(wchar_t val) { require_wchar(); - set(int_value, value); + set(int_value, val); } #endif @@ -514,27 +514,27 @@ class value { value(std::nullptr_t) { pointer = nullptr; } template - value(const T &value, + value(const T &val, typename std::enable_if::value, int>::type = 0) { static_assert(get_type() == INT, "invalid type"); - int_value = value; + int_value = val; } template - value(const T &value, + value(const T &val, typename std::enable_if::value, int>::type = 0) { static_assert(get_type() == CUSTOM, "invalid type"); - custom.value = &value; + custom.value = &val; custom.format = &format_custom_arg; } template - value(const named_arg &na) { + value(const named_arg &val) { static_assert(get_type &>() == NAMED_ARG, "invalid type"); - basic_arg arg = make_arg(na.value); - std::memcpy(na.data, &arg, sizeof(arg)); - pointer = &na; + basic_arg arg = make_arg(val.value); + std::memcpy(val.data, &arg, sizeof(arg)); + pointer = &val; } const named_arg_base &as_named_arg() { @@ -543,16 +543,16 @@ class value { private: template - constexpr void set(T &field, const U &value) { + constexpr void set(T &field, const U &val) { static_assert(get_type() == TYPE, "invalid type"); - field = value; + field = val; } template - void set_string(const T &value) { + void set_string(const T &val) { static_assert(get_type() == STRING, "invalid type"); - string.value = value.data(); - string.size = value.size(); + string.value = val.data(); + string.size = val.size(); } template @@ -752,14 +752,14 @@ class arg_map { template class context_base : public basic_parse_context { private: - Range &range_; + Range range_; basic_format_args args_; protected: using char_type = typename Range::value_type; using format_arg = basic_arg; - context_base(Range &range, basic_string_view format_str, + context_base(Range range, basic_string_view format_str, basic_format_args args) : basic_parse_context(format_str), range_(range), args_(args) {} @@ -782,10 +782,40 @@ class context_base : public basic_parse_context { public: basic_parse_context &parse_context() { return *this; } - Range &range() { return range_; } + Range range() { return range_; } +}; + +// A range that can grow dynamically. +template +class dynamic_range { + private: + Container &container_; + + public: + using iterator = decltype(container_.begin()); + using value_type = typename Container::value_type; + + struct sentinel { + friend bool operator!=(sentinel, iterator) { return false; } + friend bool operator!=(iterator, sentinel) { return false; } + }; + + dynamic_range(Container &c) : container_(c) {} + + iterator begin() const { return container_.begin(); } + sentinel end() const { return sentinel(); } + + friend iterator grow(dynamic_range r, size_t n) { + auto size = r.container_.size(); + r.container_.resize(size + n); + return r.container_.begin() + size; + } + + Container &container() const { return container_; } }; } // namespace internal +// Formatting context. template class basic_context : public internal::context_base> { @@ -802,7 +832,6 @@ class basic_context : FMT_DISALLOW_COPY_AND_ASSIGN(basic_context); using base = internal::context_base; - using format_arg = typename base::format_arg; using base::get_arg; @@ -813,7 +842,7 @@ class basic_context : stored in the object so make sure they have appropriate lifetimes. \endrst */ - basic_context(Range &range, basic_string_view format_str, + basic_context(Range range, basic_string_view format_str, basic_format_args args) : base(range, format_str, args) {} @@ -825,8 +854,8 @@ class basic_context : format_arg get_arg(basic_string_view name); }; -using context = basic_context; -using wcontext = basic_context; +using context = basic_context>; +using wcontext = basic_context>; template class arg_store { diff --git a/include/fmt/format.cc b/include/fmt/format.cc index 32975ca1..1eb3a2a9 100644 --- a/include/fmt/format.cc +++ b/include/fmt/format.cc @@ -178,7 +178,7 @@ void format_error_code(buffer &out, int error_code, ++error_code_size; } error_code_size += internal::count_digits(abs_value); - basic_writer w(out); + writer w(out); if (message.size() <= internal::INLINE_BUFFER_SIZE - error_code_size) { w.write(message); w.write(SEP); @@ -339,7 +339,7 @@ FMT_FUNC void internal::format_windows_error( if (result != 0) { utf16_to_utf8 utf8_message; if (utf8_message.convert(system_message) == ERROR_SUCCESS) { - basic_writer w(out); + writer w(out); w.write(message); w.write(": "); w.write(utf8_message); @@ -366,7 +366,7 @@ FMT_FUNC void format_system_error( char *system_message = &buf[0]; int result = safe_strerror(error_code, system_message, buf.size()); if (result == 0) { - basic_writer w(out); + writer w(out); w.write(message); w.write(": "); w.write(system_message); diff --git a/include/fmt/format.h b/include/fmt/format.h index 60dcc2ae..58ea7d9c 100644 --- a/include/fmt/format.h +++ b/include/fmt/format.h @@ -610,33 +610,6 @@ constexpr const Char *pointer_from(null_terminating_iterator it) { return it.ptr_; } -// A range that can grow dynamically. -template -class dynamic_range { - private: - Container &container_; - - public: - using iterator = decltype(container_.begin()); - using value_type = typename Container::value_type; - - struct sentinel { - friend bool operator!=(sentinel, iterator) { return false; } - friend bool operator!=(iterator, sentinel) { return false; } - }; - - explicit dynamic_range(Container &c) : container_(c) {} - - iterator begin() const { return container_.begin(); } - sentinel end() const { return sentinel(); } - - friend iterator grow(dynamic_range r, size_t n) { - auto size = r.container_.size(); - r.container_.resize(size + n); - return r.container_.begin() + size; - } -}; - // Returns true if value is negative, false otherwise. // Same as (value < 0) but doesn't produce warnings if T is an unsigned type. template @@ -1131,8 +1104,8 @@ constexpr void handle_cstring_type_spec(char spec, Handler &&handler) { handler.on_error("invalid type specifier"); } -template -constexpr void check_string_type_spec(char spec, ErrorHandler &&eh) { +template +constexpr void check_string_type_spec(Char spec, ErrorHandler &&eh) { if (spec != 0 && spec != 's') eh.on_error("invalid type specifier"); } @@ -1275,7 +1248,7 @@ class arg_formatter_base { } public: - arg_formatter_base(Range &r, format_specs &s): writer_(r), specs_(s) {} + arg_formatter_base(Range r, format_specs &s): writer_(r), specs_(s) {} void operator()(monostate) { FMT_ASSERT(false, "invalid argument type"); @@ -2002,14 +1975,12 @@ class arg_formatter: public internal::arg_formatter_base { /** \rst Constructs an argument formatter object. - *buffer* is a reference to the buffer to be used for output, - *ctx* is a reference to the formatting context, *spec* contains - format specifier information for standard argument types. + *r* is an output range, *ctx* is a reference to the formatting context, + *spec* contains format specifier information for standard argument types. \endrst */ - arg_formatter(basic_buffer &buffer, basic_context &ctx, - format_specs &spec) - : base(buffer, spec), ctx_(ctx) {} + arg_formatter(Range r, basic_context &ctx, format_specs &spec) + : base(r, spec), ctx_(ctx) {} using base::operator(); @@ -2108,7 +2079,7 @@ class basic_writer { using iterator = decltype(std::declval().begin()); // Output range. - internal::dynamic_range range_; + Range range_; iterator out_; std::unique_ptr locale_; @@ -2334,7 +2305,7 @@ class basic_writer { public: /** Constructs a ``basic_writer`` object. */ - explicit basic_writer(Range &r): range_(r), out_(r.begin()) {} + explicit basic_writer(Range out): range_(out), out_(out.begin()) {} void write(int value) { write_decimal(value); @@ -2455,10 +2426,10 @@ template void basic_writer::write_double(T value, const format_specs &spec) { // Check type. struct spec_handler { - char type; + char_type type; bool upper = false; - explicit spec_handler(char t) : type(t) {} + explicit spec_handler(char_type t) : type(t) {} void on_general() { if (type == 'G') @@ -2600,6 +2571,9 @@ void basic_writer::write_double(T value, const format_specs &spec) { }); } +using writer = basic_writer>; +using wwriter = basic_writer>; + // Reports a system error without throwing an exception. // Can be used to report errors from destructors. FMT_API void report_system_error(int error_code, @@ -2861,9 +2835,8 @@ struct dynamic_formatter { return pointer_from(it); } - template - void format(basic_buffer &buf, const T &val, - basic_context> &ctx) { + template + void format(const T &val, basic_context &ctx) { handle_specs(ctx); struct null_handler : internal::error_handler { void on_align(alignment) {} @@ -2889,8 +2862,8 @@ struct dynamic_formatter { } if (specs_.precision_ != -1) checker.end_precision(); - visit(arg_formatter>(buf, ctx, specs_), - internal::make_arg>>(val)); + visit(arg_formatter(ctx.range(), ctx, specs_), + internal::make_arg>(val)); } private: @@ -2917,19 +2890,20 @@ typename basic_context::format_arg /** Formats arguments and writes the output to the buffer. */ template -void vformat_to(typename ArgFormatter::range &out, +void vformat_to(typename ArgFormatter::range out, basic_string_view format_str, basic_format_args args) { using iterator = internal::null_terminating_iterator; using range = typename ArgFormatter::range; struct handler : internal::error_handler { - handler(range &o, basic_string_view str, + handler(range r, basic_string_view str, basic_format_args format_args) - : out(o), context(o, str, format_args) {} + : out(r), context(r, str, format_args) {} void on_text(iterator begin, iterator end) { - out.append(pointer_from(begin), pointer_from(end)); + size_t size = end - begin; + std::uninitialized_copy_n(begin, size, grow(out, size)); } void on_arg_id() { arg = context.next_arg(); } @@ -2967,7 +2941,7 @@ void vformat_to(typename ArgFormatter::range &out, return it; } - range &out; + range out; Context context; basic_arg arg; }; @@ -3005,13 +2979,26 @@ constexpr fill_spec_factory fill; constexpr format_spec_factory width; constexpr format_spec_factory type; +template +inline void vformat_range(Range out, string_view format_str, format_args args) { + vformat_to>(out, format_str, args); +} + +template +inline void format_range(Range out, string_view format_str, + const Args & ... args) { + vformat_range(out, format_str, make_args(args...)); +} + inline void vformat_to(buffer &buf, string_view format_str, format_args args) { - vformat_to>(buf, format_str, args); + using range = internal::dynamic_range; + vformat_to>(buf, format_str, args); } inline void vformat_to(wbuffer &buf, wstring_view format_str, wformat_args args) { - vformat_to>(buf, format_str, args); + using range = internal::dynamic_range; + vformat_to>(buf, format_str, args); } inline std::string vformat(string_view format_str, format_args args) { diff --git a/include/fmt/printf.h b/include/fmt/printf.h index d999f304..856c62a6 100644 --- a/include/fmt/printf.h +++ b/include/fmt/printf.h @@ -202,7 +202,7 @@ template class printf_arg_formatter; template > -class printf_context; +class basic_printf_context; /** \rst @@ -210,10 +210,9 @@ class printf_context; \endrst */ template -class printf_arg_formatter : - public internal::arg_formatter_base { +class printf_arg_formatter : public internal::arg_formatter_base { private: - printf_context& context_; + basic_printf_context &context_; void write_null_pointer() { this->spec().type_ = 0; @@ -234,7 +233,7 @@ class printf_arg_formatter : \endrst */ printf_arg_formatter(basic_buffer &buffer, format_specs &spec, - printf_context &ctx) + basic_printf_context &ctx) : base(buffer, spec), context_(ctx) {} using base::operator(); @@ -277,7 +276,8 @@ class printf_arg_formatter : } /** Formats an argument of a custom (user-defined) type. */ - void operator()(typename basic_arg>::handle handle) { + void operator()( + typename basic_arg>::handle handle) { handle.format(context_); } }; @@ -288,15 +288,16 @@ struct printf_formatter { auto parse(ParseContext& ctx) { return ctx.begin(); } template - void format(const T &value, printf_context &ctx) { - internal::format_value(ctx.range(), value); + void format(const T &value, basic_printf_context &ctx) { + internal::format_value(ctx.range().container(), value); } }; /** This template formats data and writes the output to a writer. */ template -class printf_context : - private internal::context_base> { +class basic_printf_context : + private internal::context_base< + Range, basic_printf_context> { public: /** The character type for the output. */ using char_type = typename Range::value_type; @@ -305,7 +306,7 @@ class printf_context : using formatter_type = printf_formatter; private: - typedef internal::context_base Base; + typedef internal::context_base Base; using format_arg = typename Base::format_arg; using format_specs = basic_format_specs; using iterator = internal::null_terminating_iterator; @@ -329,8 +330,8 @@ class printf_context : appropriate lifetimes. \endrst */ - printf_context(Range &range, basic_string_view format_str, - basic_format_args args) + basic_printf_context(Range range, basic_string_view format_str, + basic_format_args args) : Base(range, format_str, args) {} using Base::parse_context; @@ -341,7 +342,8 @@ class printf_context : }; template -void printf_context::parse_flags(format_specs &spec, iterator &it) { +void basic_printf_context::parse_flags( + format_specs &spec, iterator &it) { for (;;) { switch (*it++) { case '-': @@ -367,8 +369,8 @@ void printf_context::parse_flags(format_specs &spec, iterator &it) { } template -typename printf_context::format_arg - printf_context::get_arg(iterator it, unsigned arg_index) { +typename basic_printf_context::format_arg + basic_printf_context::get_arg(iterator it, unsigned arg_index) { (void)it; if (arg_index == std::numeric_limits::max()) return this->do_get_arg(this->next_arg_id()); @@ -376,7 +378,7 @@ typename printf_context::format_arg } template -unsigned printf_context::parse_header( +unsigned basic_printf_context::parse_header( iterator &it, format_specs &spec) { unsigned arg_index = std::numeric_limits::max(); char_type c = *it; @@ -413,8 +415,8 @@ unsigned printf_context::parse_header( } template -void printf_context::format() { - Range &buffer = this->range(); +void basic_printf_context::format() { + auto &buffer = this->range().container(); Base &base = *this; auto start = iterator(base); auto it = start; @@ -503,7 +505,8 @@ void printf_context::format() { break; case 'c': // TODO: handle wchar_t - visit(internal::CharConverter>(arg), arg); + visit(internal::CharConverter>(arg), + arg); break; } } @@ -516,13 +519,16 @@ void printf_context::format() { buffer.append(pointer_from(start), pointer_from(it)); } -template +template void printf(basic_buffer &buf, basic_string_view format, - basic_format_args>> args) { - printf_context>(buf, format, args).format(); + basic_format_args args) { + Context(buf, format, args).format(); } -typedef basic_format_args> printf_args; +template +using printf_context = basic_printf_context>; + +using printf_args = basic_format_args>; inline std::string vsprintf(string_view format, printf_args args) { memory_buffer buffer; @@ -544,8 +550,8 @@ inline std::string sprintf(string_view format_str, const Args & ... args) { return vsprintf(format_str, make_args>(args...)); } -inline std::wstring vsprintf( - wstring_view format, basic_format_args> args) { +inline std::wstring vsprintf(wstring_view format, + basic_format_args> args) { wmemory_buffer buffer; printf(buffer, format, args); return to_string(buffer); diff --git a/include/fmt/string.h b/include/fmt/string.h index 4bc01d6a..c9a2eac3 100644 --- a/include/fmt/string.h +++ b/include/fmt/string.h @@ -82,7 +82,7 @@ typedef basic_string_buffer wstring_buffer; template std::string to_string(const T &value) { string_buffer buf; - basic_writer(buf).write(value); + writer(buf).write(value); std::string str; buf.move_to(str); return str; diff --git a/include/fmt/time.h b/include/fmt/time.h index 871d1c27..f0faca46 100644 --- a/include/fmt/time.h +++ b/include/fmt/time.h @@ -31,7 +31,7 @@ struct formatter { } void format(const std::tm &tm, context &ctx) { - buffer &buf = ctx.range(); + buffer &buf = ctx.range().container(); std::size_t start = buf.size(); for (;;) { std::size_t size = buf.capacity() - start; diff --git a/test/custom-formatter-test.cc b/test/custom-formatter-test.cc index 8934de68..06fe9df6 100644 --- a/test/custom-formatter-test.cc +++ b/test/custom-formatter-test.cc @@ -14,18 +14,22 @@ using fmt::printf_arg_formatter; // A custom argument formatter that doesn't print `-` for floating-point values // rounded to 0. -class CustomArgFormatter : public fmt::arg_formatter { +class CustomArgFormatter : + public fmt::arg_formatter> { public: - CustomArgFormatter(fmt::buffer &buf, fmt::basic_context &ctx, - fmt::format_specs &s) - : fmt::arg_formatter(buf, ctx, s) {} + using range = fmt::internal::dynamic_range; + using base = fmt::arg_formatter; - using fmt::arg_formatter::operator(); + CustomArgFormatter(range r, fmt::basic_context &ctx, + fmt::format_specs &s) + : base(r, ctx, s) {} + + using base::operator(); void operator()(double value) { if (round(value * pow(10, spec().precision())) == 0) value = 0; - fmt::arg_formatter::operator()(value); + base::operator()(value); } }; diff --git a/test/format-test.cc b/test/format-test.cc index 3321ed59..58347d9e 100644 --- a/test/format-test.cc +++ b/test/format-test.cc @@ -88,7 +88,8 @@ void std_format(long double value, std::wstring &result) { template ::testing::AssertionResult check_write(const T &value, const char *type) { fmt::basic_memory_buffer buffer; - fmt::basic_writer> writer(buffer); + using range = fmt::internal::dynamic_range>; + fmt::basic_writer writer(buffer); writer.write(value); std::basic_string actual = to_string(buffer); std::basic_string expected; @@ -141,16 +142,16 @@ TEST(StringViewTest, ConvertToString) { } TEST(WriterTest, NotCopyConstructible) { - EXPECT_FALSE(std::is_copy_constructible>::value); + EXPECT_FALSE(std::is_copy_constructible::value); } TEST(WriterTest, NotCopyAssignable) { - EXPECT_FALSE(std::is_copy_assignable>::value); + EXPECT_FALSE(std::is_copy_assignable::value); } TEST(WriterTest, Data) { memory_buffer buf; - fmt::basic_writer w(buf); + fmt::writer w(buf); w.write(42); EXPECT_EQ("42", to_string(buf)); } @@ -203,14 +204,14 @@ TEST(WriterTest, WriteLongDouble) { TEST(WriterTest, WriteDoubleAtBufferBoundary) { memory_buffer buf; - fmt::basic_writer writer(buf); + fmt::writer writer(buf); for (int i = 0; i < 100; ++i) writer.write(1.23456789); } TEST(WriterTest, WriteDoubleWithFilledBuffer) { memory_buffer buf; - fmt::basic_writer writer(buf); + fmt::writer writer(buf); // Fill the buffer. for (int i = 0; i < fmt::internal::INLINE_BUFFER_SIZE; ++i) writer.write(' '); @@ -244,7 +245,7 @@ TEST(WriterTest, WriteWideString) { template std::string write_str(T... args) { memory_buffer buf; - fmt::basic_writer writer(buf); + fmt::writer writer(buf); writer.write(args...); return to_string(buf); } @@ -252,7 +253,7 @@ std::string write_str(T... args) { template std::wstring write_wstr(T... args) { wmemory_buffer buf; - fmt::basic_writer writer(buf); + fmt::wwriter writer(buf); writer.write(args...); return to_string(buf); } @@ -345,13 +346,13 @@ TEST(WriterTest, pad) { { memory_buffer buf; - fmt::basic_writer w(buf); + fmt::writer w(buf); w << Date(2012, 12, 9); EXPECT_EQ("2012-12-9", to_string(buf)); } { memory_buffer buf; - fmt::basic_writer w(buf); + fmt::writer w(buf); w << iso8601(Date(2012, 1, 9)); EXPECT_EQ("2012-01-09", to_string(buf)); } @@ -1223,7 +1224,7 @@ struct formatter { } void format(const Date &d, context &ctx) { - format_to(ctx.range(), "{}-{}-{}", d.year(), d.month(), d.day()); + format_range(ctx.range(), "{}-{}-{}", d.year(), d.month(), d.day()); } }; } @@ -1482,17 +1483,19 @@ TEST(FormatTest, Enum) { EXPECT_EQ("0", fmt::format("{}", A)); } -class mock_arg_formatter: - public fmt::internal::arg_formatter_base { +using buffer_range = fmt::internal::dynamic_range; + +class mock_arg_formatter : + public fmt::internal::arg_formatter_base { private: MOCK_METHOD1(call, void (int value)); public: - using base = fmt::internal::arg_formatter_base; - using range = fmt::buffer; + using base = fmt::internal::arg_formatter_base; + using range = buffer_range; - mock_arg_formatter(fmt::buffer &b, fmt::context &, fmt::format_specs &s) - : base(b, s) { + mock_arg_formatter(buffer_range r, fmt::context &, fmt::format_specs &s) + : base(r, s) { EXPECT_CALL(*this, call(42)); } @@ -1533,9 +1536,9 @@ template <> struct formatter : dynamic_formatter<> { void format(variant value, context& ctx) { if (value.type == variant::INT) - dynamic_formatter::format(ctx.range(), 42, ctx); + dynamic_formatter::format(42, ctx); else - dynamic_formatter::format(ctx.range(), "foo", ctx); + dynamic_formatter::format("foo", ctx); } }; } diff --git a/test/ostream-test.cc b/test/ostream-test.cc index 58b28128..05993f20 100644 --- a/test/ostream-test.cc +++ b/test/ostream-test.cc @@ -58,9 +58,11 @@ TEST(OStreamTest, Enum) { EXPECT_EQ("0", fmt::format("{}", A)); } -struct TestArgFormatter : fmt::arg_formatter { +struct TestArgFormatter + : fmt::arg_formatter> { + using base = fmt::arg_formatter>; TestArgFormatter(fmt::buffer &buf, fmt::context &ctx, fmt::format_specs &s) - : fmt::arg_formatter(buf, ctx, s) {} + : base(buf, ctx, s) {} }; TEST(OStreamTest, CustomArg) { diff --git a/test/util-test.cc b/test/util-test.cc index 09f0c238..90904868 100644 --- a/test/util-test.cc +++ b/test/util-test.cc @@ -81,9 +81,11 @@ struct formatter { return ctx.begin(); } - void format(Test, basic_context> &ctx) { + using range = fmt::internal::dynamic_range>; + + void format(Test, basic_context &ctx) { const Char *test = "test"; - ctx.range().append(test, test + std::strlen(test)); + ctx.range().container().append(test, test + std::strlen(test)); } }; } @@ -517,8 +519,8 @@ VISIT_TYPE(float, double); #define CHECK_ARG_(Char, expected, value) { \ testing::StrictMock> visitor; \ EXPECT_CALL(visitor, visit(expected)); \ - fmt::visit(visitor, \ - make_arg>>(value)); \ + using range = fmt::internal::dynamic_range>; \ + fmt::visit(visitor, make_arg>(value)); \ } #define CHECK_ARG(value) { \