From 934c8e5f764730727dd4523b1206109c6e823875 Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Sat, 24 Dec 2022 11:40:34 -0800 Subject: [PATCH] Refactor precision parsing --- include/fmt/chrono.h | 60 ++++++---------- include/fmt/core.h | 166 +++++++++++++++++-------------------------- include/fmt/format.h | 30 +++++--- test/core-test.cc | 18 +++-- test/format-test.cc | 4 +- 5 files changed, 126 insertions(+), 152 deletions(-) diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h index 4a10af84..02eb8d46 100644 --- a/include/fmt/chrono.h +++ b/include/fmt/chrono.h @@ -2000,36 +2000,6 @@ struct formatter, Char> { basic_string_view format_str; using duration = std::chrono::duration; - struct spec_handler { - formatter& f; - basic_format_parse_context& context; - basic_string_view format_str; - - template FMT_CONSTEXPR arg_ref_type make_arg_ref(Id arg_id) { - context.check_arg_id(arg_id); - return arg_ref_type(arg_id); - } - - FMT_CONSTEXPR arg_ref_type make_arg_ref(basic_string_view arg_id) { - context.check_arg_id(arg_id); - return arg_ref_type(arg_id); - } - - FMT_CONSTEXPR arg_ref_type make_arg_ref(detail::auto_id) { - return arg_ref_type(context.next_arg_id()); - } - - void on_error(const char* msg) { FMT_THROW(format_error(msg)); } - FMT_CONSTEXPR void on_precision(int _precision) { - f.precision = _precision; - } - FMT_CONSTEXPR void end_precision() {} - - template FMT_CONSTEXPR void on_dynamic_precision(Id arg_id) { - f.precision_ref = make_arg_ref(arg_id); - } - }; - using iterator = typename basic_format_parse_context::iterator; struct parse_range { iterator begin; @@ -2039,7 +2009,6 @@ struct formatter, Char> { FMT_CONSTEXPR parse_range do_parse(basic_format_parse_context& ctx) { auto begin = ctx.begin(), end = ctx.end(); if (begin == end || *begin == '}') return {begin, begin}; - auto handler = spec_handler{*this, ctx, format_str}; auto align_result = detail::parse_align(begin, end); specs.align = align_result.align; @@ -2048,28 +2017,41 @@ struct formatter, Char> { begin = align_result.end; if (begin == end) return {begin, begin}; - auto width_result = detail::parse_width(begin, end, ctx); - auto width = width_result.width; - switch (width.kind) { + auto width = detail::parse_dynamic_spec(begin, end, ctx); + switch (width.spec.kind) { case detail::dynamic_spec_kind::none: break; case detail::dynamic_spec_kind::value: - specs.width = width.value; + specs.width = width.spec.value; break; case detail::dynamic_spec_kind::index: - width_ref = arg_ref_type(width.value); + width_ref = arg_ref_type(width.spec.value); break; case detail::dynamic_spec_kind::name: - width_ref = arg_ref_type(width.name); + width_ref = arg_ref_type(width.spec.name); break; } - begin = width_result.end; + begin = width.end; if (begin == end) return {begin, begin}; auto checker = detail::chrono_format_checker(); if (*begin == '.') { checker.has_precision_integral = !std::is_floating_point::value; - begin = detail::parse_precision(begin, end, handler); + auto prec = detail::parse_precision(begin, end, ctx); + switch (prec.spec.kind) { + case detail::dynamic_spec_kind::none: + break; + case detail::dynamic_spec_kind::value: + precision = prec.spec.value; + break; + case detail::dynamic_spec_kind::index: + precision_ref = arg_ref_type(prec.spec.value); + break; + case detail::dynamic_spec_kind::name: + precision_ref = arg_ref_type(prec.spec.name); + break; + } + begin = prec.end; } if (begin != end && *begin == 'L') { ++begin; diff --git a/include/fmt/core.h b/include/fmt/core.h index 21fe1d76..06272ff2 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -2243,9 +2243,6 @@ template class specs_setter { specs_.fill[0] = Char('0'); } - FMT_CONSTEXPR void on_precision(int precision) { - specs_.precision = precision; - } FMT_CONSTEXPR void end_precision() {} FMT_CONSTEXPR void on_type(presentation_type type) { specs_.type = type; } @@ -2265,24 +2262,36 @@ class dynamic_specs_handler FMT_CONSTEXPR auto parse_context() -> ParseContext& { return context_; } - FMT_CONSTEXPR void on_width(const dynamic_spec& width) { - switch (width.kind) { + FMT_CONSTEXPR void on_width(const dynamic_spec& spec) { + switch (spec.kind) { case dynamic_spec_kind::none: break; case dynamic_spec_kind::value: - specs_.width = width.value; + specs_.width = spec.value; break; case dynamic_spec_kind::index: - specs_.width_ref = arg_ref_type(width.value); + specs_.width_ref = arg_ref(spec.value); break; case dynamic_spec_kind::name: - specs_.width_ref = arg_ref_type(width.name); + specs_.width_ref = arg_ref(spec.name); break; } } - template FMT_CONSTEXPR void on_dynamic_precision(Id arg_id) { - specs_.precision_ref = make_arg_ref(arg_id); + FMT_CONSTEXPR void on_precision(const dynamic_spec& 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(spec.value); + break; + case dynamic_spec_kind::name: + specs_.precision_ref = arg_ref(spec.name); + break; + } } FMT_CONSTEXPR void on_error(const char* message) { @@ -2292,26 +2301,6 @@ class dynamic_specs_handler private: dynamic_format_specs& specs_; ParseContext& context_; - - using arg_ref_type = arg_ref; - - FMT_CONSTEXPR auto make_arg_ref(int arg_id) -> arg_ref_type { - context_.check_arg_id(arg_id); - context_.check_dynamic_spec(arg_id); - return arg_ref_type(arg_id); - } - - FMT_CONSTEXPR auto make_arg_ref(auto_id) -> arg_ref_type { - int arg_id = context_.next_arg_id(); - context_.check_dynamic_spec(arg_id); - return arg_ref_type(arg_id); - } - - FMT_CONSTEXPR auto make_arg_ref(basic_string_view arg_id) - -> arg_ref_type { - context_.check_arg_id(arg_id); - return arg_ref_type(arg_id); - } }; template constexpr bool is_ascii_letter(Char c) { @@ -2475,40 +2464,41 @@ FMT_CONSTEXPR FMT_INLINE auto parse_arg_id(const Char* begin, const Char* end, return begin; } -template struct parse_width_result { - const Char* end; - dynamic_spec width; +template struct dynamic_spec_id_handler { + basic_format_parse_context& ctx; + dynamic_spec spec; + + FMT_CONSTEXPR void operator()() { + spec.kind = dynamic_spec_kind::index; + spec.value = ctx.next_arg_id(); + ctx.check_dynamic_spec(spec.value); + } + FMT_CONSTEXPR void operator()(int id) { + spec.kind = dynamic_spec_kind::index; + spec.value = id; + ctx.check_arg_id(id); + ctx.check_dynamic_spec(id); + } + FMT_CONSTEXPR void operator()(basic_string_view id) { + spec.kind = dynamic_spec_kind::name; + spec.name = id; + ctx.check_arg_id(id); + } + FMT_CONSTEXPR void on_error(const char* message) { + if (message) throw_format_error("invalid format string"); + } }; +template struct parse_dynamic_spec_result { + const Char* end; + dynamic_spec spec; +}; + +// Parses [integer | "{" [arg_id] "}"]. template -FMT_CONSTEXPR auto parse_width(const Char* begin, const Char* end, - basic_format_parse_context& ctx) - -> parse_width_result { - struct id_handler { - basic_format_parse_context& ctx; - dynamic_spec spec; - - FMT_CONSTEXPR void operator()() { - spec.kind = dynamic_spec_kind::index; - spec.value = ctx.next_arg_id(); - ctx.check_dynamic_spec(spec.value); - } - FMT_CONSTEXPR void operator()(int id) { - spec.kind = dynamic_spec_kind::index; - spec.value = id; - ctx.check_arg_id(id); - ctx.check_dynamic_spec(id); - } - FMT_CONSTEXPR void operator()(basic_string_view id) { - spec.kind = dynamic_spec_kind::name; - spec.name = id; - ctx.check_arg_id(id); - } - FMT_CONSTEXPR void on_error(const char* message) { - if (message) throw_format_error("invalid format string"); - } - }; - +FMT_CONSTEXPR auto parse_dynamic_spec(const Char* begin, const Char* end, + basic_format_parse_context& ctx) + -> parse_dynamic_spec_result { FMT_ASSERT(begin != end, ""); if ('0' <= *begin && *begin <= '9') { int value = parse_nonnegative_int(begin, end, -1); @@ -2516,7 +2506,8 @@ FMT_CONSTEXPR auto parse_width(const Char* begin, const Char* end, throw_format_error("number is too big"); } else if (*begin == '{') { ++begin; - auto handler = id_handler{ctx, {dynamic_spec_kind::none, {}}}; + auto handler = + dynamic_spec_id_handler{ctx, {dynamic_spec_kind::none, {}}}; if (begin != end) begin = parse_arg_id(begin, end, handler); if (begin != end && *begin == '}') { ++begin; @@ -2527,42 +2518,16 @@ FMT_CONSTEXPR auto parse_width(const Char* begin, const Char* end, return {begin, {dynamic_spec_kind::none, {}}}; } -template +template FMT_CONSTEXPR auto parse_precision(const Char* begin, const Char* end, - Handler&& handler) -> const Char* { - using detail::auto_id; - struct precision_adapter { - Handler& handler; - - FMT_CONSTEXPR void operator()() { handler.on_dynamic_precision(auto_id()); } - FMT_CONSTEXPR void operator()(int id) { handler.on_dynamic_precision(id); } - FMT_CONSTEXPR void operator()(basic_string_view id) { - handler.on_dynamic_precision(id); - } - FMT_CONSTEXPR void on_error(const char* message) { - if (message) handler.on_error(message); - } - }; - + basic_format_parse_context& ctx) + -> parse_dynamic_spec_result { ++begin; - auto c = begin != end ? *begin : Char(); - if ('0' <= c && c <= '9') { - auto precision = parse_nonnegative_int(begin, end, -1); - if (precision != -1) - handler.on_precision(precision); - else - handler.on_error("number is too big"); - } else if (c == '{') { - ++begin; - if (begin != end) - begin = parse_arg_id(begin, end, precision_adapter{handler}); - if (begin == end || *begin++ != '}') - return handler.on_error("invalid format string"), begin; - } else { - return handler.on_error("missing precision specifier"), begin; + if (begin == end || *begin == '}') { + throw_format_error("missing precision"); + return {begin, {dynamic_spec_kind::none, {}}}; } - handler.end_precision(); - return begin; + return parse_dynamic_spec(begin, end, ctx); } template @@ -2666,14 +2631,17 @@ FMT_CONSTEXPR FMT_INLINE auto parse_format_specs(const Char* begin, if (++begin == end) return begin; } - auto width_result = parse_width(begin, end, handler.parse_context()); - handler.on_width(width_result.width); - begin = width_result.end; + auto width = parse_dynamic_spec(begin, end, handler.parse_context()); + handler.on_width(width.spec); + begin = width.end; if (begin == end) return begin; // Parse precision. if (*begin == '.') { - begin = parse_precision(begin, end, handler); + 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(); + begin = precision.end; if (begin == end) return begin; } diff --git a/include/fmt/format.h b/include/fmt/format.h index 534ad3d8..0cc16743 100644 --- a/include/fmt/format.h +++ b/include/fmt/format.h @@ -3627,28 +3627,42 @@ template class specs_handler : public specs_setter { return parse_context_; } - FMT_CONSTEXPR void on_width(const dynamic_spec& width) { + FMT_CONSTEXPR void on_width(const dynamic_spec& spec) { auto arg = format_arg(); - switch (width.kind) { + switch (spec.kind) { case dynamic_spec_kind::none: return; case dynamic_spec_kind::value: - this->specs_.width = width.value; + this->specs_.width = spec.value; return; case dynamic_spec_kind::index: - arg = detail::get_arg(context_, width.value); + arg = detail::get_arg(context_, spec.value); break; case dynamic_spec_kind::name: - arg = detail::get_arg(context_, width.name); + arg = detail::get_arg(context_, spec.name); break; } this->specs_.width = get_dynamic_spec(arg, context_.error_handler()); } - template FMT_CONSTEXPR void on_dynamic_precision(Id arg_id) { - this->specs_.precision = get_dynamic_spec( - get_arg(arg_id), context_.error_handler()); + FMT_CONSTEXPR void on_precision(const dynamic_spec& 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(arg, context_.error_handler()); } void on_error(const char* message) { context_.on_error(message); } diff --git a/test/core-test.cc b/test/core-test.cc index 55bd238b..55f7fb0b 100644 --- a/test/core-test.cc +++ b/test/core-test.cc @@ -568,10 +568,20 @@ struct test_format_specs_handler { } } - constexpr void on_precision(int p) { precision = p; } - constexpr void on_dynamic_precision(fmt::detail::auto_id) {} - constexpr void on_dynamic_precision(int index) { precision_ref = index; } - constexpr void on_dynamic_precision(string_view) {} + constexpr void on_precision(const fmt::detail::dynamic_spec& 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; } diff --git a/test/format-test.cc b/test/format-test.cc index 8f9164b9..3a75ba56 100644 --- a/test/format-test.cc +++ b/test/format-test.cc @@ -929,9 +929,9 @@ TEST(format_test, precision) { "number is too big"); EXPECT_THROW_MSG((void)fmt::format(runtime("{0:."), 0), format_error, - "missing precision specifier"); + "missing precision"); EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.}"), 0), format_error, - "missing precision specifier"); + "missing precision"); EXPECT_THROW_MSG((void)fmt::format(runtime("{0:.2"), 0), format_error, "precision not allowed for this argument type");