From f627e73e27b55e018fd07a0789fdda874ca7062d Mon Sep 17 00:00:00 2001 From: Alexey Ochapov Date: Fri, 22 Jan 2021 04:45:52 +0300 Subject: [PATCH 1/2] add support for manual indexing and named fields, add tests --- include/fmt/chrono.h | 4 +- include/fmt/compile.h | 262 ++++++++++++++++++++++++++++++------------ include/fmt/format.h | 19 ++- test/compile-test.cc | 38 +++++- test/format-test.cc | 8 +- 5 files changed, 237 insertions(+), 94 deletions(-) diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h index d68e81489ceb..247e4c012707 100644 --- a/include/fmt/chrono.h +++ b/include/fmt/chrono.h @@ -1090,7 +1090,7 @@ struct formatter, Char> { template FMT_CONSTEXPR arg_ref_type make_arg_ref(Id arg_id) { context.check_arg_id(arg_id); - return arg_ref_type(arg_id); + return arg_ref_type(arg_id, true); } FMT_CONSTEXPR arg_ref_type make_arg_ref(basic_string_view arg_id) { @@ -1099,7 +1099,7 @@ struct formatter, Char> { } FMT_CONSTEXPR arg_ref_type make_arg_ref(detail::auto_id) { - return arg_ref_type(context.next_arg_id()); + return arg_ref_type(context.next_arg_id(), false); } void on_error(const char* msg) { FMT_THROW(format_error(msg)); } diff --git a/include/fmt/compile.h b/include/fmt/compile.h index becf0fe01e37..90692c99309b 100644 --- a/include/fmt/compile.h +++ b/include/fmt/compile.h @@ -72,7 +72,13 @@ const T& first(const T& value, const Tail&...) { // Part of a compiled format string. It can be either literal text or a // replacement field. template struct format_part { - enum class kind { arg_index, arg_name, text, replacement }; + enum class kind { + arg_index_auto, + arg_index_manual, + arg_name, + text, + replacement + }; struct replacement { arg_ref arg_id; @@ -92,11 +98,14 @@ template struct format_part { // Position past the end of the argument id. const Char* arg_id_end = nullptr; - FMT_CONSTEXPR format_part(kind k = kind::arg_index, value v = {}) + FMT_CONSTEXPR format_part(kind k = kind::arg_index_auto, value v = {}) : part_kind(k), val(v) {} - static FMT_CONSTEXPR format_part make_arg_index(int index) { - return format_part(kind::arg_index, index); + static FMT_CONSTEXPR format_part make_arg_index_auto(int index) { + return format_part(kind::arg_index_auto, index); + } + static FMT_CONSTEXPR format_part make_arg_index_manual(int index) { + return format_part(kind::arg_index_manual, index); } static FMT_CONSTEXPR format_part make_arg_name(basic_string_view name) { return format_part(kind::arg_name, name); @@ -173,13 +182,13 @@ class format_string_compiler : public error_handler { } FMT_CONSTEXPR int on_arg_id() { - part_ = part::make_arg_index(parse_context_.next_arg_id()); + part_ = part::make_arg_index_auto(parse_context_.next_arg_id()); return 0; } FMT_CONSTEXPR int on_arg_id(int id) { parse_context_.check_arg_id(id); - part_ = part::make_arg_index(id); + part_ = part::make_arg_index_manual(id); return 0; } @@ -200,9 +209,12 @@ class format_string_compiler : public error_handler { repl.specs, parse_context_); auto it = parse_format_specs(begin, end, handler); if (*it != '}') on_error("missing '}' in format string"); - repl.arg_id = part_.part_kind == part::kind::arg_index - ? arg_ref(part_.val.arg_index) - : arg_ref(part_.val.str); + repl.arg_id = + (part_.part_kind == part::kind::arg_index_auto || + part_.part_kind == part::kind::arg_index_manual) + ? arg_ref(part_.val.arg_index, + part_.part_kind == part::kind::arg_index_manual) + : arg_ref(part_.val.str); auto replacement_part = part::make_replacement(repl); replacement_part.arg_id_end = begin; handler_(replacement_part); @@ -255,7 +267,8 @@ auto vformat_to(OutputIt out, CompiledFormat& cf, break; } - case format_part_t::kind::arg_index: + case format_part_t::kind::arg_index_auto: + case format_part_t::kind::arg_index_manual: advance_to(parse_ctx, part.arg_id_end); detail::format_arg(parse_ctx, ctx, value.arg_index); break; @@ -267,7 +280,8 @@ auto vformat_to(OutputIt out, CompiledFormat& cf, case format_part_t::kind::replacement: { const auto& arg_id_value = value.repl.arg_id.val; - const auto arg = value.repl.arg_id.kind == arg_id_kind::index + const auto arg = (value.repl.arg_id.kind == arg_id_kind::index_auto || + value.repl.arg_id.kind == arg_id_kind::index_manual) ? ctx.arg(arg_id_value.index) : ctx.arg(arg_id_value.name); @@ -463,6 +477,39 @@ template struct field { template struct is_compiled_format> : std::true_type {}; +// A replacement field that refers to argument with name. +template struct runtime_named_field { + using char_type = Char; + basic_string_view name; + + template + constexpr static bool try_format_argument(OutputIt& end, OutputIt out, + basic_string_view arg_name, + const T& arg) { + if constexpr (!is_named_arg::type>::value) { + return false; + } + if (arg_name == arg.name) { + end = write(out, arg.value); + return true; + } + return false; + } + + template + constexpr OutputIt format(OutputIt out, const Args&... args) const { + auto end = out; + bool found = (try_format_argument(end, out, name, args) || ...); + if (!found) { + throw format_error("argument with specified name is not found"); + } + return end; + } +}; + +template +struct is_compiled_format> : std::true_type {}; + // A replacement field that refers to argument N and has format specifiers. template struct spec_field { using char_type = Char; @@ -482,6 +529,47 @@ template struct spec_field { template struct is_compiled_format> : std::true_type {}; +// A replacement field that refers to argument with name and format specifiers. +template struct runtime_named_spec_field { + using char_type = Char; + basic_string_view name; + basic_format_parse_context context; + + template + constexpr static bool try_format_argument( + OutputIt& end, OutputIt out, basic_string_view arg_name, + basic_format_parse_context parse_context, const T& arg, + const Args&... args) { + if constexpr (!is_named_arg::type>::value) { + return false; + } + if (arg_name == arg.name) { + auto fmt = formatter, Char>(); + fmt.parse(parse_context); + const auto& vargs = + make_format_args>(args...); + basic_format_context format_context(out, vargs); + end = fmt.format(arg.value, format_context); + return true; + } + return false; + } + + template + constexpr OutputIt format(OutputIt out, const Args&... args) const { + auto end = out; + bool found = + (try_format_argument(end, out, name, context, args, args...) || ...); + if (!found) { + throw format_error("argument with specified name is not found"); + } + return end; + } +}; + +template +struct is_compiled_format> : std::true_type {}; + template struct concat { L lhs; R rhs; @@ -512,80 +600,110 @@ constexpr size_t parse_text(basic_string_view str, size_t pos) { return pos; } -template -constexpr auto compile_format_string(S format_str); - -template -constexpr auto parse_tail(T head, S format_str) { - if constexpr (POS != - basic_string_view(format_str).size()) { - constexpr auto tail = compile_format_string(format_str); - if constexpr (std::is_same, - unknown_format>()) - return tail; - else - return make_concat(head, tail); - } else { - return head; - } -} - template struct parse_specs_result { formatter fmt; size_t end; - int next_arg_id; }; template constexpr parse_specs_result parse_specs(basic_string_view str, - size_t pos, int arg_id) { + size_t pos, int next_arg_id) { str.remove_prefix(pos); - auto ctx = basic_format_parse_context(str, {}, arg_id + 1); + auto ctx = basic_format_parse_context(str, {}, next_arg_id); auto f = formatter(); auto end = f.parse(ctx); - return {f, pos + fmt::detail::to_unsigned(end - str.data()) + 1, - ctx.next_arg_id()}; + return {f, pos + fmt::detail::to_unsigned(end - str.data()) + 1}; } -// Compiles a non-empty format string and returns the compiled representation -// or unknown_format() on unrecognized input. -template -constexpr auto compile_format_string(S format_str) { - using char_type = typename S::char_type; - constexpr basic_string_view str = format_str; - if constexpr (str[POS] == '{') { - if (POS + 1 == str.size()) - throw format_error("unmatched '{' in format string"); - if constexpr (str[POS + 1] == '{') { - return parse_tail(make_text(str, POS, 1), format_str); - } else if constexpr (str[POS + 1] == '}') { - using id_type = get_type; - return parse_tail(field(), - format_str); - } else if constexpr (str[POS + 1] == ':') { - using id_type = get_type; - constexpr auto result = parse_specs(str, POS + 2, ID); - return parse_tail( - spec_field{result.fmt}, format_str); - } else { - return unknown_format(); +template struct parts_array { + constexpr void append(const format_part& part) { + array_[size_++] = part; + } + constexpr const auto& operator[](size_t index) const { return array_[index]; } + constexpr size_t size() const { return size_; } + + private: + std::array, max_size> array_{}; + size_t size_{}; +}; + +template constexpr auto get_parts_array() { + using char_type = typename CompiledString::char_type; + constexpr basic_string_view format_str = CompiledString{}; + constexpr auto dumb_estimate_counter = [format_str]() { + auto c = format_str.data(); + size_t result = 0; + while (c != format_str.data() + format_str.size()) { + if (*c == static_cast('{')) { + ++result; + } + ++c; } - } else if constexpr (str[POS] == '}') { - if (POS + 1 == str.size()) - throw format_error("unmatched '}' in format string"); - return parse_tail(make_text(str, POS, 1), format_str); - } else { - constexpr auto end = parse_text(str, POS + 1); - if constexpr (end - POS > 1) { - return parse_tail(make_text(str, POS, end - POS), - format_str); + return result * 2 + 1; // "[]{}[]" : #{} = 1, #[] = #{} + 1 = 2 + }; + constexpr auto estimated_parts_amount = dumb_estimate_counter(); + parts_array result; + compile_format_string( + format_str, + [&result](const format_part& part) { result.append(part); }); + return result; +} + +template +constexpr static auto compiled_parts = get_parts_array(); + +template +constexpr auto make_compiled_part() { + using char_type = typename CompiledString::char_type; + + constexpr format_part part = compiled_parts[part_index]; + if constexpr (part.part_kind == decltype(part)::kind::arg_index_auto || + part.part_kind == decltype(part)::kind::arg_index_manual) { + using id_type = get_type; + return field{}; + } else if constexpr (part.part_kind == decltype(part)::kind::arg_name) { + return runtime_named_field{part.val.str}; + } else if constexpr (part.part_kind == decltype(part)::kind::text) { + if constexpr (part.val.str.size() == 1) { + return code_unit{part.val.str[0]}; } else { - return parse_tail(code_unit{str[POS]}, - format_str); + return text{part.val.str}; + } + } else if constexpr (part.part_kind == decltype(part)::kind::replacement) { + constexpr basic_string_view str = CompiledString{}; + constexpr auto pos = static_cast(part.arg_id_end - str.data()); + if constexpr (part.val.repl.arg_id.kind == arg_id_kind::index_auto || + part.val.repl.arg_id.kind == arg_id_kind::index_manual) { + constexpr auto arg_index = part.val.repl.arg_id.val.index; + using id_type = get_type; + constexpr bool is_manual_indexing = + part.val.repl.arg_id.kind == arg_id_kind::index_manual; + constexpr auto next_arg_id = is_manual_indexing ? 0 : arg_index + 1; + constexpr auto result = parse_specs(str, pos, next_arg_id); + return spec_field{result.fmt}; + } else if constexpr (part.val.repl.arg_id.kind == arg_id_kind::name) { + constexpr auto arg_name = part.val.repl.arg_id.val.name; + constexpr auto specs_str = + basic_string_view(part.arg_id_end, str.size() - pos); + auto parse_context = + basic_format_parse_context(specs_str, {}, -1); + return runtime_named_spec_field{arg_name, parse_context}; } } } +// Prepares a compiled representation for non-empty format string +template +constexpr auto make_compiled_parts() { + if constexpr (part_index == compiled_parts.size() - 1) { + return make_compiled_part(); + } else { + return make_concat( + make_compiled_part(), + make_compiled_parts()); + } +} + template ::value || detail::is_compiled_string::value)> @@ -595,14 +713,8 @@ constexpr auto compile(S format_str) { return detail::make_text(str, 0, 0); } else { constexpr auto result = - detail::compile_format_string, 0, 0>( - format_str); - if constexpr (std::is_same, - detail::unknown_format>()) { - return detail::compiled_format(to_string_view(format_str)); - } else { - return result; - } + make_compiled_parts, S, 0>(); + return result; } } #else diff --git a/include/fmt/format.h b/include/fmt/format.h index 654540819ee6..54fea1bceaa9 100644 --- a/include/fmt/format.h +++ b/include/fmt/format.h @@ -2699,23 +2699,17 @@ class specs_handler : public specs_setter { Context& context_; }; -enum class arg_id_kind { none, index, name }; +enum class arg_id_kind { none, index_auto, index_manual, name }; // An argument reference. template struct arg_ref { FMT_CONSTEXPR arg_ref() : kind(arg_id_kind::none), val() {} - FMT_CONSTEXPR explicit arg_ref(int index) - : kind(arg_id_kind::index), val(index) {} + FMT_CONSTEXPR explicit arg_ref(int index, bool is_manual) + : kind(is_manual ? arg_id_kind::index_manual : arg_id_kind::index_auto), val(index) {} FMT_CONSTEXPR explicit arg_ref(basic_string_view name) : kind(arg_id_kind::name), val(name) {} - FMT_CONSTEXPR arg_ref& operator=(int idx) { - kind = arg_id_kind::index; - val.index = idx; - return *this; - } - arg_id_kind kind; union value { FMT_CONSTEXPR value(int id = 0) : index{id} {} @@ -2769,11 +2763,11 @@ class dynamic_specs_handler FMT_CONSTEXPR arg_ref_type make_arg_ref(int arg_id) { context_.check_arg_id(arg_id); - return arg_ref_type(arg_id); + return arg_ref_type(arg_id, true); } FMT_CONSTEXPR arg_ref_type make_arg_ref(auto_id) { - return arg_ref_type(context_.next_arg_id()); + return arg_ref_type(context_.next_arg_id(), false); } FMT_CONSTEXPR arg_ref_type make_arg_ref(basic_string_view arg_id) { @@ -3332,7 +3326,8 @@ FMT_CONSTEXPR void handle_dynamic_spec(int& value, switch (ref.kind) { case arg_id_kind::none: break; - case arg_id_kind::index: + case arg_id_kind::index_auto: + case arg_id_kind::index_manual: value = detail::get_dynamic_spec(ctx.arg(ref.val.index), ctx.error_handler()); break; diff --git a/test/compile-test.cc b/test/compile-test.cc index df76f085b675..6ba4f0e21b67 100644 --- a/test/compile-test.cc +++ b/test/compile-test.cc @@ -148,6 +148,40 @@ TEST(CompileTest, DynamicWidth) { fmt::format(FMT_COMPILE("{:{}}{:{}}"), 42, 4, "foo", 5)); } +TEST(CompileTest, ManualOrdering) { + EXPECT_EQ("42", fmt::format(FMT_COMPILE("{0}"), 42)); + EXPECT_EQ(" -42", fmt::format(FMT_COMPILE("{0:4}"), -42)); + EXPECT_EQ("41 43", fmt::format(FMT_COMPILE("{0} {1}"), 41, 43)); + EXPECT_EQ("41 43", fmt::format(FMT_COMPILE("{1} {0}"), 43, 41)); + EXPECT_EQ("41 43", fmt::format(FMT_COMPILE("{0} {2}"), 41, 42, 43)); + EXPECT_EQ(" 41 43", fmt::format(FMT_COMPILE("{1:{2}} {0:4}"), 43, 41, 4)); + EXPECT_EQ( + "true 42 42 foo 0x1234 foo", + fmt::format(FMT_COMPILE("{0} {1} {2} {3} {4} {5}"), true, 42, 42.0f, + "foo", reinterpret_cast(0x1234), test_formattable())); + EXPECT_EQ(L"42", fmt::format(FMT_COMPILE(L"{0}"), 42)); +} + +TEST(CompileTest, Named) { + EXPECT_EQ("41 43", fmt::format(FMT_COMPILE("{name1} {name2}"), + fmt::arg("name1", 41), fmt::arg("name2", 43))); + EXPECT_EQ("41 43", + fmt::format(FMT_COMPILE("{name1} {name2}"), fmt::arg("name1", 41), + fmt::arg("name2", 43), fmt::arg("name3", 42))); + EXPECT_EQ("41 43", fmt::format(FMT_COMPILE("{name2} {name1}"), + fmt::arg("name1", 43), fmt::arg("name2", 41))); + + EXPECT_EQ(" 42", + fmt::format(FMT_COMPILE("{name1:4}"), fmt::arg("name1", 42))); + EXPECT_EQ(" 41 43", + fmt::format(FMT_COMPILE("{name1:{name2}} {name3:4}"), + fmt::arg("name2", 4), fmt::arg("name3", 43), + fmt::arg("name1", 41))); + + EXPECT_THROW(fmt::format(FMT_COMPILE("{invalid}"), fmt::arg("valid", 42)), + fmt::format_error); +} + TEST(CompileTest, FormatTo) { char buf[8]; auto end = fmt::format_to(buf, FMT_COMPILE("{}"), 42); @@ -174,9 +208,7 @@ TEST(CompileTest, TextAndArg) { EXPECT_EQ("42!", fmt::format(FMT_COMPILE("{}!"), 42)); } -TEST(CompileTest, Empty) { - EXPECT_EQ("", fmt::format(FMT_COMPILE(""))); -} +TEST(CompileTest, Empty) { EXPECT_EQ("", fmt::format(FMT_COMPILE(""))); } #endif #if FMT_USE_NONTYPE_TEMPLATE_PARAMETERS diff --git a/test/format-test.cc b/test/format-test.cc index 804ce1b6df15..b59d4a79ded6 100644 --- a/test/format-test.cc +++ b/test/format-test.cc @@ -2177,12 +2177,16 @@ struct test_format_specs_handler { FMT_CONSTEXPR void on_width(int w) { width = w; } FMT_CONSTEXPR void on_dynamic_width(fmt::detail::auto_id) {} - FMT_CONSTEXPR void on_dynamic_width(int index) { width_ref = index; } + FMT_CONSTEXPR void on_dynamic_width(int index) { + width_ref = fmt::detail::arg_ref(index, true); + } FMT_CONSTEXPR void on_dynamic_width(string_view) {} FMT_CONSTEXPR void on_precision(int p) { precision = p; } FMT_CONSTEXPR void on_dynamic_precision(fmt::detail::auto_id) {} - FMT_CONSTEXPR void on_dynamic_precision(int index) { precision_ref = index; } + FMT_CONSTEXPR void on_dynamic_precision(int index) { + precision_ref = fmt::detail::arg_ref(index, true); + } FMT_CONSTEXPR void on_dynamic_precision(string_view) {} FMT_CONSTEXPR void end_precision() {} From 7c42e8547c5255e2d8ab14a6e0afd0f0e120672f Mon Sep 17 00:00:00 2001 From: Alexey Ochapov Date: Mon, 25 Jan 2021 16:35:24 +0300 Subject: [PATCH 2/2] remove unnecessary `compiled_parts` static --- include/fmt/compile.h | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/include/fmt/compile.h b/include/fmt/compile.h index 90692c99309b..39e34f1c330f 100644 --- a/include/fmt/compile.h +++ b/include/fmt/compile.h @@ -649,14 +649,11 @@ template constexpr auto get_parts_array() { return result; } -template -constexpr static auto compiled_parts = get_parts_array(); - template constexpr auto make_compiled_part() { using char_type = typename CompiledString::char_type; - constexpr format_part part = compiled_parts[part_index]; + constexpr auto part = get_parts_array()[part_index]; if constexpr (part.part_kind == decltype(part)::kind::arg_index_auto || part.part_kind == decltype(part)::kind::arg_index_manual) { using id_type = get_type; @@ -695,7 +692,7 @@ constexpr auto make_compiled_part() { // Prepares a compiled representation for non-empty format string template constexpr auto make_compiled_parts() { - if constexpr (part_index == compiled_parts.size() - 1) { + if constexpr (part_index == get_parts_array().size() - 1) { return make_compiled_part(); } else { return make_concat(