Optimize format string parsing

include/fmt/compile.h

@@ -62,13 +62,15 @@ template <typename Char> struct part_counter {
     if (begin != end) ++num_parts;
   }
 
-  FMT_CONSTEXPR void on_arg_id() { ++num_parts; }
+  FMT_CONSTEXPR int on_arg_id() { return ++num_parts, 0; }
-  FMT_CONSTEXPR void on_arg_id(int) { ++num_parts; }
+  FMT_CONSTEXPR int on_arg_id(int) { return ++num_parts, 0; }
-  FMT_CONSTEXPR void on_arg_id(basic_string_view<Char>) { ++num_parts; }
+  FMT_CONSTEXPR int on_arg_id(basic_string_view<Char>) {
+    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<Char> id) {
+  FMT_CONSTEXPR int on_arg_id(basic_string_view<Char> 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<basic_format_parse_context<Char>> handler(
@@ -165,16 +170,15 @@ void format_arg(
     basic_format_parse_context<typename Context::char_type>& parse_ctx,
     Context& ctx, Id arg_id) {
   ctx.advance_to(visit_format_arg(
-      arg_formatter<OutputIt, typename Context::char_type>(
+      arg_formatter<OutputIt, typename Context::char_type>(ctx, &parse_ctx),
-          ctx, &parse_ctx),
       ctx.arg(arg_id)));
 }
 
 // vformat_to is defined in a subnamespace to prevent ADL.
 namespace cf {
 template <typename Context, typename OutputIt, typename CompiledFormat>
-auto vformat_to(OutputIt out, CompiledFormat& cf, basic_format_args<Context> args)
+auto vformat_to(OutputIt out, CompiledFormat& cf,
-    -> typename Context::iterator {
+                basic_format_args<Context> args) -> typename Context::iterator {
   using char_type = typename Context::char_type;
   basic_format_parse_context<char_type> parse_ctx(
       to_string_view(cf.format_str_));
@@ -227,10 +231,10 @@ auto vformat_to(OutputIt out, CompiledFormat& cf, basic_format_args<Context> arg
       if (specs.precision >= 0) checker.check_precision();
 
       advance_to(parse_ctx, part.arg_id_end);
-      ctx.advance_to(visit_format_arg(
+      ctx.advance_to(
-          arg_formatter<OutputIt, typename Context::char_type>(
+          visit_format_arg(arg_formatter<OutputIt, typename Context::char_type>(
-              ctx, nullptr, &specs),
+                               ctx, nullptr, &specs),
-          arg));
+                           arg));
       break;
     }
     }

include/fmt/format.h

@@ -2466,15 +2466,17 @@ inline bool find<false, char>(const char* first, const char* last, char value,
 }
 
 template <typename Handler, typename Char> struct id_adapter {
-  FMT_CONSTEXPR void operator()() { handler.on_arg_id(); }
+  Handler& handler;
-  FMT_CONSTEXPR void operator()(int id) { handler.on_arg_id(id); }
+  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<Char> 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 <bool IS_CONSTEXPR, typename Char, typename Handler>
@@ -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<char>(*p) == '}') {
-      handler.on_arg_id();
+      handler.on_replacement_field(handler.on_arg_id(), p);
-      handler.on_replacement_field(p);
     } else if (*p == '{') {
       handler.on_text(p, p + 1);
     } else {
-      p = parse_arg_id(p, end, id_adapter<Handler, Char>{handler});
+      auto adapter = id_adapter<Handler, Char>{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 <typename ArgFormatter, typename Char, typename Context>
 struct format_handler : detail::error_handler {
   basic_format_parse_context<Char> parse_context;
   Context context;
-  int arg_id;
 
   format_handler(typename ArgFormatter::iterator out,
                  basic_string_view<Char> 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(); }
+  int on_arg_id() { return parse_context.next_arg_id(); }
-  void on_arg_id(int id) {
+  int on_arg_id(int id) { return parse_context.check_arg_id(id), id; }
-    parse_context.check_arg_id(id);
+  int on_arg_id(basic_string_view<Char> id) { return context.arg_id(id); }
-    arg_id = id;
-  }
-  void on_arg_id(basic_string_view<Char> id) { arg_id = 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<Context> f(parse_context, context);
     if (visit_format_arg(f, arg)) return parse_context.begin();
     basic_format_specs<Char> specs;
@@ -2632,26 +2630,24 @@ class format_string_checker {
  public:
   explicit FMT_CONSTEXPR format_string_checker(
       basic_string_view<Char> format_str, ErrorHandler eh)
-      : arg_id_(-1),
+      : context_(format_str, num_args, eh),
-        context_(format_str, num_args, eh),
         parse_funcs_{&parse_format_specs<Args, parse_context_type>...} {}
 
   FMT_CONSTEXPR void on_text(const Char*, const Char*) {}
 
-  FMT_CONSTEXPR void on_arg_id() { arg_id_ = context_.next_arg_id(); }
+  FMT_CONSTEXPR int on_arg_id() { return context_.next_arg_id(); }
-  FMT_CONSTEXPR void on_arg_id(int id) {
+  FMT_CONSTEXPR int on_arg_id(int id) { return context_.check_arg_id(id), id; }
-    arg_id_ = id;
+  FMT_CONSTEXPR int on_arg_id(basic_string_view<Char>) {
-    context_.check_arg_id(id);
-  }
-  FMT_CONSTEXPR void on_arg_id(basic_string_view<Char>) {
     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];
 };

test/format

@@ -596,23 +596,20 @@ struct format_handler : detail::error_handler {
     context.advance_to(out);
   }
 
-  void on_arg_id() {
+  int on_arg_id() { return parse_ctx.next_arg_id(); }
-    arg = context.arg(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<Char>) { return 0; }
-  void on_arg_id(unsigned id) {
-    parse_ctx.check_arg_id(id);
-    arg = context.arg(id);
-  }
-  void on_arg_id(fmt::basic_string_view<Char>) {}
 
-  void on_replacement_field(const Char* p) {
+  void on_replacement_field(int id, const Char* p) {
-    parse_ctx.advance_to(parse_ctx.begin() + (p - &*parse_ctx.begin()));
+    auto arg = context.arg(id);
+    parse_ctx.advance_to(parse_ctx.begin() + (p  - &*parse_ctx.begin()));
     custom_formatter<Context> 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<Context> 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<Char> parse_ctx;
   Context context;
-  basic_format_arg<Context> arg;
 };
 
 template <typename T, typename Char>

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 <typename T> FMT_CONSTEXPR void on_arg_id(T) {}
+  template <typename T> 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;
   }
 

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_++); }
+  int on_arg_id() { return on_arg_id(next_arg_id_++); }
-  void on_arg_id(int 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_);