Named arguments. Better wupport for std::reference_wrapper.

include/fmt/core.h

@@ -1311,6 +1311,13 @@ template <typename T> struct is_reference_wrapper : std::false_type {};
 template <typename T>
 struct is_reference_wrapper<std::reference_wrapper<T>> : std::true_type {};
 
+template <typename T> struct unwrap_reference { using type = T; };
+template <typename T> struct unwrap_reference<std::reference_wrapper<T>> {
+  using type = T&;
+};
+template <typename T>
+using unwrap_reference_t = typename unwrap_reference<T>::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<T, internal::std_string_view<char_type>>::value ||
                 (mapped_type != internal::type::cstring_type &&
                  mapped_type != internal::type::string_type &&
-                 mapped_type != internal::type::custom_type &&
+                 mapped_type != internal::type::custom_type))
-                 mapped_type != internal::type::named_arg_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 :
+           (named_info_.empty() ? 0ULL
-               static_cast<unsigned long long>(internal::has_named_args_bit));
+                                : static_cast<unsigned long long>(
+                                      internal::has_named_args_bit));
   }
 
   const basic_format_arg<Context>* data() const {
@@ -1566,13 +1573,19 @@ class dynamic_format_arg_store
 
   template <typename T>
   void emplace_arg(const internal::named_arg<T, char_type>& arg) {
-    if (named_info_.empty())
+    if (named_info_.empty()) {
-      data_.insert(data_.begin(), basic_format_arg<Context>{});
+      data_.insert(
-    data_.emplace_back(internal::make_arg<Context>(arg));
+          data_.begin(),
+          {static_cast<const internal::named_arg_info<char_type>*>(nullptr),
+           0});
+    }
+    data_.emplace_back(internal::make_arg<Context>(
+        static_cast<const internal::unwrap_reference_t<T>&>(arg.value)));
     FMT_TRY {
       named_info_.push_back({arg.name, static_cast<int>(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<T>::value))
       emplace_arg(dynamic_args_.push<stored_type<T>>(arg));
     else
-      emplace_arg(arg);
+      emplace_arg(static_cast<const internal::unwrap_reference_t<T>&>(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<fmt::format_context> 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 <typename T> void push_back(std::reference_wrapper<T> arg) {
+    static_assert(
+        internal::is_named_arg<typename std::decay<T>::type>::value ||
+            need_copy<T>::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 <typename T>
   void push_back(const internal::named_arg<T, char_type>& arg) {
     const char_type* arg_name =
@@ -1613,31 +1659,9 @@ class dynamic_format_arg_store
     if (internal::const_check(need_copy<T>::value)) {
       emplace_arg(
           fmt::arg(arg_name, dynamic_args_.push<stored_type<T>>(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 <typename T> void push_back(std::reference_wrapper<T> arg) {
-    static_assert(
-        need_copy<T>::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 <typename T>
-  void push_back(
-      std::reference_wrapper<const internal::named_arg<T, char_type>> arg) {
-    emplace_arg(arg.get());
-  }
 };
 
 /**

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<fmt::format_context> 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<fmt::format_context> 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<fmt::format_context> 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<fmt::format_context> 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"; }

test/format-dyn-args-test.cc

@@ -4,50 +4,3 @@
 #include <fmt/core.h>
 
 #include "gtest-extra.h"
-
-TEST(FormatDynArgsTest, NamedInt) {
-  fmt::dynamic_format_arg_store<fmt::format_context> 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<fmt::format_context> 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<fmt::format_context> 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);
-}
-