From 7a1a67ff9332c2478151de10a6c5fa08867ea7dd Mon Sep 17 00:00:00 2001 From: vsol Date: Mon, 9 Mar 2020 20:50:52 +0300 Subject: [PATCH 01/25] Dynamic arguments storage. Implementation of enhancement from issue #1170. First try... --- include/fmt/core.h | 26 +++++ include/fmt/dyn-args.h | 199 +++++++++++++++++++++++++++++++++++ test/CMakeLists.txt | 1 + test/format-dyn-args-test.cc | 124 ++++++++++++++++++++++ 4 files changed, 350 insertions(+) create mode 100644 include/fmt/dyn-args.h create mode 100644 test/format-dyn-args-test.cc diff --git a/include/fmt/core.h b/include/fmt/core.h index 0e6422d9f1a9..847cf11b8868 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -1005,6 +1005,10 @@ template class basic_format_arg { friend FMT_CONSTEXPR basic_format_arg internal::make_arg( const T& value); + template + friend FMT_CONSTEXPR basic_format_arg internal::make_arg( + const named_arg_base& value); + template friend FMT_CONSTEXPR auto visit_format_arg(Visitor&& vis, const basic_format_arg& arg) @@ -1177,6 +1181,15 @@ template make_arg(const T& value) { return make_arg(value); } + +template +FMT_CONSTEXPR basic_format_arg make_arg( + const named_arg_base& value) { + basic_format_arg arg; + arg.type_ = type::named_arg_type; + arg.value_ = value; + return arg; +} } // namespace internal // Formatting context. @@ -1286,6 +1299,8 @@ inline format_arg_store make_format_args( return {args...}; } +template class dynamic_format_arg_store; + /** \rst A view of a collection of formatting arguments. To avoid lifetime issues it @@ -1357,6 +1372,17 @@ template class basic_format_args { set_data(store.data_); } + /** + \rst + Constructs a `dynamic_basic_format_args` object from `~fmt::format_arg_store`. + \endrst + */ + template + basic_format_args(const dynamic_format_arg_store& store) + : types_(store.get_types()) { + set_data(store.data_.data()); + } + /** \rst Constructs a `basic_format_args` object from a dynamic set of arguments. diff --git a/include/fmt/dyn-args.h b/include/fmt/dyn-args.h new file mode 100644 index 000000000000..285b9bf5a64b --- /dev/null +++ b/include/fmt/dyn-args.h @@ -0,0 +1,199 @@ +// Copyright (c) 2020 Vladimir Solontsov +// SPDX-License-Identifier: MIT Licence + +#ifndef FMT_DYN_ARGS_H_ +#define FMT_DYN_ARGS_H_ + +#include +#include +#include + +#include "core.h" + +#if (defined(FMT_HAS_VARIANT) || __cplusplus >= 201703L) +# include +# ifndef FMT_HAS_VARIANT +# define FMT_HAS_VARIANT +# endif +#endif + +FMT_BEGIN_NAMESPACE + +namespace internal { + +template +struct is_string_view : std::false_type{}; + +template +struct is_string_view, Char> +: std::true_type{}; + +#ifdef FMT_USE_STRING_VIEW +template +struct is_string_view, Char> +: std::true_type{}; +#endif + +#ifdef FMT_USE_EXPERIMENTAL_STRING_VIEW +template +struct is_string_view, Char> +: std::true_type{}; +#endif + +template +struct is_ref_wrapper : std::false_type{}; + +template +struct is_ref_wrapper> : std::true_type{}; + +template +struct need_dyn_copy{ + using mapped_type = mapped_type_constant; + static_assert(mapped_type::value != internal::type::named_arg_type, + "Bug indicator. Named arguments must be processed separately"); + + using type = std::integral_constant::value || + is_string_view::value || + ( + mapped_type::value != internal::type::cstring_type && + mapped_type::value != internal::type::custom_type && + mapped_type::value != internal::type::string_type + ) + )>; +}; + +template using need_dyn_copy_t = + typename need_dyn_copy::type; + +template +const T& get(const StorageValue& v){ + return v; +} + +} // namespace internal + +/** + \rst + A dynamic version of `fmt::format_arg_store<>`. + It's equipped with a storage to potentially temporary objects which lifetime + could be shorter than the format arguments object. + + It can be implicitly converted into `~fmt::basic_format_args` for passing + into type-erased formatting functions such as `~fmt::vformat`. + \endrst + */ +template +class dynamic_format_arg_store +#if FMT_GCC_VERSION < 409 + // Workaround a GCC template argument substitution bug. + : public basic_format_args +#endif +{ + private: + using char_type = typename Context::char_type; + static const bool is_packed = false; + + static const bool has_custom_args = (sizeof...(Args) > 0); + using string_type = std::basic_string; +#ifdef FMT_HAS_VARIANT + using storage_item_type = + conditional_t, + string_type>; +#else + static_assert(!has_custom_args, "std::variant<> is required to support " + "custom types in dynamic_format_arg_store"); + using storage_item_type = string_type; +#endif + using value_type = basic_format_arg; + using named_value_type = internal::named_arg_base; + + template + using storaged_type = + conditional_t::value, string_type, T>; + + // Storage of basic_format_arg must be contiguous + // Required by basic_format_args::args_ which is just a pointer + std::vector data_; + + // Storage of arguments not fitting into basic_format_arg must grow + // without relocation because items in data_ refer to it. + + std::forward_list storage_; + + // Storage of serialized name_args. Must grow without relocation + // because items in data_ refer to it. + std::forward_list named_args_; + + friend class basic_format_args; + + template + const T& get_last_pushed() const{ + using internal::get; + return get(storage_.front()); + } + + unsigned long long get_types() const { + return internal::is_unpacked_bit | data_.size(); + } + + template const T& stored_value(const T& arg, std::false_type) { + return arg; + } + + template const T& stored_value( + const std::reference_wrapper& arg, std::false_type) { + return arg.get(); + } + + template const storaged_type& stored_value(const T& arg, + std::true_type) { + using type = storaged_type; + storage_.emplace_front(type{arg}); + return get_last_pushed(); + } + + template void emplace_arg(const T& arg) { + data_.emplace_back(internal::make_arg(arg)); + } + + public: + dynamic_format_arg_store() FMT_NOEXCEPT = default; + ~dynamic_format_arg_store() FMT_NOEXCEPT = default; + + dynamic_format_arg_store(const dynamic_format_arg_store&) = delete; + dynamic_format_arg_store& operator=(const dynamic_format_arg_store&) = delete; + + dynamic_format_arg_store(dynamic_format_arg_store&&) = default; + dynamic_format_arg_store& operator=(dynamic_format_arg_store&&) = default; + + template void push_back(const T& arg) { + emplace_arg(stored_value(arg, internal::need_dyn_copy_t{})); + } + + template void push_back(std::reference_wrapper arg) { + emplace_arg(arg.get()); + } + + template void push_back( + const internal::named_arg& arg) { + // Named argument is tricky. It's returned by value from fmt::arg() + // and then pointer to it is stored in basic_format_arg<>. + // So after end of expression the pointer becomes dangling. + storage_.emplace_front(string_type{arg.name.data(), arg.name.size()}); + basic_string_view name = get_last_pushed(); + const auto& val = stored_value( + arg.value, internal::need_dyn_copy_t{}); + + auto named_with_stored_parts = fmt::arg(name, val); + // Serialize value into base + internal::arg_mapper().map(named_with_stored_parts); + named_args_.push_front(named_with_stored_parts); + data_.emplace_back(internal::make_arg(named_args_.front())); + } +}; + +FMT_END_NAMESPACE + +#endif /* FMT_DYN_ARGS_H_ */ diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 89176633115c..23ea9fcfb95e 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -95,6 +95,7 @@ add_fmt_test(grisu-test) target_compile_definitions(grisu-test PRIVATE FMT_USE_GRISU=1) add_fmt_test(gtest-extra-test) add_fmt_test(format-test mock-allocator.h) +add_fmt_test(format-dyn-args-test) if (MSVC) target_compile_options(format-test PRIVATE /bigobj) endif () diff --git a/test/format-dyn-args-test.cc b/test/format-dyn-args-test.cc new file mode 100644 index 000000000000..1ef14d1a01c8 --- /dev/null +++ b/test/format-dyn-args-test.cc @@ -0,0 +1,124 @@ +// Copyright (c) 2020 Vladimir Solontsov +// SPDX-License-Identifier: MIT Licence + +#include + +#include "gtest-extra.h" + + +TEST(FormatDynArgsTest, Basic) { + fmt::dynamic_format_arg_store store; + store.push_back(42); + store.push_back("abc1"); + store.push_back(1.5f); + + std::string result = fmt::vformat( + "{} and {} and {}", + store); + + EXPECT_EQ("42 and abc1 and 1.5", result); +} + +TEST(FormatDynArgsTest, StringsAndRefs) { + // Unfortunately the tests are compiled with old ABI + // So strings use COW. + fmt::dynamic_format_arg_store store; + char str[]{"1234567890"}; + store.push_back(str); + store.push_back(std::cref(str)); + store.push_back(std::string_view{str}); + str[0] = 'X'; + + std::string result = fmt::vformat( + "{} and {} and {}", + store); + + EXPECT_EQ("1234567890 and X234567890 and X234567890", result); +} + +struct Custom{ int i{0}; }; +template <> +struct fmt::formatter { + constexpr auto parse(format_parse_context& ctx) { + return ctx.begin(); + } + + template + auto format(const Custom& p, FormatContext& ctx) { + // ctx.out() is an output iterator to write to. + return format_to( + ctx.out(), + "cust={}", p.i); + } +}; + +#ifdef FMT_HAS_VARIANT + +TEST(FormatDynArgsTest, CustomFormat) { + fmt::dynamic_format_arg_store store; + Custom c{}; + store.push_back(c); + ++c.i; + store.push_back(c); + ++c.i; + store.push_back(std::cref(c)); + ++c.i; + + std::string result = fmt::vformat( + "{} and {} and {}", + store); + + EXPECT_EQ("cust=0 and cust=1 and cust=3", result); +} + +#endif // FMT_HAS_VARIANT + +TEST(FormatDynArgsTest, NamedArgByRef) { + fmt::dynamic_format_arg_store store; + auto a1 = fmt::arg("a1_", 42); + store.push_back(std::cref(a1)); + + std::string result = fmt::vformat( + "{a1_}", // and {} and {}", + store); + + EXPECT_EQ("42", result); +} + +TEST(FormatDynArgsTest, NamedInt) { + fmt::dynamic_format_arg_store store; + store.push_back(fmt::arg("a1", 42)); + std::string result = fmt::vformat("{a1}", store); + EXPECT_EQ("42", result); +} + +TEST(FormatDynArgsTest, NamedStrings) { + fmt::dynamic_format_arg_store store; + char str[]{"1234567890"}; + store.push_back(fmt::arg("a1", str)); + store.push_back(fmt::arg("a2", std::cref(str))); + str[0] = 'X'; + + std::string result = fmt::vformat( + "{a1} and {a2}", + store); + + EXPECT_EQ("1234567890 and X234567890", result); +} + +TEST(FormatDynArgsTest, NamedCustomFormat) { + fmt::dynamic_format_arg_store store; + Custom c{}; + store.push_back(fmt::arg("a1", c)); + ++c.i; + store.push_back(fmt::arg("a2", c)); + ++c.i; + store.push_back(fmt::arg("a3", std::cref(c))); + ++c.i; + + std::string result = fmt::vformat( + "{a1} and {a2} and {a3}", + store); + + EXPECT_EQ("cust=0 and cust=1 and cust=3", result); +} From 833bfe374938e2d49754de1c524bd66e9e3713c2 Mon Sep 17 00:00:00 2001 From: vsol Date: Mon, 9 Mar 2020 21:41:37 +0300 Subject: [PATCH 02/25] Finxing build issues. (Not complete) --- include/fmt/core.h | 2 +- include/fmt/dyn-args.h | 2 +- test/format-dyn-args-test.cc | 9 +++++++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index 847cf11b8868..c028b1dfd3e4 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -1007,7 +1007,7 @@ template class basic_format_arg { template friend FMT_CONSTEXPR basic_format_arg internal::make_arg( - const named_arg_base& value); + const internal::named_arg_base& value); template friend FMT_CONSTEXPR auto visit_format_arg(Visitor&& vis, diff --git a/include/fmt/dyn-args.h b/include/fmt/dyn-args.h index 285b9bf5a64b..849cf4c2fe78 100644 --- a/include/fmt/dyn-args.h +++ b/include/fmt/dyn-args.h @@ -36,7 +36,7 @@ struct is_string_view, Char> #ifdef FMT_USE_EXPERIMENTAL_STRING_VIEW template -struct is_string_view, Char> +struct is_string_view, Char> : std::true_type{}; #endif diff --git a/test/format-dyn-args-test.cc b/test/format-dyn-args-test.cc index 1ef14d1a01c8..1ea2991edc13 100644 --- a/test/format-dyn-args-test.cc +++ b/test/format-dyn-args-test.cc @@ -26,7 +26,7 @@ TEST(FormatDynArgsTest, StringsAndRefs) { char str[]{"1234567890"}; store.push_back(str); store.push_back(std::cref(str)); - store.push_back(std::string_view{str}); + store.push_back(fmt::string_view{str}); str[0] = 'X'; std::string result = fmt::vformat( @@ -76,7 +76,8 @@ TEST(FormatDynArgsTest, CustomFormat) { TEST(FormatDynArgsTest, NamedArgByRef) { fmt::dynamic_format_arg_store store; auto a1 = fmt::arg("a1_", 42); - store.push_back(std::cref(a1)); + auto ref = std::cref(a1); + store.push_back(ref); std::string result = fmt::vformat( "{a1_}", // and {} and {}", @@ -106,6 +107,8 @@ TEST(FormatDynArgsTest, NamedStrings) { EXPECT_EQ("1234567890 and X234567890", result); } +#ifdef FMT_HAS_VARIANT + TEST(FormatDynArgsTest, NamedCustomFormat) { fmt::dynamic_format_arg_store store; Custom c{}; @@ -122,3 +125,5 @@ TEST(FormatDynArgsTest, NamedCustomFormat) { EXPECT_EQ("cust=0 and cust=1 and cust=3", result); } + +#endif // FMT_HAS_VARIANT From c3c6d0c2d2ff32179c9312c70b25da8582458505 Mon Sep 17 00:00:00 2001 From: vsol Date: Tue, 10 Mar 2020 11:17:44 +0300 Subject: [PATCH 03/25] Fixed a lifetime issue in test and minor build fixes. --- include/fmt/dyn-args.h | 4 ++-- test/format-dyn-args-test.cc | 31 +++++++++++++++++++++++-------- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/include/fmt/dyn-args.h b/include/fmt/dyn-args.h index 849cf4c2fe78..e80f810ab2b0 100644 --- a/include/fmt/dyn-args.h +++ b/include/fmt/dyn-args.h @@ -159,8 +159,8 @@ class dynamic_format_arg_store } public: - dynamic_format_arg_store() FMT_NOEXCEPT = default; - ~dynamic_format_arg_store() FMT_NOEXCEPT = default; + dynamic_format_arg_store() = default; + ~dynamic_format_arg_store() = default; dynamic_format_arg_store(const dynamic_format_arg_store&) = delete; dynamic_format_arg_store& operator=(const dynamic_format_arg_store&) = delete; diff --git a/test/format-dyn-args-test.cc b/test/format-dyn-args-test.cc index 1ea2991edc13..ee486374d50b 100644 --- a/test/format-dyn-args-test.cc +++ b/test/format-dyn-args-test.cc @@ -37,20 +37,25 @@ TEST(FormatDynArgsTest, StringsAndRefs) { } struct Custom{ int i{0}; }; +FMT_BEGIN_NAMESPACE + template <> -struct fmt::formatter { - constexpr auto parse(format_parse_context& ctx) { +struct formatter { + auto parse(format_parse_context& ctx) const -> decltype(ctx.begin()) + { return ctx.begin(); } template - auto format(const Custom& p, FormatContext& ctx) { - // ctx.out() is an output iterator to write to. - return format_to( + auto format(const Custom& p, FormatContext& ctx) -> + decltype(format_to(ctx.out(), + std::declval())) { + return format_to( ctx.out(), "cust={}", p.i); } }; +FMT_END_NAMESPACE #ifdef FMT_HAS_VARIANT @@ -75,9 +80,19 @@ TEST(FormatDynArgsTest, CustomFormat) { TEST(FormatDynArgsTest, NamedArgByRef) { fmt::dynamic_format_arg_store store; - auto a1 = fmt::arg("a1_", 42); - auto ref = std::cref(a1); - store.push_back(ref); + + // Note: fmt::arg() constructs an object which holds a reference + // to its value. It's not an aggregate, so it doesn't extend the + // reference lifetime. As a result, it's a very bad idea passing temporary + // as a named argument value. Only GCC with optimization level >0 + // complains about this. + // + // A real life usecase is when you have both name and value alive + // guarantee their lifetime and thus don't want them to be copied into + // storages. + int a1_val{42}; + auto a1 = fmt::arg("a1_", a1_val); + store.push_back(std::cref(a1)); std::string result = fmt::vformat( "{a1_}", // and {} and {}", From 448fb746330671011b091d252a8b5496e2acde8f Mon Sep 17 00:00:00 2001 From: vsol Date: Tue, 10 Mar 2020 13:50:04 +0300 Subject: [PATCH 04/25] MSVC2015 fix --- include/fmt/core.h | 18 +++++------------- include/fmt/dyn-args.h | 2 ++ 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index c028b1dfd3e4..48de108a83fa 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -969,6 +969,11 @@ template struct arg_mapper { return val; } + FMT_CONSTEXPR const named_arg_base& map( + const named_arg_base& val){ + return val; + } + int map(...) { constexpr bool formattable = sizeof(Context) == 0; static_assert( @@ -1005,10 +1010,6 @@ template class basic_format_arg { friend FMT_CONSTEXPR basic_format_arg internal::make_arg( const T& value); - template - friend FMT_CONSTEXPR basic_format_arg internal::make_arg( - const internal::named_arg_base& value); - template friend FMT_CONSTEXPR auto visit_format_arg(Visitor&& vis, const basic_format_arg& arg) @@ -1181,15 +1182,6 @@ template make_arg(const T& value) { return make_arg(value); } - -template -FMT_CONSTEXPR basic_format_arg make_arg( - const named_arg_base& value) { - basic_format_arg arg; - arg.type_ = type::named_arg_type; - arg.value_ = value; - return arg; -} } // namespace internal // Formatting context. diff --git a/include/fmt/dyn-args.h b/include/fmt/dyn-args.h index e80f810ab2b0..6b3c5ee1346a 100644 --- a/include/fmt/dyn-args.h +++ b/include/fmt/dyn-args.h @@ -191,6 +191,8 @@ class dynamic_format_arg_store internal::arg_mapper().map(named_with_stored_parts); named_args_.push_front(named_with_stored_parts); data_.emplace_back(internal::make_arg(named_args_.front())); +// data_.emplace_back(internal::make_arg_from_serialized_named( +// named_args_.front())); } }; From 5930040c99100c277fe1df6f8d34e5315138a47a Mon Sep 17 00:00:00 2001 From: vsol Date: Tue, 10 Mar 2020 21:14:55 +0300 Subject: [PATCH 05/25] Clang-format applied. --- include/fmt/core.h | 9 ++-- include/fmt/dyn-args.h | 92 ++++++++++++++++-------------------- test/format-dyn-args-test.cc | 53 ++++++++------------- 3 files changed, 66 insertions(+), 88 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index 48de108a83fa..691aa6357fea 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -259,7 +259,8 @@ namespace internal { // A workaround for gcc 4.8 to make void_t work in a SFINAE context. template struct void_t_impl { using type = void; }; -FMT_NORETURN FMT_API void assert_fail(const char* file, int line, const char* message); +FMT_NORETURN FMT_API void assert_fail(const char* file, int line, + const char* message); #ifndef FMT_ASSERT # ifdef NDEBUG @@ -970,7 +971,7 @@ template struct arg_mapper { } FMT_CONSTEXPR const named_arg_base& map( - const named_arg_base& val){ + const named_arg_base& val) { return val; } @@ -1366,8 +1367,8 @@ template class basic_format_args { /** \rst - Constructs a `dynamic_basic_format_args` object from `~fmt::format_arg_store`. - \endrst + Constructs a `dynamic_basic_format_args` object from + `~fmt::format_arg_store`. \endrst */ template basic_format_args(const dynamic_format_arg_store& store) diff --git a/include/fmt/dyn-args.h b/include/fmt/dyn-args.h index 6b3c5ee1346a..a305b7bc3d14 100644 --- a/include/fmt/dyn-args.h +++ b/include/fmt/dyn-args.h @@ -4,9 +4,9 @@ #ifndef FMT_DYN_ARGS_H_ #define FMT_DYN_ARGS_H_ -#include #include #include +#include #include "core.h" @@ -21,53 +21,46 @@ FMT_BEGIN_NAMESPACE namespace internal { -template -struct is_string_view : std::false_type{}; +template struct is_string_view : std::false_type {}; -template -struct is_string_view, Char> -: std::true_type{}; +template +struct is_string_view, Char> : std::true_type {}; #ifdef FMT_USE_STRING_VIEW -template +template struct is_string_view, Char> -: std::true_type{}; + : std::true_type {}; #endif #ifdef FMT_USE_EXPERIMENTAL_STRING_VIEW -template +template struct is_string_view, Char> -: std::true_type{}; + : std::true_type {}; #endif -template -struct is_ref_wrapper : std::false_type{}; +template struct is_ref_wrapper : std::false_type {}; -template -struct is_ref_wrapper> : std::true_type{}; +template +struct is_ref_wrapper> : std::true_type {}; -template -struct need_dyn_copy{ +template struct need_dyn_copy { using mapped_type = mapped_type_constant; static_assert(mapped_type::value != internal::type::named_arg_type, - "Bug indicator. Named arguments must be processed separately"); - - using type = std::integral_constant::value || - is_string_view::value || - ( - mapped_type::value != internal::type::cstring_type && - mapped_type::value != internal::type::custom_type && - mapped_type::value != internal::type::string_type - ) - )>; + "Bug indicator. Named arguments must be processed separately"); + + using type = std::integral_constant< + bool, !(is_ref_wrapper::value || + is_string_view::value || + (mapped_type::value != internal::type::cstring_type && + mapped_type::value != internal::type::custom_type && + mapped_type::value != internal::type::string_type))>; }; -template using need_dyn_copy_t = - typename need_dyn_copy::type; +template +using need_dyn_copy_t = typename need_dyn_copy::type; -template -const T& get(const StorageValue& v){ +template +const T& get(const StorageValue& v) { return v; } @@ -98,18 +91,18 @@ class dynamic_format_arg_store using string_type = std::basic_string; #ifdef FMT_HAS_VARIANT using storage_item_type = - conditional_t, + conditional_t, string_type>; #else - static_assert(!has_custom_args, "std::variant<> is required to support " - "custom types in dynamic_format_arg_store"); + static_assert(!has_custom_args, + "std::variant<> is required to support " + "custom types in dynamic_format_arg_store"); using storage_item_type = string_type; #endif using value_type = basic_format_arg; using named_value_type = internal::named_arg_base; - template + template using storaged_type = conditional_t::value, string_type, T>; @@ -128,8 +121,7 @@ class dynamic_format_arg_store friend class basic_format_args; - template - const T& get_last_pushed() const{ + template const T& get_last_pushed() const { using internal::get; return get(storage_.front()); } @@ -138,23 +130,23 @@ class dynamic_format_arg_store return internal::is_unpacked_bit | data_.size(); } - template const T& stored_value(const T& arg, std::false_type) { + template const T& stored_value(const T& arg, std::false_type) { return arg; } - template const T& stored_value( - const std::reference_wrapper& arg, std::false_type) { + template + const T& stored_value(const std::reference_wrapper& arg, std::false_type) { return arg.get(); } - template const storaged_type& stored_value(const T& arg, - std::true_type) { + template + const storaged_type& stored_value(const T& arg, std::true_type) { using type = storaged_type; storage_.emplace_front(type{arg}); return get_last_pushed(); } - template void emplace_arg(const T& arg) { + template void emplace_arg(const T& arg) { data_.emplace_back(internal::make_arg(arg)); } @@ -176,23 +168,23 @@ class dynamic_format_arg_store emplace_arg(arg.get()); } - template void push_back( - const internal::named_arg& arg) { + template + void push_back(const internal::named_arg& arg) { // Named argument is tricky. It's returned by value from fmt::arg() // and then pointer to it is stored in basic_format_arg<>. // So after end of expression the pointer becomes dangling. storage_.emplace_front(string_type{arg.name.data(), arg.name.size()}); basic_string_view name = get_last_pushed(); - const auto& val = stored_value( - arg.value, internal::need_dyn_copy_t{}); + const auto& val = + stored_value(arg.value, internal::need_dyn_copy_t{}); auto named_with_stored_parts = fmt::arg(name, val); // Serialize value into base internal::arg_mapper().map(named_with_stored_parts); named_args_.push_front(named_with_stored_parts); data_.emplace_back(internal::make_arg(named_args_.front())); -// data_.emplace_back(internal::make_arg_from_serialized_named( -// named_args_.front())); + // data_.emplace_back(internal::make_arg_from_serialized_named( + // named_args_.front())); } }; diff --git a/test/format-dyn-args-test.cc b/test/format-dyn-args-test.cc index ee486374d50b..fcd9815d81c1 100644 --- a/test/format-dyn-args-test.cc +++ b/test/format-dyn-args-test.cc @@ -5,16 +5,13 @@ #include "gtest-extra.h" - TEST(FormatDynArgsTest, Basic) { fmt::dynamic_format_arg_store store; store.push_back(42); store.push_back("abc1"); store.push_back(1.5f); - std::string result = fmt::vformat( - "{} and {} and {}", - store); + std::string result = fmt::vformat("{} and {} and {}", store); EXPECT_EQ("42 and abc1 and 1.5", result); } @@ -29,31 +26,26 @@ TEST(FormatDynArgsTest, StringsAndRefs) { store.push_back(fmt::string_view{str}); str[0] = 'X'; - std::string result = fmt::vformat( - "{} and {} and {}", - store); + std::string result = fmt::vformat("{} and {} and {}", store); EXPECT_EQ("1234567890 and X234567890 and X234567890", result); } -struct Custom{ int i{0}; }; +struct Custom { + int i{0}; +}; FMT_BEGIN_NAMESPACE -template <> -struct formatter { - auto parse(format_parse_context& ctx) const -> decltype(ctx.begin()) - { +template <> struct formatter { + auto parse(format_parse_context& ctx) const -> decltype(ctx.begin()) { return ctx.begin(); } template - auto format(const Custom& p, FormatContext& ctx) -> - decltype(format_to(ctx.out(), - std::declval())) { - return format_to( - ctx.out(), - "cust={}", p.i); - } + auto format(const Custom& p, FormatContext& ctx) -> decltype(format_to( + ctx.out(), std::declval())) { + return format_to(ctx.out(), "cust={}", p.i); + } }; FMT_END_NAMESPACE @@ -69,14 +61,12 @@ TEST(FormatDynArgsTest, CustomFormat) { store.push_back(std::cref(c)); ++c.i; - std::string result = fmt::vformat( - "{} and {} and {}", - store); + std::string result = fmt::vformat("{} and {} and {}", store); EXPECT_EQ("cust=0 and cust=1 and cust=3", result); } -#endif // FMT_HAS_VARIANT +#endif // FMT_HAS_VARIANT TEST(FormatDynArgsTest, NamedArgByRef) { fmt::dynamic_format_arg_store store; @@ -88,15 +78,14 @@ TEST(FormatDynArgsTest, NamedArgByRef) { // complains about this. // // A real life usecase is when you have both name and value alive - // guarantee their lifetime and thus don't want them to be copied into + // guarantee their lifetime and thus don't want them to be copied into // storages. int a1_val{42}; auto a1 = fmt::arg("a1_", a1_val); store.push_back(std::cref(a1)); - std::string result = fmt::vformat( - "{a1_}", // and {} and {}", - store); + std::string result = fmt::vformat("{a1_}", // and {} and {}", + store); EXPECT_EQ("42", result); } @@ -115,9 +104,7 @@ TEST(FormatDynArgsTest, NamedStrings) { store.push_back(fmt::arg("a2", std::cref(str))); str[0] = 'X'; - std::string result = fmt::vformat( - "{a1} and {a2}", - store); + std::string result = fmt::vformat("{a1} and {a2}", store); EXPECT_EQ("1234567890 and X234567890", result); } @@ -134,11 +121,9 @@ TEST(FormatDynArgsTest, NamedCustomFormat) { store.push_back(fmt::arg("a3", std::cref(c))); ++c.i; - std::string result = fmt::vformat( - "{a1} and {a2} and {a3}", - store); + std::string result = fmt::vformat("{a1} and {a2} and {a3}", store); EXPECT_EQ("cust=0 and cust=1 and cust=3", result); } -#endif // FMT_HAS_VARIANT +#endif // FMT_HAS_VARIANT From f0984c43bcdd3f639295d367df5d4440579ad238 Mon Sep 17 00:00:00 2001 From: vsol Date: Thu, 12 Mar 2020 09:49:20 +0300 Subject: [PATCH 06/25] Addressed minor issues, removed named_arg support. named_args will be added in a separate PR. Pending: avoid Args types from dynamic_format_arg_store template parameters, replace std::forward_list with a custom list. --- include/fmt/core.h | 10 +++------- include/fmt/dyn-args.h | 24 ----------------------- test/format-dyn-args-test.cc | 38 ------------------------------------ 3 files changed, 3 insertions(+), 69 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index 6d063f8e18a9..0bc74fdf234e 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -971,11 +971,6 @@ template struct arg_mapper { return val; } - FMT_CONSTEXPR const named_arg_base& map( - const named_arg_base& val) { - return val; - } - int map(...) { constexpr bool formattable = sizeof(Context) == 0; static_assert( @@ -1368,8 +1363,9 @@ template class basic_format_args { /** \rst - Constructs a `dynamic_basic_format_args` object from - `~fmt::format_arg_store`. \endrst + Constructs a `basic_format_args` object from + `~fmt::dynamic_format_arg_store`. + \endrst */ template basic_format_args(const dynamic_format_arg_store& store) diff --git a/include/fmt/dyn-args.h b/include/fmt/dyn-args.h index a305b7bc3d14..7f0293c7b99a 100644 --- a/include/fmt/dyn-args.h +++ b/include/fmt/dyn-args.h @@ -85,7 +85,6 @@ class dynamic_format_arg_store { private: using char_type = typename Context::char_type; - static const bool is_packed = false; static const bool has_custom_args = (sizeof...(Args) > 0); using string_type = std::basic_string; @@ -115,10 +114,6 @@ class dynamic_format_arg_store std::forward_list storage_; - // Storage of serialized name_args. Must grow without relocation - // because items in data_ refer to it. - std::forward_list named_args_; - friend class basic_format_args; template const T& get_last_pushed() const { @@ -167,25 +162,6 @@ class dynamic_format_arg_store template void push_back(std::reference_wrapper arg) { emplace_arg(arg.get()); } - - template - void push_back(const internal::named_arg& arg) { - // Named argument is tricky. It's returned by value from fmt::arg() - // and then pointer to it is stored in basic_format_arg<>. - // So after end of expression the pointer becomes dangling. - storage_.emplace_front(string_type{arg.name.data(), arg.name.size()}); - basic_string_view name = get_last_pushed(); - const auto& val = - stored_value(arg.value, internal::need_dyn_copy_t{}); - - auto named_with_stored_parts = fmt::arg(name, val); - // Serialize value into base - internal::arg_mapper().map(named_with_stored_parts); - named_args_.push_front(named_with_stored_parts); - data_.emplace_back(internal::make_arg(named_args_.front())); - // data_.emplace_back(internal::make_arg_from_serialized_named( - // named_args_.front())); - } }; FMT_END_NAMESPACE diff --git a/test/format-dyn-args-test.cc b/test/format-dyn-args-test.cc index fcd9815d81c1..94befa1f4b1b 100644 --- a/test/format-dyn-args-test.cc +++ b/test/format-dyn-args-test.cc @@ -89,41 +89,3 @@ TEST(FormatDynArgsTest, NamedArgByRef) { EXPECT_EQ("42", result); } - -TEST(FormatDynArgsTest, NamedInt) { - fmt::dynamic_format_arg_store store; - store.push_back(fmt::arg("a1", 42)); - std::string result = fmt::vformat("{a1}", store); - EXPECT_EQ("42", result); -} - -TEST(FormatDynArgsTest, NamedStrings) { - fmt::dynamic_format_arg_store store; - char str[]{"1234567890"}; - store.push_back(fmt::arg("a1", str)); - store.push_back(fmt::arg("a2", std::cref(str))); - str[0] = 'X'; - - std::string result = fmt::vformat("{a1} and {a2}", store); - - EXPECT_EQ("1234567890 and X234567890", result); -} - -#ifdef FMT_HAS_VARIANT - -TEST(FormatDynArgsTest, NamedCustomFormat) { - fmt::dynamic_format_arg_store store; - Custom c{}; - store.push_back(fmt::arg("a1", c)); - ++c.i; - store.push_back(fmt::arg("a2", c)); - ++c.i; - store.push_back(fmt::arg("a3", std::cref(c))); - ++c.i; - - std::string result = fmt::vformat("{a1} and {a2} and {a3}", store); - - EXPECT_EQ("cust=0 and cust=1 and cust=3", result); -} - -#endif // FMT_HAS_VARIANT From 107f9a84b42ce047320d355ad3b86da19728c4fc Mon Sep 17 00:00:00 2001 From: vsol Date: Thu, 12 Mar 2020 10:30:25 +0300 Subject: [PATCH 07/25] Custom list -- first cut. Pending: cleanup. --- include/fmt/core.h | 5 ++- include/fmt/dyn-args.h | 62 ++++++++++++++++++++---------------- test/format-dyn-args-test.cc | 6 +--- 3 files changed, 38 insertions(+), 35 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index 0bc74fdf234e..fae0e0633fe9 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -1288,7 +1288,7 @@ inline format_arg_store make_format_args( return {args...}; } -template class dynamic_format_arg_store; +template class dynamic_format_arg_store; /** \rst @@ -1367,8 +1367,7 @@ template class basic_format_args { `~fmt::dynamic_format_arg_store`. \endrst */ - template - basic_format_args(const dynamic_format_arg_store& store) + basic_format_args(const dynamic_format_arg_store& store) : types_(store.get_types()) { set_data(store.data_.data()); } diff --git a/include/fmt/dyn-args.h b/include/fmt/dyn-args.h index 7f0293c7b99a..20a4e2ab18a6 100644 --- a/include/fmt/dyn-args.h +++ b/include/fmt/dyn-args.h @@ -6,6 +6,7 @@ #include #include +#include #include #include "core.h" @@ -59,10 +60,37 @@ template struct need_dyn_copy { template using need_dyn_copy_t = typename need_dyn_copy::type; -template -const T& get(const StorageValue& v) { - return v; -} +class dyn_arg_storage { + struct dyn_arg_node_base { + virtual ~dyn_arg_node_base() = default; + std::unique_ptr next_; + }; + + template + struct dyn_arg_node : dyn_arg_node_base { + T value_; + FMT_CONSTEXPR explicit dyn_arg_node(T&& arg) : value_{arg}{} + }; + + std::unique_ptr head_{nullptr}; + +public: + FMT_CONSTEXPR dyn_arg_storage() = default; + dyn_arg_storage(const dyn_arg_storage&) = delete; + FMT_CONSTEXPR dyn_arg_storage(dyn_arg_storage&&) = default; + + dyn_arg_storage& operator=(const dyn_arg_storage&) = delete; + FMT_CONSTEXPR dyn_arg_storage& operator=(dyn_arg_storage&&) = default; + + template + const T& emplace_front(T&& val) { + auto node{new dyn_arg_node{std::forward(val)}}; + std::unique_ptr ptr{node}; + swap(ptr, head_); + head_->next_ = std::move(ptr); + return node->value_; + } +}; } // namespace internal @@ -76,7 +104,7 @@ const T& get(const StorageValue& v) { into type-erased formatting functions such as `~fmt::vformat`. \endrst */ -template +template class dynamic_format_arg_store #if FMT_GCC_VERSION < 409 // Workaround a GCC template argument substitution bug. @@ -85,21 +113,8 @@ class dynamic_format_arg_store { private: using char_type = typename Context::char_type; - - static const bool has_custom_args = (sizeof...(Args) > 0); using string_type = std::basic_string; -#ifdef FMT_HAS_VARIANT - using storage_item_type = - conditional_t, - string_type>; -#else - static_assert(!has_custom_args, - "std::variant<> is required to support " - "custom types in dynamic_format_arg_store"); - using storage_item_type = string_type; -#endif using value_type = basic_format_arg; - using named_value_type = internal::named_arg_base; template using storaged_type = @@ -112,15 +127,10 @@ class dynamic_format_arg_store // Storage of arguments not fitting into basic_format_arg must grow // without relocation because items in data_ refer to it. - std::forward_list storage_; + internal::dyn_arg_storage storage_; friend class basic_format_args; - template const T& get_last_pushed() const { - using internal::get; - return get(storage_.front()); - } - unsigned long long get_types() const { return internal::is_unpacked_bit | data_.size(); } @@ -136,9 +146,7 @@ class dynamic_format_arg_store template const storaged_type& stored_value(const T& arg, std::true_type) { - using type = storaged_type; - storage_.emplace_front(type{arg}); - return get_last_pushed(); + return storage_.emplace_front(storaged_type{arg}); } template void emplace_arg(const T& arg) { diff --git a/test/format-dyn-args-test.cc b/test/format-dyn-args-test.cc index 94befa1f4b1b..ad7d60adf323 100644 --- a/test/format-dyn-args-test.cc +++ b/test/format-dyn-args-test.cc @@ -49,10 +49,8 @@ template <> struct formatter { }; FMT_END_NAMESPACE -#ifdef FMT_HAS_VARIANT - TEST(FormatDynArgsTest, CustomFormat) { - fmt::dynamic_format_arg_store store; + fmt::dynamic_format_arg_store store; Custom c{}; store.push_back(c); ++c.i; @@ -66,8 +64,6 @@ TEST(FormatDynArgsTest, CustomFormat) { EXPECT_EQ("cust=0 and cust=1 and cust=3", result); } -#endif // FMT_HAS_VARIANT - TEST(FormatDynArgsTest, NamedArgByRef) { fmt::dynamic_format_arg_store store; From 435133426efaf82d1065177fec4032d0cbfb66ed Mon Sep 17 00:00:00 2001 From: Vladimir Solontsov <56200572+vsolontsov-ll@users.noreply.github.com> Date: Thu, 12 Mar 2020 11:24:03 +0300 Subject: [PATCH 08/25] Update dyn-args.h Removed constexpr --- include/fmt/dyn-args.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/fmt/dyn-args.h b/include/fmt/dyn-args.h index 20a4e2ab18a6..2fd1cd684868 100644 --- a/include/fmt/dyn-args.h +++ b/include/fmt/dyn-args.h @@ -75,12 +75,12 @@ class dyn_arg_storage { std::unique_ptr head_{nullptr}; public: - FMT_CONSTEXPR dyn_arg_storage() = default; + dyn_arg_storage() = default; dyn_arg_storage(const dyn_arg_storage&) = delete; - FMT_CONSTEXPR dyn_arg_storage(dyn_arg_storage&&) = default; + dyn_arg_storage(dyn_arg_storage&&) = default; dyn_arg_storage& operator=(const dyn_arg_storage&) = delete; - FMT_CONSTEXPR dyn_arg_storage& operator=(dyn_arg_storage&&) = default; + dyn_arg_storage& operator=(dyn_arg_storage&&) = default; template const T& emplace_front(T&& val) { From 90219a772bb6b58c1fb9f14e57c2fa7e663a3be6 Mon Sep 17 00:00:00 2001 From: vsol Date: Fri, 13 Mar 2020 10:00:23 +0300 Subject: [PATCH 09/25] Build fixes --- include/fmt/dyn-args.h | 20 +++++++++----------- test/format-dyn-args-test.cc | 13 +++++++++---- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/include/fmt/dyn-args.h b/include/fmt/dyn-args.h index 2fd1cd684868..e9ac53783b47 100644 --- a/include/fmt/dyn-args.h +++ b/include/fmt/dyn-args.h @@ -11,13 +11,6 @@ #include "core.h" -#if (defined(FMT_HAS_VARIANT) || __cplusplus >= 201703L) -# include -# ifndef FMT_HAS_VARIANT -# define FMT_HAS_VARIANT -# endif -#endif - FMT_BEGIN_NAMESPACE namespace internal { @@ -61,18 +54,23 @@ template using need_dyn_copy_t = typename need_dyn_copy::type; class dyn_arg_storage { + // Workaround clang's -Wweak-vtables. For templates (unlike regular classes + // doesn't complain about inability to deduce translation unit to place vtable + // So dyn_arg_node_base is made a fake template + + template struct dyn_arg_node_base { virtual ~dyn_arg_node_base() = default; std::unique_ptr next_; }; template - struct dyn_arg_node : dyn_arg_node_base { + struct dyn_arg_node : dyn_arg_node_base<> { T value_; FMT_CONSTEXPR explicit dyn_arg_node(T&& arg) : value_{arg}{} }; - std::unique_ptr head_{nullptr}; + std::unique_ptr> head_{nullptr}; public: dyn_arg_storage() = default; @@ -84,8 +82,8 @@ class dyn_arg_storage { template const T& emplace_front(T&& val) { - auto node{new dyn_arg_node{std::forward(val)}}; - std::unique_ptr ptr{node}; + auto node = new dyn_arg_node{std::forward(val)}; + std::unique_ptr> ptr{node}; swap(ptr, head_); head_->next_ = std::move(ptr); return node->value_; diff --git a/test/format-dyn-args-test.cc b/test/format-dyn-args-test.cc index ad7d60adf323..21478865ec11 100644 --- a/test/format-dyn-args-test.cc +++ b/test/format-dyn-args-test.cc @@ -31,18 +31,18 @@ TEST(FormatDynArgsTest, StringsAndRefs) { EXPECT_EQ("1234567890 and X234567890 and X234567890", result); } -struct Custom { +struct custom_type { int i{0}; }; FMT_BEGIN_NAMESPACE -template <> struct formatter { +template <> struct formatter { auto parse(format_parse_context& ctx) const -> decltype(ctx.begin()) { return ctx.begin(); } template - auto format(const Custom& p, FormatContext& ctx) -> decltype(format_to( + auto format(const custom_type& p, FormatContext& ctx) -> decltype(format_to( ctx.out(), std::declval())) { return format_to(ctx.out(), "cust={}", p.i); } @@ -50,8 +50,13 @@ template <> struct formatter { FMT_END_NAMESPACE TEST(FormatDynArgsTest, CustomFormat) { + using context = fmt::format_context; fmt::dynamic_format_arg_store store; - Custom c{}; + static_assert(fmt::internal::need_dyn_copy_t::value, ""); + static_assert( + fmt::internal::mapped_type_constant::value == + fmt::internal::type::custom_type, ""); + custom_type c{}; store.push_back(c); ++c.i; store.push_back(c); From bee2b5f07c4fae4b66c162ca6665147d852a06c9 Mon Sep 17 00:00:00 2001 From: vsol Date: Sat, 14 Mar 2020 11:24:09 +0300 Subject: [PATCH 10/25] Preliminary cleanup. Pending -- move dyn-args.h to core.h --- include/fmt/dyn-args.h | 41 ++++++++++++++++++++---------------- test/format-dyn-args-test.cc | 4 ---- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/include/fmt/dyn-args.h b/include/fmt/dyn-args.h index e9ac53783b47..58e347dcdbae 100644 --- a/include/fmt/dyn-args.h +++ b/include/fmt/dyn-args.h @@ -58,21 +58,26 @@ class dyn_arg_storage { // doesn't complain about inability to deduce translation unit to place vtable // So dyn_arg_node_base is made a fake template - template - struct dyn_arg_node_base { - virtual ~dyn_arg_node_base() = default; - std::unique_ptr next_; + template struct storage_node_base { + using owning_ptr = std::unique_ptr>; + virtual ~storage_node_base() = default; + owning_ptr next_; }; - template - struct dyn_arg_node : dyn_arg_node_base<> { + using owning_ptr = storage_node_base<>::owning_ptr; + + template struct storage_node : storage_node_base<> { T value_; - FMT_CONSTEXPR explicit dyn_arg_node(T&& arg) : value_{arg}{} + FMT_CONSTEXPR explicit storage_node(const T& arg, owning_ptr&& next) + : value_{arg} { + // Must be initialised after value_ + next_ = std::move(next); + } }; - std::unique_ptr> head_{nullptr}; + owning_ptr head_{nullptr}; -public: + public: dyn_arg_storage() = default; dyn_arg_storage(const dyn_arg_storage&) = delete; dyn_arg_storage(dyn_arg_storage&&) = default; @@ -80,12 +85,9 @@ class dyn_arg_storage { dyn_arg_storage& operator=(const dyn_arg_storage&) = delete; dyn_arg_storage& operator=(dyn_arg_storage&&) = default; - template - const T& emplace_front(T&& val) { - auto node = new dyn_arg_node{std::forward(val)}; - std::unique_ptr> ptr{node}; - swap(ptr, head_); - head_->next_ = std::move(ptr); + template const T& push(const Arg& arg) { + auto node = new storage_node{arg, std::move(head_)}; + head_.reset(node); return node->value_; } }; @@ -115,7 +117,7 @@ class dynamic_format_arg_store using value_type = basic_format_arg; template - using storaged_type = + using stored_type = conditional_t::value, string_type, T>; // Storage of basic_format_arg must be contiguous @@ -143,8 +145,8 @@ class dynamic_format_arg_store } template - const storaged_type& stored_value(const T& arg, std::true_type) { - return storage_.emplace_front(storaged_type{arg}); + const stored_type& stored_value(const T& arg, std::true_type) { + return storage_.push>(arg); } template void emplace_arg(const T& arg) { @@ -162,6 +164,9 @@ class dynamic_format_arg_store dynamic_format_arg_store& operator=(dynamic_format_arg_store&&) = default; template void push_back(const T& arg) { + static_assert( + !std::is_base_of, T>::value, + "Named arguments are not supported yet"); emplace_arg(stored_value(arg, internal::need_dyn_copy_t{})); } diff --git a/test/format-dyn-args-test.cc b/test/format-dyn-args-test.cc index 21478865ec11..76e351d475d7 100644 --- a/test/format-dyn-args-test.cc +++ b/test/format-dyn-args-test.cc @@ -52,10 +52,6 @@ FMT_END_NAMESPACE TEST(FormatDynArgsTest, CustomFormat) { using context = fmt::format_context; fmt::dynamic_format_arg_store store; - static_assert(fmt::internal::need_dyn_copy_t::value, ""); - static_assert( - fmt::internal::mapped_type_constant::value == - fmt::internal::type::custom_type, ""); custom_type c{}; store.push_back(c); ++c.i; From 6e047c2a45057cb8828de70edceb8a5e065aa4fa Mon Sep 17 00:00:00 2001 From: vsol Date: Sat, 14 Mar 2020 11:38:08 +0300 Subject: [PATCH 11/25] minor (unused alias in test) --- test/format-dyn-args-test.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/test/format-dyn-args-test.cc b/test/format-dyn-args-test.cc index 76e351d475d7..902162904f76 100644 --- a/test/format-dyn-args-test.cc +++ b/test/format-dyn-args-test.cc @@ -50,7 +50,6 @@ template <> struct formatter { FMT_END_NAMESPACE TEST(FormatDynArgsTest, CustomFormat) { - using context = fmt::format_context; fmt::dynamic_format_arg_store store; custom_type c{}; store.push_back(c); From 70bc839b24dd35c63231dcecd27a91698bd6afdd Mon Sep 17 00:00:00 2001 From: vsol Date: Sat, 14 Mar 2020 12:27:25 +0300 Subject: [PATCH 12/25] merge dyn-args.h to core.h --- include/fmt/core.h | 191 +++++++++++++++++++++++++++++++++++ include/fmt/dyn-args.h | 180 --------------------------------- test/format-dyn-args-test.cc | 2 +- 3 files changed, 192 insertions(+), 181 deletions(-) delete mode 100644 include/fmt/dyn-args.h diff --git a/include/fmt/core.h b/include/fmt/core.h index 0669e7c12afd..993a9e966ade 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -10,9 +10,12 @@ #include // std::FILE #include +#include #include +#include #include #include +#include // The fmt library version in the form major * 10000 + minor * 100 + patch. #define FMT_VERSION 60103 @@ -1626,6 +1629,194 @@ inline void print(const S& format_str, Args&&... args) { internal::make_args_checked(format_str, args...)); #endif } + +namespace internal { + +template struct is_string_view : std::false_type {}; + +template +struct is_string_view, Char> : std::true_type {}; + +#ifdef FMT_USE_STRING_VIEW +template +struct is_string_view, Char> + : std::true_type {}; +#endif + +#ifdef FMT_USE_EXPERIMENTAL_STRING_VIEW +template +struct is_string_view, Char> + : std::true_type {}; +#endif + +template struct is_ref_wrapper : std::false_type {}; + +template +struct is_ref_wrapper> : std::true_type {}; + +template struct need_dyn_copy { + using mapped_type = mapped_type_constant; + static_assert(mapped_type::value != internal::type::named_arg_type, + "Bug indicator. Named arguments must be processed separately"); + + using type = std::integral_constant< + bool, !(is_ref_wrapper::value || + is_string_view::value || + (mapped_type::value != internal::type::cstring_type && + mapped_type::value != internal::type::custom_type && + mapped_type::value != internal::type::string_type))>; +}; + +template +using need_dyn_copy_t = typename need_dyn_copy::type; + +class dyn_arg_storage { + // Workaround clang's -Wweak-vtables. For templates (unlike regular classes + // doesn't complain about inability to deduce translation unit to place vtable + // So dyn_arg_node_base is made a fake template + + template struct storage_node_base { + using owning_ptr = std::unique_ptr>; + virtual ~storage_node_base() = default; + owning_ptr next_; + }; + + using owning_ptr = storage_node_base<>::owning_ptr; + + template struct storage_node : storage_node_base<> { + T value_; + FMT_CONSTEXPR explicit storage_node(const T& arg, owning_ptr&& next) + : value_{arg} { + // Must be initialised after value_ + next_ = std::move(next); + } + }; + + owning_ptr head_{nullptr}; + + public: + dyn_arg_storage() = default; + dyn_arg_storage(const dyn_arg_storage&) = delete; + dyn_arg_storage(dyn_arg_storage&&) = default; + + dyn_arg_storage& operator=(const dyn_arg_storage&) = delete; + dyn_arg_storage& operator=(dyn_arg_storage&&) = default; + + template const T& push(const Arg& arg) { + auto node = new storage_node{arg, std::move(head_)}; + head_.reset(node); + return node->value_; + } +}; + +} // namespace internal + +/** + \rst + A dynamic version of `fmt::format_arg_store<>`. + It's equipped with a storage to potentially temporary objects which lifetime + could be shorter than the format arguments object. + + It can be implicitly converted into `~fmt::basic_format_args` for passing + into type-erased formatting functions such as `~fmt::vformat`. + \endrst + */ +template +class dynamic_format_arg_store +#if FMT_GCC_VERSION < 409 + // Workaround a GCC template argument substitution bug. + : public basic_format_args +#endif +{ + private: + using char_type = typename Context::char_type; + using string_type = std::basic_string; + using value_type = basic_format_arg; + + template + using stored_type = + conditional_t::value, string_type, T>; + + // Storage of basic_format_arg must be contiguous + // Required by basic_format_args::args_ which is just a pointer + std::vector data_; + + // Storage of arguments not fitting into basic_format_arg must grow + // without relocation because items in data_ refer to it. + + internal::dyn_arg_storage storage_; + + friend class basic_format_args; + + unsigned long long get_types() const { + return internal::is_unpacked_bit | data_.size(); + } + + template const T& stored_value(const T& arg, std::false_type) { + return arg; + } + + template + const T& stored_value(const std::reference_wrapper& arg, std::false_type) { + return arg.get(); + } + + template + const stored_type& stored_value(const T& arg, std::true_type) { + return storage_.push>(arg); + } + + template void emplace_arg(const T& arg) { + data_.emplace_back(internal::make_arg(arg)); + } + + public: + dynamic_format_arg_store() = default; + ~dynamic_format_arg_store() = default; + + dynamic_format_arg_store(const dynamic_format_arg_store&) = delete; + dynamic_format_arg_store& operator=(const dynamic_format_arg_store&) = delete; + + dynamic_format_arg_store(dynamic_format_arg_store&&) = default; + dynamic_format_arg_store& operator=(dynamic_format_arg_store&&) = default; + + /** + \rst + Adds an argument into the dynamic store for later passing to a formating + function. + + Note that custom types and string types (but not string views!) are copied + into the store with dynamic memory (in addition to resizing vector). + + **Example**:: + + #include + fmt::dynamic_format_arg_store store; + store.push_back(42); + store.push_back("abc1"); + store.push_back(1.5f); + std::string result = fmt::vformat("{} and {} and {}", store); + \endrst + */ + template void push_back(const T& arg) { + static_assert( + !std::is_base_of, T>::value, + "Named arguments are not supported yet"); + emplace_arg(stored_value(arg, internal::need_dyn_copy_t{})); + } + + /** + \rst + Adds an argument into the dynamic store for later passing to a formating + function without copying into type-erasing list. + + Note that primitive type values are copied into basic_format_arg<>. + \endrst + */ + template void push_back(std::reference_wrapper arg) { + emplace_arg(arg.get()); + } +}; FMT_END_NAMESPACE #endif // FMT_CORE_H_ diff --git a/include/fmt/dyn-args.h b/include/fmt/dyn-args.h deleted file mode 100644 index 58e347dcdbae..000000000000 --- a/include/fmt/dyn-args.h +++ /dev/null @@ -1,180 +0,0 @@ -// Copyright (c) 2020 Vladimir Solontsov -// SPDX-License-Identifier: MIT Licence - -#ifndef FMT_DYN_ARGS_H_ -#define FMT_DYN_ARGS_H_ - -#include -#include -#include -#include - -#include "core.h" - -FMT_BEGIN_NAMESPACE - -namespace internal { - -template struct is_string_view : std::false_type {}; - -template -struct is_string_view, Char> : std::true_type {}; - -#ifdef FMT_USE_STRING_VIEW -template -struct is_string_view, Char> - : std::true_type {}; -#endif - -#ifdef FMT_USE_EXPERIMENTAL_STRING_VIEW -template -struct is_string_view, Char> - : std::true_type {}; -#endif - -template struct is_ref_wrapper : std::false_type {}; - -template -struct is_ref_wrapper> : std::true_type {}; - -template struct need_dyn_copy { - using mapped_type = mapped_type_constant; - static_assert(mapped_type::value != internal::type::named_arg_type, - "Bug indicator. Named arguments must be processed separately"); - - using type = std::integral_constant< - bool, !(is_ref_wrapper::value || - is_string_view::value || - (mapped_type::value != internal::type::cstring_type && - mapped_type::value != internal::type::custom_type && - mapped_type::value != internal::type::string_type))>; -}; - -template -using need_dyn_copy_t = typename need_dyn_copy::type; - -class dyn_arg_storage { - // Workaround clang's -Wweak-vtables. For templates (unlike regular classes - // doesn't complain about inability to deduce translation unit to place vtable - // So dyn_arg_node_base is made a fake template - - template struct storage_node_base { - using owning_ptr = std::unique_ptr>; - virtual ~storage_node_base() = default; - owning_ptr next_; - }; - - using owning_ptr = storage_node_base<>::owning_ptr; - - template struct storage_node : storage_node_base<> { - T value_; - FMT_CONSTEXPR explicit storage_node(const T& arg, owning_ptr&& next) - : value_{arg} { - // Must be initialised after value_ - next_ = std::move(next); - } - }; - - owning_ptr head_{nullptr}; - - public: - dyn_arg_storage() = default; - dyn_arg_storage(const dyn_arg_storage&) = delete; - dyn_arg_storage(dyn_arg_storage&&) = default; - - dyn_arg_storage& operator=(const dyn_arg_storage&) = delete; - dyn_arg_storage& operator=(dyn_arg_storage&&) = default; - - template const T& push(const Arg& arg) { - auto node = new storage_node{arg, std::move(head_)}; - head_.reset(node); - return node->value_; - } -}; - -} // namespace internal - -/** - \rst - A dynamic version of `fmt::format_arg_store<>`. - It's equipped with a storage to potentially temporary objects which lifetime - could be shorter than the format arguments object. - - It can be implicitly converted into `~fmt::basic_format_args` for passing - into type-erased formatting functions such as `~fmt::vformat`. - \endrst - */ -template -class dynamic_format_arg_store -#if FMT_GCC_VERSION < 409 - // Workaround a GCC template argument substitution bug. - : public basic_format_args -#endif -{ - private: - using char_type = typename Context::char_type; - using string_type = std::basic_string; - using value_type = basic_format_arg; - - template - using stored_type = - conditional_t::value, string_type, T>; - - // Storage of basic_format_arg must be contiguous - // Required by basic_format_args::args_ which is just a pointer - std::vector data_; - - // Storage of arguments not fitting into basic_format_arg must grow - // without relocation because items in data_ refer to it. - - internal::dyn_arg_storage storage_; - - friend class basic_format_args; - - unsigned long long get_types() const { - return internal::is_unpacked_bit | data_.size(); - } - - template const T& stored_value(const T& arg, std::false_type) { - return arg; - } - - template - const T& stored_value(const std::reference_wrapper& arg, std::false_type) { - return arg.get(); - } - - template - const stored_type& stored_value(const T& arg, std::true_type) { - return storage_.push>(arg); - } - - template void emplace_arg(const T& arg) { - data_.emplace_back(internal::make_arg(arg)); - } - - public: - dynamic_format_arg_store() = default; - ~dynamic_format_arg_store() = default; - - dynamic_format_arg_store(const dynamic_format_arg_store&) = delete; - dynamic_format_arg_store& operator=(const dynamic_format_arg_store&) = delete; - - dynamic_format_arg_store(dynamic_format_arg_store&&) = default; - dynamic_format_arg_store& operator=(dynamic_format_arg_store&&) = default; - - template void push_back(const T& arg) { - static_assert( - !std::is_base_of, T>::value, - "Named arguments are not supported yet"); - emplace_arg(stored_value(arg, internal::need_dyn_copy_t{})); - } - - template void push_back(std::reference_wrapper arg) { - emplace_arg(arg.get()); - } -}; - -FMT_END_NAMESPACE - -#endif /* FMT_DYN_ARGS_H_ */ diff --git a/test/format-dyn-args-test.cc b/test/format-dyn-args-test.cc index 902162904f76..da4da7ff80b9 100644 --- a/test/format-dyn-args-test.cc +++ b/test/format-dyn-args-test.cc @@ -1,7 +1,7 @@ // Copyright (c) 2020 Vladimir Solontsov // SPDX-License-Identifier: MIT Licence -#include +#include #include "gtest-extra.h" From def562c56ad8b4d5d2472b2b0621918c5422127f Mon Sep 17 00:00:00 2001 From: vsol Date: Sat, 14 Mar 2020 19:10:28 +0300 Subject: [PATCH 13/25] minor fixes --- include/fmt/core.h | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index 993a9e966ade..ac4d8292473b 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -1638,14 +1638,13 @@ template struct is_string_view, Char> : std::true_type {}; #ifdef FMT_USE_STRING_VIEW -template -struct is_string_view, Char> - : std::true_type {}; +template +struct is_string_view, Char> : std::true_type {}; #endif #ifdef FMT_USE_EXPERIMENTAL_STRING_VIEW -template -struct is_string_view, Char> +template +struct is_string_view, Char> : std::true_type {}; #endif @@ -1655,25 +1654,22 @@ template struct is_ref_wrapper> : std::true_type {}; template struct need_dyn_copy { - using mapped_type = mapped_type_constant; - static_assert(mapped_type::value != internal::type::named_arg_type, + static constexpr auto mapped_type = mapped_type_constant::value; + static_assert(mapped_type != internal::type::named_arg_type, "Bug indicator. Named arguments must be processed separately"); using type = std::integral_constant< bool, !(is_ref_wrapper::value || is_string_view::value || - (mapped_type::value != internal::type::cstring_type && - mapped_type::value != internal::type::custom_type && - mapped_type::value != internal::type::string_type))>; + (mapped_type != internal::type::cstring_type && + mapped_type != internal::type::custom_type && + mapped_type != internal::type::string_type))>; }; -template -using need_dyn_copy_t = typename need_dyn_copy::type; - class dyn_arg_storage { // Workaround clang's -Wweak-vtables. For templates (unlike regular classes // doesn't complain about inability to deduce translation unit to place vtable - // So dyn_arg_node_base is made a fake template + // So storage_node_base is made a fake template. template struct storage_node_base { using owning_ptr = std::unique_ptr>; @@ -1730,16 +1726,14 @@ class dynamic_format_arg_store { private: using char_type = typename Context::char_type; - using string_type = std::basic_string; - using value_type = basic_format_arg; template - using stored_type = - conditional_t::value, string_type, T>; + using stored_type = conditional_t::value, + std::basic_string, T>; // Storage of basic_format_arg must be contiguous - // Required by basic_format_args::args_ which is just a pointer - std::vector data_; + // Required by basic_format_args::args_ which is just a pointer. + std::vector> data_; // Storage of arguments not fitting into basic_format_arg must grow // without relocation because items in data_ refer to it. @@ -1802,7 +1796,8 @@ class dynamic_format_arg_store static_assert( !std::is_base_of, T>::value, "Named arguments are not supported yet"); - emplace_arg(stored_value(arg, internal::need_dyn_copy_t{})); + emplace_arg(stored_value( + arg, typename internal::need_dyn_copy::type{})); } /** From 49a46f8d5034cc2d48c966734484674578d4f13b Mon Sep 17 00:00:00 2001 From: vsol Date: Sat, 14 Mar 2020 19:16:32 +0300 Subject: [PATCH 14/25] Removed conditional compilation as there's already the `internal::std_string_veiew` alias --- include/fmt/core.h | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index ac4d8292473b..34ff34051339 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -1637,16 +1637,8 @@ template struct is_string_view : std::false_type {}; template struct is_string_view, Char> : std::true_type {}; -#ifdef FMT_USE_STRING_VIEW template -struct is_string_view, Char> : std::true_type {}; -#endif - -#ifdef FMT_USE_EXPERIMENTAL_STRING_VIEW -template -struct is_string_view, Char> - : std::true_type {}; -#endif +struct is_string_view, Char> : std::true_type {}; template struct is_ref_wrapper : std::false_type {}; From 863aa6343333ca3b89208a89cfd89451510f11a3 Mon Sep 17 00:00:00 2001 From: vsol Date: Sat, 14 Mar 2020 19:28:43 +0300 Subject: [PATCH 15/25] trying to fix MSVC --- include/fmt/core.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index 34ff34051339..e4230347e7dc 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -1646,7 +1646,8 @@ template struct is_ref_wrapper> : std::true_type {}; template struct need_dyn_copy { - static constexpr auto mapped_type = mapped_type_constant::value; + static constexpr internal::type mapped_type = + mapped_type_constant::value; static_assert(mapped_type != internal::type::named_arg_type, "Bug indicator. Named arguments must be processed separately"); From 473ba57fcfb355984dc79c7da29f8c347625fc05 Mon Sep 17 00:00:00 2001 From: vsol Date: Sat, 14 Mar 2020 20:51:41 +0300 Subject: [PATCH 16/25] Fixed comments Changed dispatching from tag to explicit `if`. However I would prefer tag-based dispatching if `if constexpr`, but the later is not available in older standards. --- include/fmt/core.h | 36 +++++++++++++++++++----------------- include/fmt/format.h | 4 ---- 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index e4230347e7dc..388e7b97798b 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -272,6 +272,10 @@ struct monostate {}; namespace internal { +// A helper function to suppress bogus "conditional expression is constant" +// warnings. +template FMT_CONSTEXPR T const_check(T value) { return value; } + // A workaround for gcc 4.8 to make void_t work in a SFINAE context. template struct void_t_impl { using type = void; }; @@ -1674,11 +1678,20 @@ class dyn_arg_storage { template struct storage_node : storage_node_base<> { T value_; - FMT_CONSTEXPR explicit storage_node(const T& arg, owning_ptr&& next) + template + FMT_CONSTEXPR storage_node(const Arg& arg, owning_ptr&& next) : value_{arg} { // Must be initialised after value_ next_ = std::move(next); } + + template + FMT_CONSTEXPR storage_node(const basic_string_view& arg, + owning_ptr&& next) + : value_{arg.data(), arg.size()} { + // Must be initialised after value_ + next_ = std::move(next); + } }; owning_ptr head_{nullptr}; @@ -1739,20 +1752,6 @@ class dynamic_format_arg_store return internal::is_unpacked_bit | data_.size(); } - template const T& stored_value(const T& arg, std::false_type) { - return arg; - } - - template - const T& stored_value(const std::reference_wrapper& arg, std::false_type) { - return arg.get(); - } - - template - const stored_type& stored_value(const T& arg, std::true_type) { - return storage_.push>(arg); - } - template void emplace_arg(const T& arg) { data_.emplace_back(internal::make_arg(arg)); } @@ -1789,8 +1788,11 @@ class dynamic_format_arg_store static_assert( !std::is_base_of, T>::value, "Named arguments are not supported yet"); - emplace_arg(stored_value( - arg, typename internal::need_dyn_copy::type{})); + using need_copy_t = typename internal::need_dyn_copy::type; + if (internal::const_check(need_copy_t::value)) + emplace_arg(storage_.push>(arg)); + else + emplace_arg(arg); } /** diff --git a/include/fmt/format.h b/include/fmt/format.h index 07d4f5f8731f..5a479732bc29 100644 --- a/include/fmt/format.h +++ b/include/fmt/format.h @@ -213,10 +213,6 @@ FMT_END_NAMESPACE FMT_BEGIN_NAMESPACE namespace internal { -// A helper function to suppress bogus "conditional expression is constant" -// warnings. -template FMT_CONSTEXPR T const_check(T value) { return value; } - // An equivalent of `*reinterpret_cast(&source)` that doesn't have // undefined behavior (e.g. due to type aliasing). // Example: uint64_t d = bit_cast(2.718); From ae512cd100dfe0d2e2e3105eba6bd2a4d02129e7 Mon Sep 17 00:00:00 2001 From: vsol Date: Mon, 16 Mar 2020 10:37:05 +0300 Subject: [PATCH 17/25] Fixes after review --- include/fmt/core.h | 70 +++++++++++++++++++++++----------------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index 388e7b97798b..dd71e5ad1c30 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -1644,57 +1644,40 @@ struct is_string_view, Char> : std::true_type {}; template struct is_string_view, Char> : std::true_type {}; -template struct is_ref_wrapper : std::false_type {}; +template struct is_reference_wrapper : std::false_type {}; template -struct is_ref_wrapper> : std::true_type {}; - -template struct need_dyn_copy { - static constexpr internal::type mapped_type = - mapped_type_constant::value; - static_assert(mapped_type != internal::type::named_arg_type, - "Bug indicator. Named arguments must be processed separately"); - - using type = std::integral_constant< - bool, !(is_ref_wrapper::value || - is_string_view::value || - (mapped_type != internal::type::cstring_type && - mapped_type != internal::type::custom_type && - mapped_type != internal::type::string_type))>; -}; +struct is_reference_wrapper> : std::true_type {}; class dyn_arg_storage { - // Workaround clang's -Wweak-vtables. For templates (unlike regular classes - // doesn't complain about inability to deduce translation unit to place vtable - // So storage_node_base is made a fake template. + // Workaround for clang's -Wweak-vtables. Unlike for regular classes, for + // templates it doesn't complain about inability to deduce single translation + // unit for placing vtable. So storage_node_base is made a fake template. template struct storage_node_base { using owning_ptr = std::unique_ptr>; virtual ~storage_node_base() = default; - owning_ptr next_; + owning_ptr next; }; - using owning_ptr = storage_node_base<>::owning_ptr; - template struct storage_node : storage_node_base<> { - T value_; + T value; template - FMT_CONSTEXPR storage_node(const Arg& arg, owning_ptr&& next) - : value_{arg} { + FMT_CONSTEXPR storage_node(const Arg& arg, owning_ptr&& next) : value{arg} { // Must be initialised after value_ - next_ = std::move(next); + this->next = std::move(next); } template FMT_CONSTEXPR storage_node(const basic_string_view& arg, owning_ptr&& next) - : value_{arg.data(), arg.size()} { - // Must be initialised after value_ - next_ = std::move(next); + : value{arg.data(), arg.size()} { + // Must be initialised after value + this->next = std::move(next); } }; - owning_ptr head_{nullptr}; + storage_node_base<>::owning_ptr head_{nullptr}; public: dyn_arg_storage() = default; @@ -1707,7 +1690,7 @@ class dyn_arg_storage { template const T& push(const Arg& arg) { auto node = new storage_node{arg, std::move(head_)}; head_.reset(node); - return node->value_; + return node->value; } }; @@ -1733,6 +1716,22 @@ class dynamic_format_arg_store private: using char_type = typename Context::char_type; + template struct need_dyn_copy { + static constexpr internal::type mapped_type = + internal::mapped_type_constant::value; +// static_assert( +// mapped_type != internal::type::named_arg_type, +// "Bug indicator. Named arguments must be processed separately"); + + using type = std::integral_constant< + bool, !(internal::is_reference_wrapper::value || + internal::is_string_view::value || + (mapped_type != internal::type::cstring_type && + mapped_type != internal::type::string_type && + mapped_type != internal::type::custom_type && + mapped_type != internal::type::named_arg_type))>; + }; + template using stored_type = conditional_t::value, std::basic_string, T>; @@ -1788,8 +1787,7 @@ class dynamic_format_arg_store static_assert( !std::is_base_of, T>::value, "Named arguments are not supported yet"); - using need_copy_t = typename internal::need_dyn_copy::type; - if (internal::const_check(need_copy_t::value)) + if (internal::const_check(need_dyn_copy::type::value)) emplace_arg(storage_.push>(arg)); else emplace_arg(arg); @@ -1799,11 +1797,13 @@ class dynamic_format_arg_store \rst Adds an argument into the dynamic store for later passing to a formating function without copying into type-erasing list. - - Note that primitive type values are copied into basic_format_arg<>. \endrst */ template void push_back(std::reference_wrapper arg) { + static_assert( + need_dyn_copy::type::value, + "Primitive types and string views directly supported by " + "basic_format_arg. Passing them by reference is not allowed"); emplace_arg(arg.get()); } }; From c902f34a9accfc579a75159467ad1d4983b087ed Mon Sep 17 00:00:00 2001 From: vsol Date: Sun, 3 May 2020 20:58:23 +0300 Subject: [PATCH 18/25] Support named args in dynamic_format_arg_store (#1655). First cut. --- include/fmt/core.h | 133 ++++++++++++++++++++++++----------- test/CMakeLists.txt | 1 + test/format-dyn-args-test.cc | 47 +++++++++++++ 3 files changed, 138 insertions(+), 43 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index 2e5a51fec1da..44cd025d41e9 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -629,6 +629,7 @@ using wparse_context FMT_DEPRECATED_ALIAS = basic_format_parse_context; template class basic_format_arg; template class basic_format_args; +template class dynamic_format_arg_store; // A formatter for objects of type T. template @@ -1131,6 +1132,7 @@ template class basic_format_arg { friend class basic_format_args; friend class internal::arg_map; + friend class dynamic_format_arg_store; using char_type = typename Context::char_type; @@ -1419,6 +1421,50 @@ inline format_arg_store make_format_args( return {args...}; } +namespace internal { +template struct named_arg_base { + const Char* name; + + // Serialized value. + mutable char data[sizeof(basic_format_arg>)]; + + named_arg_base(const Char* nm) : name(nm) {} + + template basic_format_arg deserialize() const { + basic_format_arg arg; + std::memcpy(&arg, data, sizeof(basic_format_arg)); + return arg; + } +}; + +struct view {}; + +template +struct named_arg : view, named_arg_base { + const T& value; + + named_arg(const Char* name, const T& val) + : named_arg_base(name), value(val) {} +}; + +} // namespace internal + +/** + \rst + Returns a named argument to be used in a formatting function. It should only + be used in a call to a formatting function. + + **Example**:: + + fmt::print("Elapsed time: {s:.2f} seconds", fmt::arg("s", 1.23)); + \endrst + */ +template +inline internal::named_arg arg(const Char* name, const T& arg) { + static_assert(!internal::is_named_arg(), "nested named arguments"); + return {name, arg}; +} + /** \rst A dynamic version of `fmt::format_arg_store<>`. @@ -1460,6 +1506,7 @@ class dynamic_format_arg_store // Storage of basic_format_arg must be contiguous. std::vector> data_; + std::vector> named_info_; // Storage of arguments not fitting into basic_format_arg must grow // without relocation because items in data_ refer to it. @@ -1468,13 +1515,32 @@ class dynamic_format_arg_store friend class basic_format_args; unsigned long long get_types() const { - return internal::is_unpacked_bit | data_.size(); + return internal::is_unpacked_bit | data_.size() | + (named_info_.empty() ? 0ULL : internal::has_named_args_bit); + } + + const basic_format_arg* data() const { + return named_info_.empty() ? data_.data() : data_.data() + 1; } template void emplace_arg(const T& arg) { data_.emplace_back(internal::make_arg(arg)); } + template + void emplace_arg(const internal::named_arg& arg) { + if (named_info_.empty()) + data_.insert(data_.begin(), basic_format_arg{}); + data_.emplace_back(internal::make_arg(arg)); + try { + named_info_.push_back({arg.name, static_cast(data_.size() - 2u)}); + data_[0].value_.named_args = {named_info_.data(), named_info_.size()}; + } catch (...) { + data_.pop_back(); + throw; + } + } + public: /** \rst @@ -1503,6 +1569,18 @@ class dynamic_format_arg_store emplace_arg(arg); } + template + void push_back(const internal::named_arg& arg) { + const char_type* arg_name = + dynamic_args_.push>(arg.name).c_str(); + if (internal::const_check(need_copy::value)) { + emplace_arg( + fmt::arg(arg_name, dynamic_args_.push>(arg.value))); + } + else + emplace_arg(fmt::arg(arg_name, arg.value)); + } + /** Adds a reference to the argument into the dynamic store for later passing to a formating function. @@ -1513,6 +1591,16 @@ class dynamic_format_arg_store "objects of built-in types and string views are always copied"); emplace_arg(arg.get()); } + + /** + Adds a reference to the argument into the dynamic store for later passing to + a formating function. + */ + template + void push_back( + std::reference_wrapper> arg) { + emplace_arg(arg.get()); + } }; /** @@ -1597,7 +1685,7 @@ template class basic_format_args { \endrst */ FMT_INLINE basic_format_args(const dynamic_format_arg_store& store) - : basic_format_args(store.get_types(), store.data_.data()) {} + : basic_format_args(store.get_types(), store.data()) {} /** \rst @@ -1659,31 +1747,6 @@ template struct is_contiguous_back_insert_iterator> : is_contiguous {}; -template struct named_arg_base { - const Char* name; - - // Serialized value. - mutable char data[sizeof(basic_format_arg>)]; - - named_arg_base(const Char* nm) : name(nm) {} - - template basic_format_arg deserialize() const { - basic_format_arg arg; - std::memcpy(&arg, data, sizeof(basic_format_arg)); - return arg; - } -}; - -struct view {}; - -template -struct named_arg : view, named_arg_base { - const T& value; - - named_arg(const Char* name, const T& val) - : named_arg_base(name), value(val) {} -}; - // Reports a compile-time error if S is not a valid format string. template ::value)> FMT_INLINE void check_format_string(const S&) { @@ -1727,22 +1790,6 @@ inline void vprint_mojibake(std::FILE*, string_view, format_args) {} #endif } // namespace internal -/** - \rst - Returns a named argument to be used in a formatting function. It should only - be used in a call to a formatting function. - - **Example**:: - - fmt::print("Elapsed time: {s:.2f} seconds", fmt::arg("s", 1.23)); - \endrst - */ -template -inline internal::named_arg arg(const Char* name, const T& arg) { - static_assert(!internal::is_named_arg(), "nested named arguments"); - return {name, arg}; -} - /** Formats a string and writes the output to ``out``. */ // GCC 8 and earlier cannot handle std::back_insert_iterator with // vformat_to(...) overload, so SFINAE on iterator type instead. diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 89176633115c..23ea9fcfb95e 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -95,6 +95,7 @@ add_fmt_test(grisu-test) target_compile_definitions(grisu-test PRIVATE FMT_USE_GRISU=1) add_fmt_test(gtest-extra-test) add_fmt_test(format-test mock-allocator.h) +add_fmt_test(format-dyn-args-test) if (MSVC) target_compile_options(format-test PRIVATE /bigobj) endif () diff --git a/test/format-dyn-args-test.cc b/test/format-dyn-args-test.cc index acc5ef782875..b2c4c5f2672d 100644 --- a/test/format-dyn-args-test.cc +++ b/test/format-dyn-args-test.cc @@ -4,3 +4,50 @@ #include #include "gtest-extra.h" + +TEST(FormatDynArgsTest, NamedInt) { + fmt::dynamic_format_arg_store store; + store.push_back(fmt::arg("a1", 42)); + std::string result = fmt::vformat("{a1}", store); + EXPECT_EQ("42", result); +} + +TEST(FormatDynArgsTest, NamedStrings) { + fmt::dynamic_format_arg_store store; + char str[]{"1234567890"}; + store.push_back(fmt::arg("a1", str)); + store.push_back(fmt::arg("a2", std::cref(str))); + str[0] = 'X'; + + std::string result = fmt::vformat( + "{a1} and {a2}", + store); + + EXPECT_EQ("1234567890 and X234567890", result); +} + +TEST(FormatDynArgsTest, NamedArgByRef) { + fmt::dynamic_format_arg_store store; + + // Note: fmt::arg() constructs an object which holds a reference + // to its value. It's not an aggregate, so it doesn't extend the + // reference lifetime. As a result, it's a very bad idea passing temporary + // as a named argument value. Only GCC with optimization level >0 + // complains about this. + // + // A real life usecase is when you have both name and value alive + // guarantee their lifetime and thus don't want them to be copied into + // storages. + int a1_val{42}; + auto a1 = fmt::arg("a1_", a1_val); + store.push_back("abc"); + store.push_back(1.5f); + store.push_back(std::cref(a1)); + + std::string result = fmt::vformat( + "{a1_} and {} and {} and {}", + store); + + EXPECT_EQ("42 and abc and 1.5 and 42", result); +} + From efbcb27d9bad47c851bb3d1f2ebaa3c02fffea53 Mon Sep 17 00:00:00 2001 From: vsol Date: Sun, 3 May 2020 22:27:42 +0300 Subject: [PATCH 19/25] fix build: exceptions and cast... --- include/fmt/core.h | 45 ++++++++++++++++++++++++++++++++++++++++---- include/fmt/format.h | 34 --------------------------------- 2 files changed, 41 insertions(+), 38 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index 44cd025d41e9..e6bec17ee230 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -117,6 +117,42 @@ # endif #endif +#ifndef FMT_THROW +# if FMT_EXCEPTIONS +# if FMT_MSC_VER || FMT_NVCC +FMT_BEGIN_NAMESPACE +namespace internal { +template inline void do_throw(const Exception& x) { + // Silence unreachable code warnings in MSVC and NVCC because these + // are nearly impossible to fix in a generic code. + volatile bool b = true; + if (b) throw x; +} +} // namespace internal +FMT_END_NAMESPACE +# define FMT_THROW(x) internal::do_throw(x) +# else +# define FMT_THROW(x) throw x +# endif +# define FMT_RETHROW() throw +# else +# define FMT_THROW(x) \ + do { \ + static_cast(sizeof(x)); \ + FMT_ASSERT(false, ""); \ + } while (false) +# define FMT_RETHROW() FMT_ASSERT(false, "") +# endif +#endif + +#if FMT_EXCEPTIONS +# define FMT_TRY try +# define FMT_CATCH(x) catch (x) +#else +# define FMT_TRY if (true) +# define FMT_CATCH(x) if (false) +#endif + // Define FMT_USE_NOEXCEPT to make fmt use noexcept (C++11 feature). #ifndef FMT_USE_NOEXCEPT # define FMT_USE_NOEXCEPT 0 @@ -1516,7 +1552,8 @@ class dynamic_format_arg_store unsigned long long get_types() const { return internal::is_unpacked_bit | data_.size() | - (named_info_.empty() ? 0ULL : internal::has_named_args_bit); + (named_info_.empty() ? 0ULL : + static_cast(internal::has_named_args_bit)); } const basic_format_arg* data() const { @@ -1532,12 +1569,12 @@ class dynamic_format_arg_store if (named_info_.empty()) data_.insert(data_.begin(), basic_format_arg{}); data_.emplace_back(internal::make_arg(arg)); - try { + FMT_TRY { named_info_.push_back({arg.name, static_cast(data_.size() - 2u)}); data_[0].value_.named_args = {named_info_.data(), named_info_.size()}; - } catch (...) { + } FMT_CATCH(...) { data_.pop_back(); - throw; + FMT_RETHROW(); } } diff --git a/include/fmt/format.h b/include/fmt/format.h index a087ca31c594..fd891ca80460 100644 --- a/include/fmt/format.h +++ b/include/fmt/format.h @@ -88,40 +88,6 @@ # define FMT_FALLTHROUGH #endif -#ifndef FMT_THROW -# if FMT_EXCEPTIONS -# if FMT_MSC_VER || FMT_NVCC -FMT_BEGIN_NAMESPACE -namespace internal { -template inline void do_throw(const Exception& x) { - // Silence unreachable code warnings in MSVC and NVCC because these - // are nearly impossible to fix in a generic code. - volatile bool b = true; - if (b) throw x; -} -} // namespace internal -FMT_END_NAMESPACE -# define FMT_THROW(x) internal::do_throw(x) -# else -# define FMT_THROW(x) throw x -# endif -# else -# define FMT_THROW(x) \ - do { \ - static_cast(sizeof(x)); \ - FMT_ASSERT(false, ""); \ - } while (false) -# endif -#endif - -#if FMT_EXCEPTIONS -# define FMT_TRY try -# define FMT_CATCH(x) catch (x) -#else -# define FMT_TRY if (true) -# define FMT_CATCH(x) if (false) -#endif - #ifndef FMT_USE_USER_DEFINED_LITERALS // For Intel and NVIDIA compilers both they and the system gcc/msc support UDLs. # if (FMT_HAS_FEATURE(cxx_user_literals) || FMT_GCC_VERSION >= 407 || \ From e46c0a45b8da3917c5ca3dec15e99d0a2de8e2fb Mon Sep 17 00:00:00 2001 From: vsol Date: Mon, 4 May 2020 12:40:48 +0300 Subject: [PATCH 20/25] Fix namespace issue around exceptions marcos for MSVC --- include/fmt/core.h | 72 +++++++++++++++++++++++----------------------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index e6bec17ee230..9362642b5527 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -117,42 +117,6 @@ # endif #endif -#ifndef FMT_THROW -# if FMT_EXCEPTIONS -# if FMT_MSC_VER || FMT_NVCC -FMT_BEGIN_NAMESPACE -namespace internal { -template inline void do_throw(const Exception& x) { - // Silence unreachable code warnings in MSVC and NVCC because these - // are nearly impossible to fix in a generic code. - volatile bool b = true; - if (b) throw x; -} -} // namespace internal -FMT_END_NAMESPACE -# define FMT_THROW(x) internal::do_throw(x) -# else -# define FMT_THROW(x) throw x -# endif -# define FMT_RETHROW() throw -# else -# define FMT_THROW(x) \ - do { \ - static_cast(sizeof(x)); \ - FMT_ASSERT(false, ""); \ - } while (false) -# define FMT_RETHROW() FMT_ASSERT(false, "") -# endif -#endif - -#if FMT_EXCEPTIONS -# define FMT_TRY try -# define FMT_CATCH(x) catch (x) -#else -# define FMT_TRY if (true) -# define FMT_CATCH(x) if (false) -#endif - // Define FMT_USE_NOEXCEPT to make fmt use noexcept (C++11 feature). #ifndef FMT_USE_NOEXCEPT # define FMT_USE_NOEXCEPT 0 @@ -240,6 +204,42 @@ FMT_END_NAMESPACE FMT_INLINE_NAMESPACE v6 { #endif +#ifndef FMT_THROW +# if FMT_EXCEPTIONS +# if FMT_MSC_VER || FMT_NVCC +FMT_BEGIN_NAMESPACE +namespace internal { +template inline void do_throw(const Exception& x) { + // Silence unreachable code warnings in MSVC and NVCC because these + // are nearly impossible to fix in a generic code. + volatile bool b = true; + if (b) throw x; +} +} // namespace internal +FMT_END_NAMESPACE +# define FMT_THROW(x) internal::do_throw(x) +# else +# define FMT_THROW(x) throw x +# endif +# define FMT_RETHROW() throw +# else +# define FMT_THROW(x) \ + do { \ + static_cast(sizeof(x)); \ + FMT_ASSERT(false, ""); \ + } while (false) +# define FMT_RETHROW() FMT_ASSERT(false, "") +# endif +#endif + +#if FMT_EXCEPTIONS +# define FMT_TRY try +# define FMT_CATCH(x) catch (x) +#else +# define FMT_TRY if (true) +# define FMT_CATCH(x) if (false) +#endif + #if !defined(FMT_HEADER_ONLY) && defined(_WIN32) # define FMT_CLASS_API FMT_SUPPRESS_MSC_WARNING(4275) # ifdef FMT_EXPORT From 6199e2a02d1f31161a62d5b80ff87252a95f7e17 Mon Sep 17 00:00:00 2001 From: vsol Date: Mon, 4 May 2020 14:40:18 +0300 Subject: [PATCH 21/25] Named arguments. Better wupport for std::reference_wrapper. --- include/fmt/core.h | 80 +++++++++++++++++++++++------------- test/core-test.cc | 60 +++++++++++++++++++++++++++ test/format-dyn-args-test.cc | 47 --------------------- 3 files changed, 112 insertions(+), 75 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index 9362642b5527..dad05be988d1 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -1311,6 +1311,13 @@ template struct is_reference_wrapper : std::false_type {}; template struct is_reference_wrapper> : std::true_type {}; +template struct unwrap_reference { using type = T; }; +template struct unwrap_reference> { + using type = T&; +}; +template +using unwrap_reference_t = typename unwrap_reference::type; + class dynamic_arg_list { // Workaround for clang's -Wweak-vtables. Unlike for regular classes, for // templates it doesn't complain about inability to deduce single translation @@ -1531,8 +1538,7 @@ class dynamic_format_arg_store std::is_same>::value || (mapped_type != internal::type::cstring_type && mapped_type != internal::type::string_type && - mapped_type != internal::type::custom_type && - mapped_type != internal::type::named_arg_type)) + mapped_type != internal::type::custom_type)) }; }; @@ -1552,8 +1558,9 @@ class dynamic_format_arg_store unsigned long long get_types() const { return internal::is_unpacked_bit | data_.size() | - (named_info_.empty() ? 0ULL : - static_cast(internal::has_named_args_bit)); + (named_info_.empty() ? 0ULL + : static_cast( + internal::has_named_args_bit)); } const basic_format_arg* data() const { @@ -1566,13 +1573,19 @@ class dynamic_format_arg_store template void emplace_arg(const internal::named_arg& arg) { - if (named_info_.empty()) - data_.insert(data_.begin(), basic_format_arg{}); - data_.emplace_back(internal::make_arg(arg)); + if (named_info_.empty()) { + data_.insert( + data_.begin(), + {static_cast*>(nullptr), + 0}); + } + data_.emplace_back(internal::make_arg( + static_cast&>(arg.value))); FMT_TRY { named_info_.push_back({arg.name, static_cast(data_.size() - 2u)}); data_[0].value_.named_args = {named_info_.data(), named_info_.size()}; - } FMT_CATCH(...) { + } + FMT_CATCH(...) { data_.pop_back(); FMT_RETHROW(); } @@ -1603,40 +1616,51 @@ class dynamic_format_arg_store if (internal::const_check(need_copy::value)) emplace_arg(dynamic_args_.push>(arg)); else - emplace_arg(arg); - } - - template - void push_back(const internal::named_arg& arg) { - const char_type* arg_name = - dynamic_args_.push>(arg.name).c_str(); - if (internal::const_check(need_copy::value)) { - emplace_arg( - fmt::arg(arg_name, dynamic_args_.push>(arg.value))); - } - else - emplace_arg(fmt::arg(arg_name, arg.value)); + emplace_arg(static_cast&>(arg)); } /** + \rst Adds a reference to the argument into the dynamic store for later passing to - a formating function. + a formating function. Supports wrapped named arguments. + + **Example**:: + fmt::dynamic_format_arg_store store; + char str[] = "1234567890"; + store.push_back(std::cref(str)); + int a1_val{42}; + auto a1 = fmt::arg("a1_", a1_val); + store.push_back(std::cref(a1)); + + // Changing str affects the output but only for string and custom types. + str[0] = 'X'; + + std::string result = fmt::vformat("{} and {a1_}"); + assert(result == "X234567890 and 42"); + \endrst */ template void push_back(std::reference_wrapper arg) { static_assert( - need_copy::value, + internal::is_named_arg::type>::value || + need_copy::value, "objects of built-in types and string views are always copied"); emplace_arg(arg.get()); } /** - Adds a reference to the argument into the dynamic store for later passing to - a formating function. + Adds named argument into the dynamic store for later passing to a formating + function. std::reference_wrapper is supported to avoid copying of the + argument. */ template - void push_back( - std::reference_wrapper> arg) { - emplace_arg(arg.get()); + void push_back(const internal::named_arg& arg) { + const char_type* arg_name = + dynamic_args_.push>(arg.name).c_str(); + if (internal::const_check(need_copy::value)) { + emplace_arg( + fmt::arg(arg_name, dynamic_args_.push>(arg.value))); + } else + emplace_arg(fmt::arg(arg_name, arg.value)); } }; diff --git a/test/core-test.cc b/test/core-test.cc index adda8943b49d..8c2f7bd9e878 100644 --- a/test/core-test.cc +++ b/test/core-test.cc @@ -456,6 +456,66 @@ TEST(FormatDynArgsTest, CustomFormat) { EXPECT_EQ("cust=0 and cust=1 and cust=3", result); } +TEST(FormatDynArgsTest, NamedInt) { + fmt::dynamic_format_arg_store store; + store.push_back(fmt::arg("a1", 42)); + std::string result = fmt::vformat("{a1}", store); + EXPECT_EQ("42", result); +} + +TEST(FormatDynArgsTest, NamedStrings) { + fmt::dynamic_format_arg_store store; + char str[]{"1234567890"}; + store.push_back(fmt::arg("a1", str)); + store.push_back(fmt::arg("a2", std::cref(str))); + str[0] = 'X'; + + std::string result = fmt::vformat( + "{a1} and {a2}", + store); + + EXPECT_EQ("1234567890 and X234567890", result); +} + +TEST(FormatDynArgsTest, NamedArgByRef) { + fmt::dynamic_format_arg_store store; + + // Note: fmt::arg() constructs an object which holds a reference + // to its value. It's not an aggregate, so it doesn't extend the + // reference lifetime. As a result, it's a very bad idea passing temporary + // as a named argument value. Only GCC with optimization level >0 + // complains about this. + // + // A real life usecase is when you have both name and value alive + // guarantee their lifetime and thus don't want them to be copied into + // storages. + int a1_val{42}; + auto a1 = fmt::arg("a1_", a1_val); + store.push_back("abc"); + store.push_back(1.5f); + store.push_back(std::cref(a1)); + + std::string result = fmt::vformat( + "{a1_} and {} and {} and {}", + store); + + EXPECT_EQ("42 and abc and 1.5 and 42", result); +} + +TEST(FormatDynArgsTest, NamedCustomFormat) { + fmt::dynamic_format_arg_store store; + custom_type c{}; + store.push_back(fmt::arg("c1", c)); + ++c.i; + store.push_back(fmt::arg("c2", c)); + ++c.i; + store.push_back(fmt::arg("c_ref", std::cref(c))); + ++c.i; + + std::string result = fmt::vformat("{c1} and {c2} and {c_ref}", store); + EXPECT_EQ("cust=0 and cust=1 and cust=3", result); +} + struct copy_throwable { copy_throwable() {} copy_throwable(const copy_throwable&) { throw "deal with it"; } diff --git a/test/format-dyn-args-test.cc b/test/format-dyn-args-test.cc index b2c4c5f2672d..acc5ef782875 100644 --- a/test/format-dyn-args-test.cc +++ b/test/format-dyn-args-test.cc @@ -4,50 +4,3 @@ #include #include "gtest-extra.h" - -TEST(FormatDynArgsTest, NamedInt) { - fmt::dynamic_format_arg_store store; - store.push_back(fmt::arg("a1", 42)); - std::string result = fmt::vformat("{a1}", store); - EXPECT_EQ("42", result); -} - -TEST(FormatDynArgsTest, NamedStrings) { - fmt::dynamic_format_arg_store store; - char str[]{"1234567890"}; - store.push_back(fmt::arg("a1", str)); - store.push_back(fmt::arg("a2", std::cref(str))); - str[0] = 'X'; - - std::string result = fmt::vformat( - "{a1} and {a2}", - store); - - EXPECT_EQ("1234567890 and X234567890", result); -} - -TEST(FormatDynArgsTest, NamedArgByRef) { - fmt::dynamic_format_arg_store store; - - // Note: fmt::arg() constructs an object which holds a reference - // to its value. It's not an aggregate, so it doesn't extend the - // reference lifetime. As a result, it's a very bad idea passing temporary - // as a named argument value. Only GCC with optimization level >0 - // complains about this. - // - // A real life usecase is when you have both name and value alive - // guarantee their lifetime and thus don't want them to be copied into - // storages. - int a1_val{42}; - auto a1 = fmt::arg("a1_", a1_val); - store.push_back("abc"); - store.push_back(1.5f); - store.push_back(std::cref(a1)); - - std::string result = fmt::vformat( - "{a1_} and {} and {} and {}", - store); - - EXPECT_EQ("42 and abc and 1.5 and 42", result); -} - From ced27f1232773fdcaefa2185df6d5e0c9e9d0fee Mon Sep 17 00:00:00 2001 From: vsol Date: Thu, 7 May 2020 10:55:29 +0300 Subject: [PATCH 22/25] Fix review comments --- include/fmt/core.h | 79 +++++++++++++++++++------------------------- include/fmt/format.h | 34 +++++++++++++++++++ test/CMakeLists.txt | 1 - 3 files changed, 68 insertions(+), 46 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index dad05be988d1..11d65d9be887 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -204,42 +204,6 @@ FMT_INLINE_NAMESPACE v6 { #endif -#ifndef FMT_THROW -# if FMT_EXCEPTIONS -# if FMT_MSC_VER || FMT_NVCC -FMT_BEGIN_NAMESPACE -namespace internal { -template inline void do_throw(const Exception& x) { - // Silence unreachable code warnings in MSVC and NVCC because these - // are nearly impossible to fix in a generic code. - volatile bool b = true; - if (b) throw x; -} -} // namespace internal -FMT_END_NAMESPACE -# define FMT_THROW(x) internal::do_throw(x) -# else -# define FMT_THROW(x) throw x -# endif -# define FMT_RETHROW() throw -# else -# define FMT_THROW(x) \ - do { \ - static_cast(sizeof(x)); \ - FMT_ASSERT(false, ""); \ - } while (false) -# define FMT_RETHROW() FMT_ASSERT(false, "") -# endif -#endif - -#if FMT_EXCEPTIONS -# define FMT_TRY try -# define FMT_CATCH(x) catch (x) -#else -# define FMT_TRY if (true) -# define FMT_CATCH(x) if (false) -#endif - #if !defined(FMT_HEADER_ONLY) && defined(_WIN32) # define FMT_CLASS_API FMT_SUPPRESS_MSC_WARNING(4275) # ifdef FMT_EXPORT @@ -1490,6 +1454,33 @@ struct named_arg : view, named_arg_base { : named_arg_base(name), value(val) {} }; +// scope_exit is proposed in TS v3. This implementation is slightly different +// because without deduction guide there's no easy way to construct from lambda. +// Factory template function is needed (see make_scope_exit). +// Move assignment is disabled because of unclear semantic: whether to call +// currently assigned functor or not... +template class scope_exit { + bool passive_{false}; + F func_; + + public: + explicit scope_exit(F&& f) noexcept : func_(std::forward(f)) {} + ~scope_exit() { + if (!passive_) func_(); + } + scope_exit(const F&) = delete; + scope_exit(scope_exit&& rhs) noexcept : func_{std::move(rhs.func_)} { + rhs.release(); + } + const scope_exit& operator=(const scope_exit&) = delete; + const scope_exit& operator=(scope_exit&&) = delete; + void release() noexcept { passive_ = true; } +}; + +template scope_exit make_scope_exit(F&& f) { + return scope_exit{std::forward(f)}; +} + } // namespace internal /** @@ -1581,14 +1572,11 @@ class dynamic_format_arg_store } data_.emplace_back(internal::make_arg( static_cast&>(arg.value))); - FMT_TRY { - named_info_.push_back({arg.name, static_cast(data_.size() - 2u)}); - data_[0].value_.named_args = {named_info_.data(), named_info_.size()}; - } - FMT_CATCH(...) { - data_.pop_back(); - FMT_RETHROW(); - } + + auto guard{internal::make_scope_exit([this]() { this->data_.pop_back(); })}; + named_info_.push_back({arg.name, static_cast(data_.size() - 2u)}); + data_[0].value_.named_args = {named_info_.data(), named_info_.size()}; + guard.release(); } public: @@ -1622,7 +1610,8 @@ class dynamic_format_arg_store /** \rst Adds a reference to the argument into the dynamic store for later passing to - a formating function. Supports wrapped named arguments. + a formating function. Supports named arguments wrapped in + std::reference_wrapper (via std::ref()/std::cref()). **Example**:: fmt::dynamic_format_arg_store store; diff --git a/include/fmt/format.h b/include/fmt/format.h index fd891ca80460..a087ca31c594 100644 --- a/include/fmt/format.h +++ b/include/fmt/format.h @@ -88,6 +88,40 @@ # define FMT_FALLTHROUGH #endif +#ifndef FMT_THROW +# if FMT_EXCEPTIONS +# if FMT_MSC_VER || FMT_NVCC +FMT_BEGIN_NAMESPACE +namespace internal { +template inline void do_throw(const Exception& x) { + // Silence unreachable code warnings in MSVC and NVCC because these + // are nearly impossible to fix in a generic code. + volatile bool b = true; + if (b) throw x; +} +} // namespace internal +FMT_END_NAMESPACE +# define FMT_THROW(x) internal::do_throw(x) +# else +# define FMT_THROW(x) throw x +# endif +# else +# define FMT_THROW(x) \ + do { \ + static_cast(sizeof(x)); \ + FMT_ASSERT(false, ""); \ + } while (false) +# endif +#endif + +#if FMT_EXCEPTIONS +# define FMT_TRY try +# define FMT_CATCH(x) catch (x) +#else +# define FMT_TRY if (true) +# define FMT_CATCH(x) if (false) +#endif + #ifndef FMT_USE_USER_DEFINED_LITERALS // For Intel and NVIDIA compilers both they and the system gcc/msc support UDLs. # if (FMT_HAS_FEATURE(cxx_user_literals) || FMT_GCC_VERSION >= 407 || \ diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 23ea9fcfb95e..89176633115c 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -95,7 +95,6 @@ add_fmt_test(grisu-test) target_compile_definitions(grisu-test PRIVATE FMT_USE_GRISU=1) add_fmt_test(gtest-extra-test) add_fmt_test(format-test mock-allocator.h) -add_fmt_test(format-dyn-args-test) if (MSVC) target_compile_options(format-test PRIVATE /bigobj) endif () From d2a2c525e6c68e8ec070b240af6f155ab1b0e3e3 Mon Sep 17 00:00:00 2001 From: vsol Date: Thu, 7 May 2020 12:33:05 +0300 Subject: [PATCH 23/25] Fix gcc 4.8 build --- include/fmt/core.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index 11d65d9be887..f2613d91562f 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -1573,7 +1573,8 @@ class dynamic_format_arg_store data_.emplace_back(internal::make_arg( static_cast&>(arg.value))); - auto guard{internal::make_scope_exit([this]() { this->data_.pop_back(); })}; + auto guard = + internal::make_scope_exit([this]() { this->data_.pop_back(); }); named_info_.push_back({arg.name, static_cast(data_.size() - 2u)}); data_[0].value_.named_args = {named_info_.data(), named_info_.size()}; guard.release(); From 5f6f93b7e511811e7d52159583a4a874b3dcd6b4 Mon Sep 17 00:00:00 2001 From: vsol Date: Thu, 7 May 2020 21:46:52 +0300 Subject: [PATCH 24/25] Minor fixes. --- include/fmt/core.h | 43 ++++++++----------------------------------- 1 file changed, 8 insertions(+), 35 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index f2613d91562f..ef1534b72d5b 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -1454,33 +1454,6 @@ struct named_arg : view, named_arg_base { : named_arg_base(name), value(val) {} }; -// scope_exit is proposed in TS v3. This implementation is slightly different -// because without deduction guide there's no easy way to construct from lambda. -// Factory template function is needed (see make_scope_exit). -// Move assignment is disabled because of unclear semantic: whether to call -// currently assigned functor or not... -template class scope_exit { - bool passive_{false}; - F func_; - - public: - explicit scope_exit(F&& f) noexcept : func_(std::forward(f)) {} - ~scope_exit() { - if (!passive_) func_(); - } - scope_exit(const F&) = delete; - scope_exit(scope_exit&& rhs) noexcept : func_{std::move(rhs.func_)} { - rhs.release(); - } - const scope_exit& operator=(const scope_exit&) = delete; - const scope_exit& operator=(scope_exit&&) = delete; - void release() noexcept { passive_ = true; } -}; - -template scope_exit make_scope_exit(F&& f) { - return scope_exit{std::forward(f)}; -} - } // namespace internal /** @@ -1565,16 +1538,16 @@ class dynamic_format_arg_store template void emplace_arg(const internal::named_arg& arg) { if (named_info_.empty()) { - data_.insert( - data_.begin(), - {static_cast*>(nullptr), - 0}); + constexpr const internal::named_arg_info* zero_ptr{nullptr}; + data_.insert(data_.begin(), {zero_ptr, 0}); } data_.emplace_back(internal::make_arg( static_cast&>(arg.value))); - - auto guard = - internal::make_scope_exit([this]() { this->data_.pop_back(); }); + auto pop_one = [](std::vector>* data) { + data->pop_back(); + }; + std::unique_ptr>, decltype(pop_one)> + guard{&data_, pop_one}; named_info_.push_back({arg.name, static_cast(data_.size() - 2u)}); data_[0].value_.named_args = {named_info_.data(), named_info_.size()}; guard.release(); @@ -1631,7 +1604,7 @@ class dynamic_format_arg_store */ template void push_back(std::reference_wrapper arg) { static_assert( - internal::is_named_arg::type>::value || + internal::is_named_arg::type>::value || need_copy::value, "objects of built-in types and string views are always copied"); emplace_arg(arg.get()); From 00b68834a65301d3e02e0bee3c03db93d854b1ed Mon Sep 17 00:00:00 2001 From: vsol Date: Sat, 9 May 2020 13:13:55 +0300 Subject: [PATCH 25/25] Review fixes. --- include/fmt/core.h | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index ef1534b72d5b..edab4ac8e02c 100644 --- a/include/fmt/core.h +++ b/include/fmt/core.h @@ -1271,16 +1271,13 @@ inline basic_format_arg make_arg(const T& value) { } template struct is_reference_wrapper : std::false_type {}; - template struct is_reference_wrapper> : std::true_type {}; -template struct unwrap_reference { using type = T; }; -template struct unwrap_reference> { - using type = T&; -}; -template -using unwrap_reference_t = typename unwrap_reference::type; +template const T& unwrap(const T& v) { return v; } +template const T& unwrap(const std::reference_wrapper& v) { + return static_cast(v); +} class dynamic_arg_list { // Workaround for clang's -Wweak-vtables. Unlike for regular classes, for @@ -1541,8 +1538,8 @@ class dynamic_format_arg_store constexpr const internal::named_arg_info* zero_ptr{nullptr}; data_.insert(data_.begin(), {zero_ptr, 0}); } - data_.emplace_back(internal::make_arg( - static_cast&>(arg.value))); + data_.emplace_back( + internal::make_arg(internal::unwrap(arg.value))); auto pop_one = [](std::vector>* data) { data->pop_back(); }; @@ -1578,7 +1575,7 @@ class dynamic_format_arg_store if (internal::const_check(need_copy::value)) emplace_arg(dynamic_args_.push>(arg)); else - emplace_arg(static_cast&>(arg)); + emplace_arg(internal::unwrap(arg)); } /** @@ -1622,8 +1619,9 @@ class dynamic_format_arg_store if (internal::const_check(need_copy::value)) { emplace_arg( fmt::arg(arg_name, dynamic_args_.push>(arg.value))); - } else + } else { emplace_arg(fmt::arg(arg_name, arg.value)); + } } };