From f11e9687083c19612277dbd07d5c4cc91cde39e9 Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Sat, 6 Jun 2020 07:13:38 -0700 Subject: [PATCH] Optimize format string parsing --- include/fmt/compile.h | 40 ++++++++++++++++------------- include/fmt/format.h | 59 ++++++++++++++++++++----------------------- test/format | 20 ++++++--------- test/format-test.cc | 9 ++++--- test/scan.h | 11 ++++---- 5 files changed, 68 insertions(+), 71 deletions(-) diff --git a/include/fmt/compile.h b/include/fmt/compile.h index 03a819bc..fb66ae7c 100644 --- a/include/fmt/compile.h +++ b/include/fmt/compile.h @@ -62,13 +62,15 @@ template struct part_counter { if (begin != end) ++num_parts; } - FMT_CONSTEXPR void on_arg_id() { ++num_parts; } - FMT_CONSTEXPR void on_arg_id(int) { ++num_parts; } - FMT_CONSTEXPR void on_arg_id(basic_string_view) { ++num_parts; } + FMT_CONSTEXPR int on_arg_id() { return ++num_parts, 0; } + FMT_CONSTEXPR int on_arg_id(int) { return ++num_parts, 0; } + FMT_CONSTEXPR int on_arg_id(basic_string_view) { + return ++num_parts, 0; + } - FMT_CONSTEXPR void on_replacement_field(const Char*) {} + FMT_CONSTEXPR void on_replacement_field(int, const Char*) {} - FMT_CONSTEXPR const Char* on_format_specs(const Char* begin, + FMT_CONSTEXPR const Char* on_format_specs(int, const Char* begin, const Char* end) { // Find the matching brace. unsigned brace_counter = 0; @@ -116,25 +118,28 @@ class format_string_compiler : public error_handler { handler_(part::make_text({begin, to_unsigned(end - begin)})); } - FMT_CONSTEXPR void on_arg_id() { + FMT_CONSTEXPR int on_arg_id() { part_ = part::make_arg_index(parse_context_.next_arg_id()); + return 0; } - FMT_CONSTEXPR void on_arg_id(int id) { + FMT_CONSTEXPR int on_arg_id(int id) { parse_context_.check_arg_id(id); part_ = part::make_arg_index(id); + return 0; } - FMT_CONSTEXPR void on_arg_id(basic_string_view id) { + FMT_CONSTEXPR int on_arg_id(basic_string_view id) { part_ = part::make_arg_name(id); + return 0; } - FMT_CONSTEXPR void on_replacement_field(const Char* ptr) { + FMT_CONSTEXPR void on_replacement_field(int, const Char* ptr) { part_.arg_id_end = ptr; handler_(part_); } - FMT_CONSTEXPR const Char* on_format_specs(const Char* begin, + FMT_CONSTEXPR const Char* on_format_specs(int, const Char* begin, const Char* end) { auto repl = typename part::replacement(); dynamic_specs_handler> handler( @@ -165,16 +170,15 @@ void format_arg( basic_format_parse_context& parse_ctx, Context& ctx, Id arg_id) { ctx.advance_to(visit_format_arg( - arg_formatter( - ctx, &parse_ctx), + arg_formatter(ctx, &parse_ctx), ctx.arg(arg_id))); } // vformat_to is defined in a subnamespace to prevent ADL. namespace cf { template -auto vformat_to(OutputIt out, CompiledFormat& cf, basic_format_args args) - -> typename Context::iterator { +auto vformat_to(OutputIt out, CompiledFormat& cf, + basic_format_args args) -> typename Context::iterator { using char_type = typename Context::char_type; basic_format_parse_context parse_ctx( to_string_view(cf.format_str_)); @@ -227,10 +231,10 @@ auto vformat_to(OutputIt out, CompiledFormat& cf, basic_format_args arg if (specs.precision >= 0) checker.check_precision(); advance_to(parse_ctx, part.arg_id_end); - ctx.advance_to(visit_format_arg( - arg_formatter( - ctx, nullptr, &specs), - arg)); + ctx.advance_to( + visit_format_arg(arg_formatter( + ctx, nullptr, &specs), + arg)); break; } } diff --git a/include/fmt/format.h b/include/fmt/format.h index c4cc87b7..8f584165 100644 --- a/include/fmt/format.h +++ b/include/fmt/format.h @@ -2466,15 +2466,17 @@ inline bool find(const char* first, const char* last, char value, } template struct id_adapter { - FMT_CONSTEXPR void operator()() { handler.on_arg_id(); } - FMT_CONSTEXPR void operator()(int id) { handler.on_arg_id(id); } + Handler& handler; + int arg_id; + + FMT_CONSTEXPR void operator()() { arg_id = handler.on_arg_id(); } + FMT_CONSTEXPR void operator()(int id) { arg_id = handler.on_arg_id(id); } FMT_CONSTEXPR void operator()(basic_string_view id) { - handler.on_arg_id(id); + arg_id = handler.on_arg_id(id); } FMT_CONSTEXPR void on_error(const char* message) { handler.on_error(message); } - Handler& handler; }; template @@ -2508,17 +2510,17 @@ FMT_CONSTEXPR_DECL FMT_INLINE void parse_format_string( ++p; if (p == end) return handler.on_error("invalid format string"); if (static_cast(*p) == '}') { - handler.on_arg_id(); - handler.on_replacement_field(p); + handler.on_replacement_field(handler.on_arg_id(), p); } else if (*p == '{') { handler.on_text(p, p + 1); } else { - p = parse_arg_id(p, end, id_adapter{handler}); + auto adapter = id_adapter{handler, 0}; + p = parse_arg_id(p, end, adapter); Char c = p != end ? *p : Char(); if (c == '}') { - handler.on_replacement_field(p); + handler.on_replacement_field(adapter.arg_id, p); } else if (c == ':') { - p = handler.on_format_specs(p + 1, end); + p = handler.on_format_specs(adapter.arg_id, p + 1, end); if (p == end || *p != '}') return handler.on_error("unknown format specifier"); } else { @@ -2548,7 +2550,6 @@ template struct format_handler : detail::error_handler { basic_format_parse_context parse_context; Context context; - int arg_id; format_handler(typename ArgFormatter::iterator out, basic_string_view str, @@ -2563,23 +2564,20 @@ struct format_handler : detail::error_handler { context.advance_to(out); } - void on_arg_id() { arg_id = parse_context.next_arg_id(); } - void on_arg_id(int id) { - parse_context.check_arg_id(id); - arg_id = id; - } - void on_arg_id(basic_string_view id) { arg_id = context.arg_id(id); } + int on_arg_id() { return parse_context.next_arg_id(); } + int on_arg_id(int id) { return parse_context.check_arg_id(id), id; } + int on_arg_id(basic_string_view id) { return context.arg_id(id); } - void on_replacement_field(const Char* p) { + void on_replacement_field(int id, const Char* p) { advance_to(parse_context, p); - auto arg = get_arg(context, arg_id); + auto arg = get_arg(context, id); context.advance_to( visit_format_arg(ArgFormatter(context, &parse_context), arg)); } - const Char* on_format_specs(const Char* begin, const Char* end) { + const Char* on_format_specs(int id, const Char* begin, const Char* end) { advance_to(parse_context, begin); - auto arg = get_arg(context, arg_id); + auto arg = get_arg(context, id); detail::custom_formatter f(parse_context, context); if (visit_format_arg(f, arg)) return parse_context.begin(); basic_format_specs specs; @@ -2632,26 +2630,24 @@ class format_string_checker { public: explicit FMT_CONSTEXPR format_string_checker( basic_string_view format_str, ErrorHandler eh) - : arg_id_(-1), - context_(format_str, num_args, eh), + : context_(format_str, num_args, eh), parse_funcs_{&parse_format_specs...} {} FMT_CONSTEXPR void on_text(const Char*, const Char*) {} - FMT_CONSTEXPR void on_arg_id() { arg_id_ = context_.next_arg_id(); } - FMT_CONSTEXPR void on_arg_id(int id) { - arg_id_ = id; - context_.check_arg_id(id); - } - FMT_CONSTEXPR void on_arg_id(basic_string_view) { + FMT_CONSTEXPR int on_arg_id() { return context_.next_arg_id(); } + FMT_CONSTEXPR int on_arg_id(int id) { return context_.check_arg_id(id), id; } + FMT_CONSTEXPR int on_arg_id(basic_string_view) { on_error("compile-time checks don't support named arguments"); + return 0; } - FMT_CONSTEXPR void on_replacement_field(const Char*) {} + FMT_CONSTEXPR void on_replacement_field(int, const Char*) {} - FMT_CONSTEXPR const Char* on_format_specs(const Char* begin, const Char*) { + FMT_CONSTEXPR const Char* on_format_specs(int id, const Char* begin, + const Char*) { advance_to(context_, begin); - return arg_id_ < num_args ? parse_funcs_[arg_id_](context_) : begin; + return id < num_args ? parse_funcs_[id](context_) : begin; } FMT_CONSTEXPR void on_error(const char* message) { @@ -2665,7 +2661,6 @@ class format_string_checker { // Format specifier parsing function. using parse_func = const Char* (*)(parse_context_type&); - int arg_id_; parse_context_type context_; parse_func parse_funcs_[num_args > 0 ? num_args : 1]; }; diff --git a/test/format b/test/format index 8b9e5b92..2701ed06 100644 --- a/test/format +++ b/test/format @@ -596,23 +596,20 @@ struct format_handler : detail::error_handler { context.advance_to(out); } - void on_arg_id() { - arg = context.arg(parse_ctx.next_arg_id()); - } - void on_arg_id(unsigned id) { - parse_ctx.check_arg_id(id); - arg = context.arg(id); - } - void on_arg_id(fmt::basic_string_view) {} + int on_arg_id() { return parse_ctx.next_arg_id(); } + int on_arg_id(unsigned id) { return parse_ctx.check_arg_id(id), id; } + int on_arg_id(fmt::basic_string_view) { return 0; } - void on_replacement_field(const Char* p) { - parse_ctx.advance_to(parse_ctx.begin() + (p - &*parse_ctx.begin())); + void on_replacement_field(int id, const Char* p) { + auto arg = context.arg(id); + parse_ctx.advance_to(parse_ctx.begin() + (p - &*parse_ctx.begin())); custom_formatter f(parse_ctx, context); if (!visit_format_arg(f, arg)) context.advance_to(visit_format_arg(ArgFormatter(context, &parse_ctx), arg)); } - const Char* on_format_specs(const Char* begin, const Char* end) { + const Char* on_format_specs(int id, const Char* begin, const Char* end) { + auto arg = context.arg(id); parse_ctx.advance_to(parse_ctx.begin() + (begin - &*parse_ctx.begin())); custom_formatter f(parse_ctx, context); if (visit_format_arg(f, arg)) return &*parse_ctx.begin(); @@ -630,7 +627,6 @@ struct format_handler : detail::error_handler { basic_format_parse_context parse_ctx; Context context; - basic_format_arg arg; }; template diff --git a/test/format-test.cc b/test/format-test.cc index 0b94a8d9..1de2df68 100644 --- a/test/format-test.cc +++ b/test/format-test.cc @@ -2223,13 +2223,14 @@ TEST(FormatTest, ConstexprSpecsChecker) { struct test_format_string_handler { FMT_CONSTEXPR void on_text(const char*, const char*) {} - FMT_CONSTEXPR void on_arg_id() {} + FMT_CONSTEXPR int on_arg_id() { return 0; } - template FMT_CONSTEXPR void on_arg_id(T) {} + template FMT_CONSTEXPR int on_arg_id(T) { return 0; } - FMT_CONSTEXPR void on_replacement_field(const char*) {} + FMT_CONSTEXPR void on_replacement_field(int, const char*) {} - FMT_CONSTEXPR const char* on_format_specs(const char* begin, const char*) { + FMT_CONSTEXPR const char* on_format_specs(int, const char* begin, + const char*) { return begin; } diff --git a/test/scan.h b/test/scan.h index 83f7bfd5..de82067a 100644 --- a/test/scan.h +++ b/test/scan.h @@ -169,14 +169,15 @@ struct scan_handler : error_handler { scan_ctx_.advance_to(it + size); } - void on_arg_id() { on_arg_id(next_arg_id_++); } - void on_arg_id(int id) { + int on_arg_id() { return on_arg_id(next_arg_id_++); } + int on_arg_id(int id) { if (id >= args_.size) on_error("argument index out of range"); arg_ = args_.data[id]; + return id; } - void on_arg_id(string_view) { on_error("invalid format"); } + int on_arg_id(string_view) { return on_error("invalid format"), 0; } - void on_replacement_field(const char*) { + void on_replacement_field(int, const char*) { auto it = scan_ctx_.begin(), end = scan_ctx_.end(); switch (arg_.type) { case scan_type::int_type: @@ -208,7 +209,7 @@ struct scan_handler : error_handler { } } - const char* on_format_specs(const char* begin, const char*) { + const char* on_format_specs(int, const char* begin, const char*) { if (arg_.type != scan_type::custom_type) return begin; parse_ctx_.advance_to(begin); arg_.custom.scan(arg_.custom.value, parse_ctx_, scan_ctx_);