Support named args in dynamic_format_arg_store (#1655).

First cut.

include/fmt/core.h

@@ -629,6 +629,7 @@ using wparse_context FMT_DEPRECATED_ALIAS = basic_format_parse_context<wchar_t>;
 
 template <typename Context> class basic_format_arg;
 template <typename Context> class basic_format_args;
+template <typename Context> class dynamic_format_arg_store;
 
 // A formatter for objects of type T.
 template <typename T, typename Char = char, typename Enable = void>
@@ -1131,6 +1132,7 @@ template <typename Context> class basic_format_arg {
 
   friend class basic_format_args<Context>;
   friend class internal::arg_map<Context>;
+  friend class dynamic_format_arg_store<Context>;
 
   using char_type = typename Context::char_type;
 
@@ -1419,6 +1421,50 @@ inline format_arg_store<Context, Args...> make_format_args(
   return {args...};
 }
 
+namespace internal {
+template <typename Char> struct named_arg_base {
+  const Char* name;
+
+  // Serialized value<context>.
+  mutable char data[sizeof(basic_format_arg<buffer_context<Char>>)];
+
+  named_arg_base(const Char* nm) : name(nm) {}
+
+  template <typename Context> basic_format_arg<Context> deserialize() const {
+    basic_format_arg<Context> arg;
+    std::memcpy(&arg, data, sizeof(basic_format_arg<Context>));
+    return arg;
+  }
+};
+
+struct view {};
+
+template <typename T, typename Char>
+struct named_arg : view, named_arg_base<Char> {
+  const T& value;
+
+  named_arg(const Char* name, const T& val)
+      : named_arg_base<Char>(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 <typename Char, typename T>
+inline internal::named_arg<T, Char> arg(const Char* name, const T& arg) {
+  static_assert(!internal::is_named_arg<T>(), "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<basic_format_arg<Context>> data_;
+  std::vector<internal::named_arg_info<char_type>> 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<Context>;
 
   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<Context>* data() const {
+    return named_info_.empty() ? data_.data() : data_.data() + 1;
   }
 
   template <typename T> void emplace_arg(const T& arg) {
     data_.emplace_back(internal::make_arg<Context>(arg));
   }
 
+  template <typename T>
+  void emplace_arg(const internal::named_arg<T, char_type>& arg) {
+    if (named_info_.empty())
+      data_.insert(data_.begin(), basic_format_arg<Context>{});
+    data_.emplace_back(internal::make_arg<Context>(arg));
+    try {
+      named_info_.push_back({arg.name, static_cast<int>(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 <typename T>
+  void push_back(const internal::named_arg<T, char_type>& arg) {
+    const char_type* arg_name =
+        dynamic_args_.push<std::basic_string<char_type>>(arg.name).c_str();
+    if (internal::const_check(need_copy<T>::value)) {
+      emplace_arg(
+          fmt::arg(arg_name, dynamic_args_.push<stored_type<T>>(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 <typename T>
+  void push_back(
+      std::reference_wrapper<const internal::named_arg<T, char_type>> arg) {
+    emplace_arg(arg.get());
+  }
 };
 
 /**
@@ -1597,7 +1685,7 @@ template <typename Context> class basic_format_args {
    \endrst
    */
   FMT_INLINE basic_format_args(const dynamic_format_arg_store<Context>& store)
-      : basic_format_args(store.get_types(), store.data_.data()) {}
+      : basic_format_args(store.get_types(), store.data()) {}
 
   /**
    \rst
@@ -1659,31 +1747,6 @@ template <typename Container>
 struct is_contiguous_back_insert_iterator<std::back_insert_iterator<Container>>
     : is_contiguous<Container> {};
 
-template <typename Char> struct named_arg_base {
-  const Char* name;
-
-  // Serialized value<context>.
-  mutable char data[sizeof(basic_format_arg<buffer_context<Char>>)];
-
-  named_arg_base(const Char* nm) : name(nm) {}
-
-  template <typename Context> basic_format_arg<Context> deserialize() const {
-    basic_format_arg<Context> arg;
-    std::memcpy(&arg, data, sizeof(basic_format_arg<Context>));
-    return arg;
-  }
-};
-
-struct view {};
-
-template <typename T, typename Char>
-struct named_arg : view, named_arg_base<Char> {
-  const T& value;
-
-  named_arg(const Char* name, const T& val)
-      : named_arg_base<Char>(name), value(val) {}
-};
-
 // Reports a compile-time error if S is not a valid format string.
 template <typename..., typename S, FMT_ENABLE_IF(!is_compile_string<S>::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 <typename Char, typename T>
-inline internal::named_arg<T, Char> arg(const Char* name, const T& arg) {
-  static_assert(!internal::is_named_arg<T>(), "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<Container> with
 // vformat_to<ArgFormatter>(...) overload, so SFINAE on iterator type instead.

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 ()

test/format-dyn-args-test.cc

@@ -4,3 +4,50 @@
 #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);
+}
+