From dfb857ebef2ec9e43fb5a91ba561036feb3d7c17 Mon Sep 17 00:00:00 2001
From: Victor Zverovich <viz@fb.com>
Date: Sat, 24 Dec 2022 19:22:39 -0800
Subject: [PATCH] Refactor format spec parsing

---
 doc/build.py         |   3 +-
 include/fmt/core.h   | 285 +++++++++++++++----------------------------
 include/fmt/format.h | 149 +++-------------------
 test/core-test.cc    | 140 ++-------------------
 test/format-test.cc  |  78 +++---------
 test/xchar-test.cc   |  10 +-
 6 files changed, 150 insertions(+), 515 deletions(-)

diff --git a/doc/build.py b/doc/build.py
index 56142da2..1c993d51 100755
--- a/doc/build.py
+++ b/doc/build.py
@@ -83,8 +83,7 @@ def build_docs(version='dev', **kwargs):
   internal_symbols = [
     'fmt::detail::.*',
     'basic_data<>',
-    'fmt::type_identity',
-    'fmt::dynamic_formatter'
+    'fmt::type_identity'
   ]
   noisy_warnings = [
     'warning: (Compound|Member .* of class) (' + '|'.join(internal_symbols) + \
diff --git a/include/fmt/core.h b/include/fmt/core.h
index cc5f443f..1f20044f 100644
--- a/include/fmt/core.h
+++ b/include/fmt/core.h
@@ -1919,9 +1919,9 @@ class format_arg_store
   See `~fmt::arg` for lifetime considerations.
   \endrst
  */
-template <typename Context = format_context, typename... Args>
-constexpr auto make_format_args(Args&&... args)
-    -> format_arg_store<Context, remove_cvref_t<Args>...> {
+template <typename Context = format_context, typename... T>
+constexpr auto make_format_args(T&&... args)
+    -> format_arg_store<Context, remove_cvref_t<T>...> {
   return {FMT_FORWARD(args)...};
 }
 
@@ -2206,90 +2206,6 @@ template <typename Char> struct dynamic_spec {
   };
 };
 
-// A format specifier handler that sets fields in format_specs.
-template <typename Char> class specs_setter {
- protected:
-  format_specs<Char>& specs_;
-
- public:
-  explicit FMT_CONSTEXPR specs_setter(format_specs<Char>& specs)
-      : specs_(specs) {}
-
-  FMT_CONSTEXPR void on_align(align_t align) { specs_.align = align; }
-  FMT_CONSTEXPR void on_fill(basic_string_view<Char> fill) {
-    specs_.fill = fill;
-  }
-  FMT_CONSTEXPR void on_sign(sign_t s) { specs_.sign = s; }
-  FMT_CONSTEXPR void on_hash() { specs_.alt = true; }
-  FMT_CONSTEXPR void on_localized() { specs_.localized = true; }
-
-  FMT_CONSTEXPR void on_zero() {
-    // Ignore 0 if align is specified for compatibility with std::format.
-    if (specs_.align != align::none) return;
-    specs_.align = align::numeric;
-    specs_.fill[0] = Char('0');
-  }
-
-  FMT_CONSTEXPR void end_precision() {}
-
-  FMT_CONSTEXPR void on_type(presentation_type type) { specs_.type = type; }
-};
-
-// Format spec handler that saves references to arguments representing dynamic
-// width and precision to be resolved at formatting time.
-template <typename ParseContext>
-class dynamic_specs_handler
-    : public specs_setter<typename ParseContext::char_type> {
- public:
-  using char_type = typename ParseContext::char_type;
-
-  FMT_CONSTEXPR dynamic_specs_handler(dynamic_format_specs<char_type>& specs,
-                                      ParseContext& ctx)
-      : specs_setter<char_type>(specs), specs_(specs), context_(ctx) {}
-
-  FMT_CONSTEXPR auto parse_context() -> ParseContext& { return context_; }
-
-  FMT_CONSTEXPR void on_width(const dynamic_spec<char_type>& spec) {
-    switch (spec.kind) {
-    case dynamic_spec_kind::none:
-      break;
-    case dynamic_spec_kind::value:
-      specs_.width = spec.value;
-      break;
-    case dynamic_spec_kind::index:
-      specs_.width_ref = arg_ref<char_type>(spec.value);
-      break;
-    case dynamic_spec_kind::name:
-      specs_.width_ref = arg_ref<char_type>(spec.name);
-      break;
-    }
-  }
-
-  FMT_CONSTEXPR void on_precision(const dynamic_spec<char_type>& spec) {
-    switch (spec.kind) {
-    case dynamic_spec_kind::none:
-      break;
-    case dynamic_spec_kind::value:
-      specs_.precision = spec.value;
-      break;
-    case dynamic_spec_kind::index:
-      specs_.precision_ref = arg_ref<char_type>(spec.value);
-      break;
-    case dynamic_spec_kind::name:
-      specs_.precision_ref = arg_ref<char_type>(spec.name);
-      break;
-    }
-  }
-
-  FMT_CONSTEXPR void on_error(const char* message) {
-    throw_format_error(message);
-  }
-
- private:
-  dynamic_format_specs<char_type>& specs_;
-  ParseContext& context_;
-};
-
 template <typename Char> constexpr bool is_ascii_letter(Char c) {
   return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z');
 }
@@ -2558,90 +2474,136 @@ FMT_CONSTEXPR auto parse_presentation_type(Char type) -> presentation_type {
   }
 }
 
