Fix handling of types with to_string_view and formatter specialization (

#2180)
fmtlib · Mar 18, 2021 · 83e417d · 83e417d
1 parent a6408a3
commit 83e417d
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 19 deletions.
diff --git a/include/fmt/args.h b/include/fmt/args.h
@@ -95,7 +95,9 @@ class dynamic_format_arg_store
   };
 
   template <typename T>
-  using stored_type = conditional_t<detail::is_string<T>::value,
+  using stored_type = conditional_t<detail::is_string<T>::value &&
+                                        !has_formatter<T, Context>::value &&
+                                        !detail::is_reference_wrapper<T>::value,
                                     std::basic_string<char_type>, T>;
 
   // Storage of basic_format_arg must be contiguous.

diff --git a/test/args-test.cc b/test/args-test.cc
@@ -10,7 +10,7 @@
 #include "gmock.h"
 
 TEST(ArgsTest, Basic) {
-  fmt::dynamic_format_arg_store<fmt::format_context> store;
+  auto store = fmt::dynamic_format_arg_store<fmt::format_context>();
   store.push_back(42);
   store.push_back("abc1");
   store.push_back(1.5f);
@@ -19,14 +19,14 @@ TEST(ArgsTest, Basic) {
 
 TEST(ArgsTest, StringsAndRefs) {
   // Unfortunately the tests are compiled with old ABI so strings use COW.
-  fmt::dynamic_format_arg_store<fmt::format_context> store;
+  auto store = fmt::dynamic_format_arg_store<fmt::format_context>();
   char str[] = "1234567890";
   store.push_back(str);
   store.push_back(std::cref(str));
   store.push_back(fmt::string_view{str});
   str[0] = 'X';
 
-  std::string result = fmt::vformat("{} and {} and {}", store);
+  auto result = fmt::vformat("{} and {} and {}", store);
   EXPECT_EQ("1234567890 and X234567890 and X234567890", result);
 }
 
@@ -48,59 +48,84 @@ template <> struct formatter<custom_type> {
 FMT_END_NAMESPACE
 
 TEST(ArgsTest, CustomFormat) {
-  fmt::dynamic_format_arg_store<fmt::format_context> store;
-  custom_type c{};
+  auto store = fmt::dynamic_format_arg_store<fmt::format_context>();
+  auto c = custom_type();
   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);
+  auto result = fmt::vformat("{} and {} and {}", store);
   EXPECT_EQ("cust=0 and cust=1 and cust=3", result);
 }
 
+struct to_stringable {
+  friend std::string_view to_string_view(const custom_type&) { return ""; }
+};
+
+FMT_BEGIN_NAMESPACE
+template <> struct formatter<to_stringable> {
+  auto parse(format_parse_context& ctx) const -> decltype(ctx.begin()) {
+    return ctx.begin();
+  }
+
+  template <typename FormatContext>
+  auto format(const to_stringable&, FormatContext& ctx) -> decltype(ctx.out()) {
+    return ctx.out();
+  }
+};
+FMT_END_NAMESPACE
+
+TEST(ArgsTest, ToStringAndFormatter) {
+  auto store = fmt::dynamic_format_arg_store<fmt::format_context>();
+  auto s = to_stringable();
+  store.push_back(s);
+  store.push_back(std::cref(s));
+  fmt::vformat("", store);
+}
+
 TEST(ArgsTest, NamedInt) {
-  fmt::dynamic_format_arg_store<fmt::format_context> store;
+  auto store = fmt::dynamic_format_arg_store<fmt::format_context>();
   store.push_back(fmt::arg("a1", 42));
   EXPECT_EQ("42", fmt::vformat("{a1}", store));
 }
 
 TEST(ArgsTest, NamedStrings) {
-  fmt::dynamic_format_arg_store<fmt::format_context> store;
-  char str[]{"1234567890"};
+  auto store = fmt::dynamic_format_arg_store<fmt::format_context>();
+  char str[] = "1234567890";
   store.push_back(fmt::arg("a1", str));
   store.push_back(fmt::arg("a2", std::cref(str)));
   str[0] = 'X';
   EXPECT_EQ("1234567890 and X234567890", fmt::vformat("{a1} and {a2}", store));
 }
 
 TEST(ArgsTest, NamedArgByRef) {
-  fmt::dynamic_format_arg_store<fmt::format_context> store;
+  auto store = fmt::dynamic_format_arg_store<fmt::format_context>();
   char band[] = "Rolling Stones";
   store.push_back(fmt::arg("band", std::cref(band)));
   band[9] = 'c';  // Changing band affects the output.
   EXPECT_EQ(fmt::vformat("{band}", store), "Rolling Scones");
 }
 
 TEST(ArgsTest, NamedCustomFormat) {
-  fmt::dynamic_format_arg_store<fmt::format_context> store;
-  custom_type c{};
+  auto store = fmt::dynamic_format_arg_store<fmt::format_context>();
+  auto c = custom_type();
   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);
+  auto result = fmt::vformat("{c1} and {c2} and {c_ref}", store);
   EXPECT_EQ("cust=0 and cust=1 and cust=3", result);
 }
 
 TEST(ArgsTest, Clear) {
-  fmt::dynamic_format_arg_store<fmt::format_context> store;
+  auto store = fmt::dynamic_format_arg_store<fmt::format_context>();
   store.push_back(42);
 
-  std::string result = fmt::vformat("{}", store);
+  auto result = fmt::vformat("{}", store);
   EXPECT_EQ("42", result);
 
   store.push_back(43);
@@ -114,11 +139,11 @@ TEST(ArgsTest, Clear) {
 }
 
 TEST(ArgsTest, Reserve) {
-  fmt::dynamic_format_arg_store<fmt::format_context> store;
+  auto store = fmt::dynamic_format_arg_store<fmt::format_context>();
   store.reserve(2, 1);
   store.push_back(1.5f);
   store.push_back(fmt::arg("a1", 42));
-  std::string result = fmt::vformat("{a1} and {}", store);
+  auto result = fmt::vformat("{a1} and {}", store);
   EXPECT_EQ("42 and 1.5", result);
 }
 
@@ -139,7 +164,7 @@ template <> struct formatter<copy_throwable> {
 FMT_END_NAMESPACE
 
 TEST(ArgsTest, ThrowOnCopy) {
-  fmt::dynamic_format_arg_store<fmt::format_context> store;
+  auto store = fmt::dynamic_format_arg_store<fmt::format_context>();
   store.push_back(std::string("foo"));
   try {
     store.push_back(copy_throwable());