From dd97f4920caf7e03aff26f40048408de038963cf Mon Sep 17 00:00:00 2001
From: Victor Zverovich <viz@fb.com>
Date: Sat, 21 Mar 2020 08:51:48 -0700
Subject: [PATCH] Improve exception safety in dynamic_format_arg_store

---
 include/fmt/core.h | 12 ++++++------
 test/core-test.cc  | 29 +++++++++++++++++++++++++++--
 2 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/include/fmt/core.h b/include/fmt/core.h
index a9116314..d1242b59 100644
--- a/include/fmt/core.h
+++ b/include/fmt/core.h
@@ -284,7 +284,7 @@ FMT_NORETURN FMT_API void assert_fail(const char* file, int line,
 
 #ifndef FMT_ASSERT
 #  ifdef NDEBUG
-     // FMT_ASSERT is not empty to avoid -Werror=empty-body.
+// FMT_ASSERT is not empty to avoid -Werror=empty-body.
 #    define FMT_ASSERT(condition, message) ((void)0)
 #  else
 #    define FMT_ASSERT(condition, message)                                    \
@@ -1236,11 +1236,11 @@ class dynamic_arg_list {
 
  public:
   template <typename T, typename Arg> const T& push(const Arg& arg) {
-    auto next = std::move(head_);
-    auto node = new typed_node<T>(arg);
-    head_.reset(node);
-    head_->next = std::move(next);
-    return node->value;
+    auto node = std::unique_ptr<typed_node<T>>(new typed_node<T>(arg));
+    auto& value = node->value;
+    node->next = std::move(head_);
+    head_ = std::move(node);
+    return value;
   }
 };
 }  // namespace internal
diff --git a/test/core-test.cc b/test/core-test.cc
index c2e4593a..f19d0423 100644
--- a/test/core-test.cc
+++ b/test/core-test.cc
@@ -435,8 +435,7 @@ template <> struct formatter<custom_type> {
   }
 
   template <typename FormatContext>
-  auto format(const custom_type& p, FormatContext& ctx) -> decltype(format_to(
-      ctx.out(), std::declval<typename FormatContext::char_type const*>())) {
+  auto format(const custom_type& p, FormatContext& ctx) -> decltype(ctx.out()) {
     return format_to(ctx.out(), "cust={}", p.i);
   }
 };
@@ -478,6 +477,32 @@ TEST(FormatDynArgsTest, NamedArgByRef) {
   EXPECT_EQ("42", result);
 }
 
+struct copy_throwable {
+  copy_throwable() {}
+  copy_throwable(const copy_throwable&) { throw "deal with it"; }
+};
+
+FMT_BEGIN_NAMESPACE
+template <> struct formatter<copy_throwable> {
+  auto parse(format_parse_context& ctx) const -> decltype(ctx.begin()) {
+    return ctx.begin();
+  }
+  auto format(copy_throwable, format_context& ctx) -> decltype(ctx.out()) {
+    return ctx.out();
+  }
+};
+FMT_END_NAMESPACE
+
+TEST(FormatDynArgsTest, ThrowOnCopy) {
+  fmt::dynamic_format_arg_store<fmt::format_context> store;
+  store.push_back(std::string("foo"));
+  try {
+    store.push_back(copy_throwable());
+  } catch (...) {
+  }
+  EXPECT_EQ(fmt::vformat("{}", store), "foo");
+}
+
 TEST(StringViewTest, ValueType) {
   static_assert(std::is_same<string_view::value_type, char>::value, "");
 }