-// Parses standard format specifiers and sends notifications about parsed
-// components to handler.
-template <typename Char, typename SpecHandler>
-FMT_CONSTEXPR FMT_INLINE auto parse_format_specs(const Char* begin,
-                                                 const Char* end,
-                                                 SpecHandler&& handler)
-    -> const Char* {
+FMT_CONSTEXPR inline void require_numeric_argument(type arg_type) {
+  if (!is_arithmetic_type(arg_type))
+    throw_format_error("format specifier requires numeric argument");
+}
+
+template <typename Char> struct parse_format_specs_result {
+  const Char* end;
+  dynamic_format_specs<Char> specs;
+};
+
+// Parses standard format specifiers.
+template <typename Char>
+FMT_CONSTEXPR FMT_INLINE auto parse_format_specs(
+    const Char* begin, const Char* end, basic_format_parse_context<Char>& ctx,
+    type arg_type) -> parse_format_specs_result<Char> {
+  auto specs = dynamic_format_specs<Char>();
   if (1 < end - begin && begin[1] == '}' && is_ascii_letter(*begin) &&
       *begin != 'L') {
-    presentation_type type = parse_presentation_type(*begin++);
-    if (type == presentation_type::none)
-      handler.on_error("invalid type specifier");
-    handler.on_type(type);
-    return begin;
+    specs.type = parse_presentation_type(*begin++);
+    if (specs.type == presentation_type::none)
+      throw_format_error("invalid type specifier");
+    return {begin, specs};
   }
+  if (begin == end) return {begin, specs};
 
-  if (begin == end) return begin;
-
-  auto align_result = parse_align(begin, end);
-  if (align_result.align != align::none) {
-    auto fill_size = align_result.end - begin - 1;
-    if (fill_size > 0) handler.on_fill({begin, to_unsigned(fill_size)});
-    handler.on_align(align_result.align);
+  auto align = parse_align(begin, end);
+  if (align.align != align::none) {
+    specs.align = align.align;
+    auto fill_size = align.end - begin - 1;
+    if (fill_size > 0) specs.fill = {begin, to_unsigned(fill_size)};
   }
-  begin = align_result.end;
-  if (begin == end) return begin;
+  begin = align.end;
+  if (begin == end) return {begin, specs};
 
   // Parse sign.
   switch (to_ascii(*begin)) {
   case '+':
-    handler.on_sign(sign::plus);
+    specs.sign = sign::plus;
     ++begin;
     break;
   case '-':
-    handler.on_sign(sign::minus);
+    specs.sign = sign::minus;
     ++begin;
     break;
   case ' ':
-    handler.on_sign(sign::space);
+    specs.sign = sign::space;
     ++begin;
     break;
   default:
     break;
   }
-  if (begin == end) return begin;
+  if (specs.sign != sign::none) {
+    require_numeric_argument(arg_type);
+    if (is_integral_type(arg_type) && arg_type != type::int_type &&
+        arg_type != type::long_long_type && arg_type != type::int128_type &&
+        arg_type != type::char_type) {
+      throw_format_error("format specifier requires signed argument");
+    }
+  }
+  if (begin == end) return {begin, specs};
 
   if (*begin == '#') {
-    handler.on_hash();
-    if (++begin == end) return begin;
+    require_numeric_argument(arg_type);
+    specs.alt = true;
+    if (++begin == end) return {begin, specs};
   }
 
   // Parse zero flag.
   if (*begin == '0') {
-    handler.on_zero();
-    if (++begin == end) return begin;
+    require_numeric_argument(arg_type);
+    if (specs.align == align::none) {
+      // Ignore 0 if align is specified for compatibility with std::format.
+      specs.align = align::numeric;
+      specs.fill[0] = Char('0');
+    }
+    if (++begin == end) return {begin, specs};
   }
 
-  auto width = parse_dynamic_spec(begin, end, handler.parse_context());
-  handler.on_width(width.spec);
+  auto width = parse_dynamic_spec(begin, end, ctx);
+  switch (width.spec.kind) {
+  case dynamic_spec_kind::none:
+    break;
+  case dynamic_spec_kind::value:
+    specs.width = width.spec.value;
+    break;
+  case dynamic_spec_kind::index:
+    specs.width_ref = arg_ref<Char>(width.spec.value);
+    break;
+  case dynamic_spec_kind::name:
+    specs.width_ref = arg_ref<Char>(width.spec.name);
+    break;
+  }
   begin = width.end;
-  if (begin == end) return begin;
+  if (begin == end) return {begin, specs};
 
   // Parse precision.
   if (*begin == '.') {
-    auto precision = parse_precision(begin, end, handler.parse_context());
-    handler.on_precision(precision.spec);
-    if (precision.spec.kind != dynamic_spec_kind::none) handler.end_precision();
+    auto precision = parse_precision(begin, end, ctx);
+    switch (precision.spec.kind) {
+    case dynamic_spec_kind::none:
+      break;
+    case dynamic_spec_kind::value:
+      specs.precision = precision.spec.value;
+      break;
+    case dynamic_spec_kind::index:
+      specs.precision_ref = arg_ref<Char>(precision.spec.value);
+      break;
+    case dynamic_spec_kind::name:
+      specs.precision_ref = arg_ref<Char>(precision.spec.name);
+      break;
+    }
+    if (is_integral_type(arg_type) || arg_type == type::pointer_type)
+      throw_format_error("precision not allowed for this argument type");
     begin = precision.end;
-    if (begin == end) return begin;
+    if (begin == end) return {begin, specs};
   }
 
   if (*begin == 'L') {
-    handler.on_localized();
+    require_numeric_argument(arg_type);
+    specs.localized = true;
     ++begin;
   }
 
   // Parse type.
   if (begin != end && *begin != '}') {
-    presentation_type type = parse_presentation_type(*begin++);
-    if (type == presentation_type::none)
-      handler.on_error("invalid type specifier");
-    handler.on_type(type);
+    specs.type = parse_presentation_type(*begin++);
+    if (specs.type == presentation_type::none)
+      throw_format_error("invalid type specifier");
   }
-  return begin;
+  return {begin, specs};
 }
 
 template <typename Char, typename Handler>
@@ -2866,57 +2828,6 @@ FMT_CONSTEXPR void check_pointer_type_spec(presentation_type type,
     eh.on_error("invalid type specifier");
 }
 
-// A parse_format_specs handler that checks if specifiers are consistent with
-// the argument type.
-template <typename Handler> class specs_checker : public Handler {
- private:
-  detail::type arg_type_;
-
-  FMT_CONSTEXPR void require_numeric_argument() {
-    if (!is_arithmetic_type(arg_type_))
-      this->on_error("format specifier requires numeric argument");
-  }
-
- public:
-  FMT_CONSTEXPR specs_checker(const Handler& handler, detail::type arg_type)
-      : Handler(handler), arg_type_(arg_type) {}
-
-  FMT_CONSTEXPR void on_align(align_t align) {
-    if (align == align::numeric) require_numeric_argument();
-    Handler::on_align(align);
-  }
-
-  FMT_CONSTEXPR void on_sign(sign_t s) {
-    require_numeric_argument();
-    if (is_integral_type(arg_type_) && arg_type_ != type::int_type &&
-        arg_type_ != type::long_long_type && arg_type_ != type::int128_type &&
-        arg_type_ != type::char_type) {
-      this->on_error("format specifier requires signed argument");
-    }
-    Handler::on_sign(s);
-  }
-
-  FMT_CONSTEXPR void on_hash() {
-    require_numeric_argument();
-    Handler::on_hash();
-  }
-
-  FMT_CONSTEXPR void on_localized() {
-    require_numeric_argument();
-    Handler::on_localized();
-  }
-
-  FMT_CONSTEXPR void on_zero() {
-    require_numeric_argument();
-    Handler::on_zero();
-  }
-
-  FMT_CONSTEXPR void end_precision() {
-    if (is_integral_type(arg_type_) || arg_type_ == type::pointer_type)
-      this->on_error("precision not allowed for this argument type");
-  }
-};
-
 constexpr int invalid_arg_index = -1;
 
 #if FMT_USE_NONTYPE_TEMPLATE_ARGS
@@ -3047,11 +2958,9 @@ struct formatter<T, Char,
   FMT_CONSTEXPR auto parse(ParseContext& ctx) -> decltype(ctx.begin()) {
     auto begin = ctx.begin(), end = ctx.end();
     if (begin == end) return begin;
-    using handler_type = detail::dynamic_specs_handler<ParseContext>;
     auto type = detail::type_constant<T, Char>::value;
-    auto checker =
-        detail::specs_checker<handler_type>(handler_type(specs_, ctx), type);
-    auto it = detail::parse_format_specs(begin, end, checker);
+    auto result = detail::parse_format_specs(begin, end, ctx, type);
+    specs_ = result.specs;
     auto eh = detail::error_handler();
     switch (type) {
     case detail::type::none_type:
@@ -3076,19 +2985,19 @@ struct formatter<T, Char,
       break;
     case detail::type::float_type:
       if (detail::const_check(FMT_USE_FLOAT))
-        detail::parse_float_type_spec(specs_, eh);
+        detail::parse_float_type_spec(result.specs, eh);
       else
         FMT_ASSERT(false, "float support disabled");
       break;
     case detail::type::double_type:
       if (detail::const_check(FMT_USE_DOUBLE))
-        detail::parse_float_type_spec(specs_, eh);
+        detail::parse_float_type_spec(result.specs, eh);
       else
         FMT_ASSERT(false, "double support disabled");
       break;
     case detail::type::long_double_type:
       if (detail::const_check(FMT_USE_LONG_DOUBLE))
-        detail::parse_float_type_spec(specs_, eh);
+        detail::parse_float_type_spec(result.specs, eh);
       else
         FMT_ASSERT(false, "long double support disabled");
       break;
@@ -3106,7 +3015,7 @@ struct formatter<T, Char,
       // formatter specializations.
       break;
     }
-    return it;
+    return result.end;
   }
 
   template <detail::type U = detail::type_constant<T, Char>::value,
diff --git a/include/fmt/format.h b/include/fmt/format.h
index f3219b75..8dbe3253 100644
--- a/include/fmt/format.h
+++ b/include/fmt/format.h
@@ -3588,66 +3588,6 @@ FMT_CONSTEXPR auto get_arg(Context& ctx, ID id) ->
   return arg;
 }
 
-// The standard format specifier handler with checking.
-template <typename Char> class specs_handler : public specs_setter<Char> {
- private:
-  basic_format_parse_context<Char>& parse_context_;
-  buffer_context<Char>& context_;
-
-  // This is only needed for compatibility with gcc 4.4.
-  using format_arg = basic_format_arg<buffer_context<Char>>;
-
- public:
-  FMT_CONSTEXPR specs_handler(format_specs<Char>& specs,
-                              basic_format_parse_context<Char>& parse_ctx,
-                              buffer_context<Char>& ctx)
-      : specs_setter<Char>(specs), parse_context_(parse_ctx), context_(ctx) {}
-
-  FMT_CONSTEXPR auto parse_context() -> basic_format_parse_context<Char>& {
-    return parse_context_;
-  }
-
-  FMT_CONSTEXPR void on_width(const dynamic_spec<Char>& spec) {
-    auto arg = format_arg();
-    switch (spec.kind) {
-    case dynamic_spec_kind::none:
-      return;
-    case dynamic_spec_kind::value:
-      this->specs_.width = spec.value;
-      return;
-    case dynamic_spec_kind::index:
-      arg = detail::get_arg(context_, spec.value);
-      break;
-    case dynamic_spec_kind::name:
-      arg = detail::get_arg(context_, spec.name);
-      break;
-    }
-    this->specs_.width =
-        get_dynamic_spec<width_checker>(arg, context_.error_handler());
-  }
-
-  FMT_CONSTEXPR void on_precision(const dynamic_spec<Char>& spec) {
-    auto arg = format_arg();
-    switch (spec.kind) {
-    case dynamic_spec_kind::none:
-      return;
-    case dynamic_spec_kind::value:
-      this->specs_.precision = spec.value;
-      return;
-    case dynamic_spec_kind::index:
-      arg = detail::get_arg(context_, spec.value);
-      break;
-    case dynamic_spec_kind::name:
-      arg = detail::get_arg(context_, spec.name);
-      break;
-    }
-    this->specs_.precision =
-        get_dynamic_spec<precision_checker>(arg, context_.error_handler());
-  }
-
-  void on_error(const char* message) { context_.on_error(message); }
-};
-
 template <template <typename> class Handler, typename Context>
 FMT_CONSTEXPR void handle_dynamic_spec(int& value,
                                        arg_ref<typename Context::char_type> ref,
@@ -3656,11 +3596,11 @@ FMT_CONSTEXPR void handle_dynamic_spec(int& value,
   case arg_id_kind::none:
     break;
   case arg_id_kind::index:
-    value = detail::get_dynamic_spec<Handler>(ctx.arg(ref.val.index),
+    value = detail::get_dynamic_spec<Handler>(get_arg(ctx, ref.val.index),
                                               ctx.error_handler());
     break;
   case arg_id_kind::name:
-    value = detail::get_dynamic_spec<Handler>(ctx.arg(ref.val.name),
+    value = detail::get_dynamic_spec<Handler>(get_arg(ctx, ref.val.name),
                                               ctx.error_handler());
     break;
   }
@@ -3877,58 +3817,6 @@ struct formatter<Char[N], Char> : formatter<basic_string_view<Char>, Char> {
   }
 };
 
-// A formatter for types known only at run time such as variant alternatives.
-//
-// Usage:
-//   using variant = std::variant<int, std::string>;
-//   template <>
-//   struct formatter<variant>: dynamic_formatter<> {
-//     auto format(const variant& v, format_context& ctx) {
-//       return visit([&](const auto& val) {
-//           return dynamic_formatter<>::format(val, ctx);
-//       }, v);
-//     }
-//   };
-template <typename Char = char> class dynamic_formatter {
- private:
-  detail::dynamic_format_specs<Char> specs_;
-  const Char* format_str_;
-
-  struct null_handler : detail::error_handler {
-    void on_align(align_t) {}
-    void on_sign(sign_t) {}
-    void on_hash() {}
-  };
-
-  template <typename Context> void handle_specs(Context& ctx) {
-    detail::handle_dynamic_spec<detail::width_checker>(specs_.width,
-                                                       specs_.width_ref, ctx);
-    detail::handle_dynamic_spec<detail::precision_checker>(
-        specs_.precision, specs_.precision_ref, ctx);
-  }
-
- public:
-  template <typename ParseContext>
-  FMT_CONSTEXPR auto parse(ParseContext& ctx) -> decltype(ctx.begin()) {
-    format_str_ = ctx.begin();
-    // Checks are deferred to formatting time when the argument type is known.
-    detail::dynamic_specs_handler<ParseContext> handler(specs_, ctx);
-    return detail::parse_format_specs(ctx.begin(), ctx.end(), handler);
-  }
-
-  template <typename T, typename FormatContext>
-  auto format(const T& val, FormatContext& ctx) -> decltype(ctx.out()) {
-    handle_specs(ctx);
-    detail::specs_checker<null_handler> checker(
-        null_handler(), detail::mapped_type_constant<T, FormatContext>::value);
-    checker.on_align(specs_.align);
-    if (specs_.sign != sign::none) checker.on_sign(specs_.sign);
-    if (specs_.alt) checker.on_hash();
-    if (specs_.precision >= 0) checker.end_precision();
-    return detail::write<Char>(ctx.out(), val, specs_, ctx.locale());
-  }
-};
-
 /**
   \rst
   Converts ``p`` to ``const void*`` for pointer formatting.
@@ -3988,12 +3876,11 @@ template <> struct formatter<bytes> {
  public:
   template <typename ParseContext>
   FMT_CONSTEXPR auto parse(ParseContext& ctx) -> decltype(ctx.begin()) {
-    using handler_type = detail::dynamic_specs_handler<ParseContext>;
-    detail::specs_checker<handler_type> handler(handler_type(specs_, ctx),
-                                                detail::type::string_type);
-    auto it = parse_format_specs(ctx.begin(), ctx.end(), handler);
+    auto result = parse_format_specs(ctx.begin(), ctx.end(), ctx,
+                                     detail::type::string_type);
+    specs_ = result.specs;
     detail::check_string_type_spec(specs_.type, detail::error_handler());
-    return it;
+    return result.end;
   }
 
   template <typename FormatContext>
@@ -4031,12 +3918,11 @@ template <typename T> struct formatter<group_digits_view<T>> : formatter<T> {
  public:
   template <typename ParseContext>
   FMT_CONSTEXPR auto parse(ParseContext& ctx) -> decltype(ctx.begin()) {
-    using handler_type = detail::dynamic_specs_handler<ParseContext>;
-    detail::specs_checker<handler_type> handler(handler_type(specs_, ctx),
-                                                detail::type::int_type);
-    auto it = parse_format_specs(ctx.begin(), ctx.end(), handler);
+    auto result =
+        parse_format_specs(ctx.begin(), ctx.end(), ctx, detail::type::int_type);
+    specs_ = result.specs;
     detail::check_string_type_spec(specs_.type, detail::error_handler());
-    return it;
+    return result.end;
   }
 
   template <typename FormatContext>
@@ -4199,8 +4085,6 @@ void vformat_to(buffer<Char>& buf, basic_string_view<Char> fmt,
   using detail::get_arg;
   using detail::locale_ref;
   using detail::parse_format_specs;
-  using detail::specs_checker;
-  using detail::specs_handler;
   using detail::to_unsigned;
   using detail::type;
   using detail::write;
@@ -4255,13 +4139,16 @@ void vformat_to(buffer<Char>& buf, basic_string_view<Char> fmt,
         visit_format_arg(custom_formatter<Char>{parse_context, context}, arg);
         return parse_context.begin();
       }
-      auto specs = format_specs<Char>();
-      specs_checker<specs_handler<Char>> handler(
-          specs_handler<Char>(specs, parse_context, context), arg.type());
-      begin = parse_format_specs(begin, end, handler);
+      auto result = parse_format_specs(begin, end, parse_context, arg.type());
+      detail::handle_dynamic_spec<detail::width_checker>(
+          result.specs.width, result.specs.width_ref, context);
+      detail::handle_dynamic_spec<detail::precision_checker>(
+          result.specs.precision, result.specs.precision_ref, context);
+      begin = result.end;
       if (begin == end || *begin != '}')
         on_error("missing '}' in format string");
-      auto f = arg_formatter<Char>{context.out(), specs, context.locale()};
+      auto f =
+          arg_formatter<Char>{context.out(), result.specs, context.locale()};
       context.advance_to(visit_format_arg(f, arg));
       return begin;
     }
diff --git a/test/core-test.cc b/test/core-test.cc
index fb833bd2..84dda8f4 100644
--- a/test/core-test.cc
+++ b/test/core-test.cc
@@ -520,88 +520,23 @@ TEST(format_test, constexpr_parse_arg_id) {
   static_assert(parse_arg_id("foo:").name.size() == 3, "");
 }
 
-struct test_format_specs_handler {
-  fmt::detail::compile_parse_context<char> ctx =
-      fmt::detail::compile_parse_context<char>({}, 43, nullptr);
-  enum result { none, hash, zero, loc, error };
-  result res = none;
-
-  fmt::align_t alignment = fmt::align::none;
-  fmt::sign_t sign = fmt::sign::none;
-  char fill = 0;
-  int width = 0;
-  fmt::detail::arg_ref<char> width_ref;
-  int precision = 0;
-  fmt::detail::arg_ref<char> precision_ref;
-  fmt::presentation_type type = fmt::presentation_type::none;
-
-  // Workaround for MSVC2017 bug that results in "expression did not evaluate
-  // to a constant" with compiler-generated copy ctor.
-  constexpr test_format_specs_handler() {}
-  constexpr test_format_specs_handler(const test_format_specs_handler& other) =
-      default;
-
-  constexpr auto parse_context() -> fmt::format_parse_context& { return ctx; }
-
-  constexpr void on_align(fmt::align_t a) { alignment = a; }
-  constexpr void on_fill(fmt::string_view f) { fill = f[0]; }
-  constexpr void on_sign(fmt::sign_t s) { sign = s; }
-  constexpr void on_hash() { res = hash; }
-  constexpr void on_zero() { res = zero; }
-  constexpr void on_localized() { res = loc; }
-
-  constexpr void on_width(const fmt::detail::dynamic_spec<char>& spec) {
-    switch (spec.kind) {
-    case fmt::detail::dynamic_spec_kind::none:
-      break;
-    case fmt::detail::dynamic_spec_kind::value:
-      width = spec.value;
-      break;
-    case fmt::detail::dynamic_spec_kind::index:
-      width_ref = spec.value;
-      break;
-    case fmt::detail::dynamic_spec_kind::name:
-      break;
-    }
-  }
-
-  constexpr void on_precision(const fmt::detail::dynamic_spec<char>& spec) {
-    switch (spec.kind) {
-    case fmt::detail::dynamic_spec_kind::none:
-      break;
-    case fmt::detail::dynamic_spec_kind::value:
-      precision = spec.value;
-      break;
-    case fmt::detail::dynamic_spec_kind::index:
-      precision_ref = spec.value;
-      break;
-    case fmt::detail::dynamic_spec_kind::name:
-      break;
-    }
-  }
-
-  constexpr void end_precision() {}
-  constexpr void on_type(fmt::presentation_type t) { type = t; }
-  constexpr void on_error(const char*) { res = error; }
-};
-
-template <size_t N>
-constexpr test_format_specs_handler parse_test_specs(const char (&s)[N]) {
-  auto h = test_format_specs_handler();
-  fmt::detail::parse_format_specs(s, s + N - 1, h);
-  return h;
+template <size_t N> constexpr auto parse_test_specs(const char (&s)[N]) {
+  auto ctx = fmt::detail::compile_parse_context<char>(fmt::string_view(s, N),
+                                                      43, nullptr);
+  auto result = fmt::detail::parse_format_specs(s, s + N - 1, ctx,
+                                                fmt::detail::type::float_type);
+  return result.specs;
 }
 
 TEST(core_test, constexpr_parse_format_specs) {
-  using handler = test_format_specs_handler;
-  static_assert(parse_test_specs("<").alignment == fmt::align::left, "");
-  static_assert(parse_test_specs("*^").fill == '*', "");
+  static_assert(parse_test_specs("<").align == fmt::align::left, "");
+  static_assert(parse_test_specs("*^").fill[0] == '*', "");
   static_assert(parse_test_specs("+").sign == fmt::sign::plus, "");
   static_assert(parse_test_specs("-").sign == fmt::sign::minus, "");
   static_assert(parse_test_specs(" ").sign == fmt::sign::space, "");
-  static_assert(parse_test_specs("#").res == handler::hash, "");
-  static_assert(parse_test_specs("0").res == handler::zero, "");
-  static_assert(parse_test_specs("L").res == handler::loc, "");
+  static_assert(parse_test_specs("#").alt, "");
+  static_assert(parse_test_specs("0").align == fmt::align::numeric, "");
+  static_assert(parse_test_specs("L").localized, "");
   static_assert(parse_test_specs("42").width == 42, "");
   static_assert(parse_test_specs("{42}").width_ref.val.index == 42, "");
   static_assert(parse_test_specs(".42").precision == 42, "");
@@ -609,59 +544,6 @@ TEST(core_test, constexpr_parse_format_specs) {
   static_assert(parse_test_specs("d").type == fmt::presentation_type::dec, "");
 }
 
-template <size_t N>
-constexpr fmt::detail::dynamic_format_specs<char> parse_dynamic_specs(
-    const char (&s)[N]) {
-  auto specs = fmt::detail::dynamic_format_specs<char>();
-  auto ctx = fmt::detail::compile_parse_context<char>({}, 43, nullptr);
-  auto h =
-      fmt::detail::dynamic_specs_handler<fmt::format_parse_context>(specs, ctx);
-  parse_format_specs(s, s + N - 1, h);
-  return specs;
-}
-
-TEST(format_test, constexpr_dynamic_specs_handler) {
-  static_assert(parse_dynamic_specs("<").align == fmt::align::left, "");
-  static_assert(parse_dynamic_specs("*^").fill[0] == '*', "");
-  static_assert(parse_dynamic_specs("+").sign == fmt::sign::plus, "");
-  static_assert(parse_dynamic_specs("-").sign == fmt::sign::minus, "");
-  static_assert(parse_dynamic_specs(" ").sign == fmt::sign::space, "");
-  static_assert(parse_dynamic_specs("#").alt, "");
-  static_assert(parse_dynamic_specs("0").align == fmt::align::numeric, "");
-  static_assert(parse_dynamic_specs("42").width == 42, "");
-  static_assert(parse_dynamic_specs("{}").width_ref.val.index == 0, "");
-  static_assert(parse_dynamic_specs("{42}").width_ref.val.index == 42, "");
-  static_assert(parse_dynamic_specs(".42").precision == 42, "");
-  static_assert(parse_dynamic_specs(".{}").precision_ref.val.index == 0, "");
-  static_assert(parse_dynamic_specs(".{42}").precision_ref.val.index == 42, "");
-  static_assert(parse_dynamic_specs("d").type == fmt::presentation_type::dec,
-                "");
-}
-
-template <size_t N>
-constexpr test_format_specs_handler check_specs(const char (&s)[N]) {
-  fmt::detail::specs_checker<test_format_specs_handler> checker(
-      test_format_specs_handler(), fmt::detail::type::double_type);
-  parse_format_specs(s, s + N - 1, checker);
-  return checker;
-}
-
-TEST(format_test, constexpr_specs_checker) {
-  using handler = test_format_specs_handler;
-  static_assert(check_specs("<").alignment == fmt::align::left, "");
-  static_assert(check_specs("*^").fill == '*', "");
-  static_assert(check_specs("+").sign == fmt::sign::plus, "");
-  static_assert(check_specs("-").sign == fmt::sign::minus, "");
-  static_assert(check_specs(" ").sign == fmt::sign::space, "");
-  static_assert(check_specs("#").res == handler::hash, "");
-  static_assert(check_specs("0").res == handler::zero, "");
-  static_assert(check_specs("42").width == 42, "");
-  static_assert(check_specs("{42}").width_ref.val.index == 42, "");
-  static_assert(check_specs(".42").precision == 42, "");
-  static_assert(check_specs(".{42}").precision_ref.val.index == 42, "");
-  static_assert(check_specs("d").type == fmt::presentation_type::dec, "");
-}
-
 struct test_format_string_handler {
   constexpr void on_text(const char*, const char*) {}
 
diff --git a/test/format-test.cc b/test/format-test.cc
index 29b71d6a..9ec329dc 100644
--- a/test/format-test.cc
+++ b/test/format-test.cc
@@ -1058,50 +1058,50 @@ TEST(format_test, runtime_precision) {
   char format_str[buffer_size];
   safe_sprintf(format_str, "{0:.{%u", UINT_MAX);
   increment(format_str + 5);
-  EXPECT_THROW_MSG((void)fmt::format(runtime(format_str), 0), format_error,
+  EXPECT_THROW_MSG((void)fmt::format(runtime(format_str), 0.0), format_error,
                    "invalid format string");
   size_t size = std::strlen(format_str);
   format_str[size] = '}';
   format_str[size + 1] = 0;
-  EXPECT_THROW_MSG((void)fmt::format(runtime(format_str), 0), format_error,
+  EXPECT_THROW_MSG((void)fmt::format(runtime(format_str), 0.0), format_error,
                    "argument not found");
   format_str[size + 1] = '}';
   format_str[size + 2] = 0;
-  EXPECT_THROW_MSG((void)fmt::format(runtime(format_str), 0), format_error,
+  EXPECT_THROW_MSG((void)fmt::format(runtime(format_str), 0.0), format_error,
                    "argument not found");
 
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{"), 0), format_error,
+  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{"), 0.0), format_error,
                    "invalid format string");
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{}"), 0), format_error,
+  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{}"), 0.0), format_error,
                    "cannot switch from manual to automatic argument indexing");
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{?}}"), 0), format_error,
+  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{?}}"), 0.0), format_error,
                    "invalid format string");
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}"), 0, 0), format_error,
                    "precision not allowed for this argument type");
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0), format_error,
+  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0.0), format_error,
                    "argument not found");
 
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{0:}}"), 0), format_error,
+  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{0:}}"), 0.0), format_error,
                    "invalid format string");
 
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0, -1), format_error,
-                   "negative precision");
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0, (INT_MAX + 1u)),
+  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0.0, -1),
+                   format_error, "negative precision");
+  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0.0, (INT_MAX + 1u)),
                    format_error, "number is too big");
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0, -1l), format_error,
-                   "negative precision");
+  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0.0, -1l),
+                   format_error, "negative precision");
   if (fmt::detail::const_check(sizeof(long) > sizeof(int))) {
     long value = INT_MAX;
-    EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0, (value + 1)),
+    EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0.0, (value + 1)),
                      format_error, "number is too big");
   }
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0, (INT_MAX + 1ul)),
+  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0.0, (INT_MAX + 1ul)),
                    format_error, "number is too big");
 
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0, '0'), format_error,
-                   "precision is not integer");
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0, 0.0), format_error,
-                   "precision is not integer");
+  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0.0, '0'),
+                   format_error, "precision is not integer");
+  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 0.0, 0.0),
+                   format_error, "precision is not integer");
 
   EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.{1}}"), 42, 2), format_error,
                    "precision not allowed for this argument type");
@@ -1985,46 +1985,6 @@ TEST(format_test, non_null_terminated_format_string) {
   EXPECT_EQ("42", fmt::format(string_view("{}foo", 2), 42));
 }
 
-struct variant {
-  enum { int_type, string_type } type;
-  explicit variant(int) : type(int_type) {}
-  explicit variant(const char*) : type(string_type) {}
-};
-
-FMT_BEGIN_NAMESPACE
-template <> struct formatter<variant> : dynamic_formatter<> {
-  auto format(variant value, format_context& ctx) -> decltype(ctx.out()) {
-    if (value.type == variant::int_type)
-      return dynamic_formatter<>::format(42, ctx);
-    return dynamic_formatter<>::format("foo", ctx);
-  }
-};
-FMT_END_NAMESPACE
-
-TEST(format_test, dynamic_formatter) {
-  auto num = variant(42);
-  auto str = variant("foo");
-  EXPECT_EQ("42", fmt::format("{:d}", num));
-  EXPECT_EQ("foo", fmt::format("{:s}", str));
-  EXPECT_EQ(" 42 foo ", fmt::format("{:{}} {:{}}", num, 3, str, 4));
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{0:{}}"), num), format_error,
-                   "cannot switch from manual to automatic argument indexing");
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{:{0}}"), num), format_error,
-                   "cannot switch from automatic to manual argument indexing");
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{:+}"), str), format_error,
-                   "format specifier requires numeric argument");
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{:-}"), str), format_error,
-                   "format specifier requires numeric argument");
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{: }"), str), format_error,
-                   "format specifier requires numeric argument");
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{:#}"), str), format_error,
-                   "format specifier requires numeric argument");
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{:0}"), str), format_error,
-                   "format specifier requires numeric argument");
-  EXPECT_THROW_MSG((void)fmt::format(runtime("{:.2}"), num), format_error,
-                   "precision not allowed for this argument type");
-}
-
 namespace adl_test {
 namespace fmt {
 namespace detail {
diff --git a/test/xchar-test.cc b/test/xchar-test.cc
index 67b73ba9..13cbcf21 100644
--- a/test/xchar-test.cc
+++ b/test/xchar-test.cc
@@ -464,13 +464,11 @@ template <class charT> struct formatter<std::complex<double>, charT> {
  public:
   FMT_CONSTEXPR typename basic_format_parse_context<charT>::iterator parse(
       basic_format_parse_context<charT>& ctx) {
-    using handler_type =
-        detail::dynamic_specs_handler<basic_format_parse_context<charT>>;
-    detail::specs_checker<handler_type> handler(handler_type(specs_, ctx),
-                                                detail::type::string_type);
-    auto it = parse_format_specs(ctx.begin(), ctx.end(), handler);
+    auto result = parse_format_specs(ctx.begin(), ctx.end(), ctx,
+                                     detail::type::string_type);
+    specs_ = result.specs;
     detail::parse_float_type_spec(specs_, detail::error_handler());
-    return it;
+    return result.end;
   }
 
   template <class FormatContext>