From 6199e2a02d1f31161a62d5b80ff87252a95f7e17 Mon Sep 17 00:00:00 2001 From: vsol Date: Mon, 4 May 2020 14:40:18 +0300 Subject: [PATCH] Named arguments. Better wupport for std::reference_wrapper. --- include/fmt/core.h | 88 +++++++++++++++++++++++------------- test/core-test.cc | 60 ++++++++++++++++++++++++ test/format-dyn-args-test.cc | 47 ------------------- 3 files changed, 116 insertions(+), 79 deletions(-) diff --git a/include/fmt/core.h b/include/fmt/core.h index 9362642b..dad05be9 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,9 +1616,42 @@ class dynamic_format_arg_store if (internal::const_check(need_copy::value)) emplace_arg(dynamic_args_.push>(arg)); else - emplace_arg(arg); + emplace_arg(static_cast&>(arg)); } + /** + \rst + Adds a reference to the argument into the dynamic store for later passing to + 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( + 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 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(const internal::named_arg& arg) { const char_type* arg_name = @@ -1613,31 +1659,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)); } - - /** - 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) { - static_assert( - 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. - */ - template - void push_back( - std::reference_wrapper> arg) { - emplace_arg(arg.get()); - } }; /** diff --git a/test/core-test.cc b/test/core-test.cc index adda8943..8c2f7bd9 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 b2c4c5f2..acc5ef78 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); -} -