Separate memory management and formatting

Array is split into an abstract Buffer class and a concrete MemoryBuffer class. BasicWriter now does all memory allocation through a Buffer object. Subclasses of BasicWriter may use different buffer types. The new BasicMemoryBuffer class uses the default MemoryBuffer.

CMakeLists.txt

@@ -24,6 +24,9 @@ else ()
     set(CPP11_FLAG -std=c++0x)
   endif ()
 endif ()
+if (CPP11_FLAG)
+  set(CMAKE_REQUIRED_FLAGS ${CPP11_FLAG})
+endif ()
 
 set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH}
   "${CMAKE_CURRENT_SOURCE_DIR}/cmake")

format.cc

@@ -154,7 +154,7 @@ void format_error_code(fmt::Writer &out, int error_code,
 
 void report_error(FormatFunc func,
     int error_code, fmt::StringRef message) FMT_NOEXCEPT(true) {
-  fmt::Writer full_message;
+  fmt::MemoryWriter full_message;
   func(full_message, error_code, message);
   // Use Writer::data instead of Writer::c_str to avoid potential memory
   // allocation.
@@ -321,7 +321,7 @@ inline Arg::StringValue<wchar_t> ignore_incompatible_str(
 void fmt::SystemError::init(
     int error_code, StringRef format_str, ArgList args) {
   error_code_ = error_code;
-  Writer w;
+  MemoryWriter w;
   internal::format_system_error(w, error_code, format(format_str, args));
   std::runtime_error &base = *this;
   base = std::runtime_error(w.str());
@@ -442,7 +442,7 @@ void fmt::internal::format_system_error(
     fmt::Writer &out, int error_code,
     fmt::StringRef message) FMT_NOEXCEPT(true) {
   try {
-    Array<char, INLINE_BUFFER_SIZE> buffer;
+    MemoryBuffer<char, INLINE_BUFFER_SIZE> buffer;
     buffer.resize(INLINE_BUFFER_SIZE);
     char *system_message = 0;
     for (;;) {
@@ -559,9 +559,9 @@ class fmt::internal::ArgFormatter :
   }
 };
 
-template <typename Char, typename Allocator>
+template <typename Char>
 template <typename StrChar>
-void fmt::BasicWriter<Char, Allocator>::write_str(
+void fmt::BasicWriter<Char>::write_str(
     const Arg::StringValue<StrChar> &str, const FormatSpec &spec) {
   // Check if StrChar is convertible to Char.
   internal::CharTraits<Char>::convert(StrChar());
@@ -1029,7 +1029,7 @@ void fmt::report_windows_error(
 #endif
 
 void fmt::print(std::FILE *f, StringRef format_str, ArgList args) {
-  Writer w;
+  MemoryWriter w;
   w.write(format_str, args);
   std::fwrite(w.data(), 1, w.size(), f);
 }
@@ -1039,7 +1039,7 @@ void fmt::print(StringRef format_str, ArgList args) {
 }
 
 void fmt::print(std::ostream &os, StringRef format_str, ArgList args) {
-  Writer w;
+  MemoryWriter w;
   w.write(format_str, args);
   os.write(w.data(), w.size());
 }
@@ -1053,7 +1053,7 @@ void fmt::print_colored(Color c, StringRef format, ArgList args) {
 }
 
 int fmt::fprintf(std::FILE *f, StringRef format, ArgList args) {
-  Writer w;
+  MemoryWriter w;
   printf(w, format, args);
   return std::fwrite(w.data(), 1, w.size(), f);
 }

test/CMakeLists.txt

@@ -27,12 +27,21 @@ foreach (target format-test printf-test)
     set_target_properties(${target} PROPERTIES COMPILE_FLAGS ${CPP11_FLAG})
   endif ()
 endforeach ()
-add_fmt_test(util-test)
+add_fmt_test(util-test mock-allocator.h)
+if (CPP11_FLAG)
+  set_target_properties(util-test PROPERTIES COMPILE_FLAGS ${CPP11_FLAG})
+endif ()
 
 foreach (src ${FMT_SOURCES})
   set(FMT_TEST_SOURCES ${FMT_TEST_SOURCES} ../${src})
 endforeach ()
 
+include(CheckIncludeFileCXX)
+check_include_file_cxx(type_traits HAVE_TYPE_TRAITS)
+if (HAVE_TYPE_TRAITS)
+  add_definitions(-DFMT_USE_TYPE_TRAITS=1)
+endif ()
+
 add_executable(macro-test macro-test.cc ${FMT_TEST_SOURCES} ${TEST_MAIN_SRC})
 set_target_properties(macro-test
   PROPERTIES COMPILE_DEFINITIONS "FMT_USE_VARIADIC_TEMPLATES=0")

test/format-impl-test.cc

@@ -88,20 +88,20 @@ TEST(FormatTest, StrError) {
 TEST(FormatTest, FormatErrorCode) {
   std::string msg = "error 42", sep = ": ";
   {
-    fmt::Writer w;
+    fmt::MemoryWriter w;
     w << "garbage";
     format_error_code(w, 42, "test");
     EXPECT_EQ("test: " + msg, w.str());
   }
   {
-    fmt::Writer w;
+    fmt::MemoryWriter w;
     std::string prefix(
         fmt::internal::INLINE_BUFFER_SIZE - msg.size() - sep.size() + 1, 'x');
     format_error_code(w, 42, prefix);
     EXPECT_EQ(msg, w.str());
   }
   {
-    fmt::Writer w;
+    fmt::MemoryWriter w;
     std::string prefix(
         fmt::internal::INLINE_BUFFER_SIZE - msg.size() - sep.size(), 'x');
     format_error_code(w, 42, prefix);

test/gtest-extra-test.cc

@@ -348,7 +348,7 @@ TEST(StreamingAssertionsTest, EXPECT_WRITE) {
 }
 
 TEST(UtilTest, FormatSystemError) {
-  fmt::Writer out;
+  fmt::MemoryWriter out;
   fmt::internal::format_system_error(out, EDOM, "test message");
   EXPECT_EQ(out.str(), format_system_error(EDOM, "test message"));
 }

test/gtest-extra.cc

@@ -92,7 +92,7 @@ std::string OutputRedirect::restore_and_read() {
 #endif  // FMT_USE_FILE_DESCRIPTORS
 
 std::string format_system_error(int error_code, fmt::StringRef message) {
-  fmt::Writer out;
+  fmt::MemoryWriter out;
   fmt::internal::format_system_error(out, error_code, message);
   return out.str();
 }

test/gtest-extra.h

@@ -25,8 +25,8 @@
  SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef FMT_GTEST_EXTRA_H
-#define FMT_GTEST_EXTRA_H
+#ifndef FMT_GTEST_EXTRA_H_
+#define FMT_GTEST_EXTRA_H_
 
 #include <string>
 #include <gtest/gtest.h>
@@ -135,4 +135,4 @@ class OutputRedirect {
 
 #endif  // FMT_USE_FILE_DESCRIPTORS
 
-#endif  // FMT_GTEST_EXTRA_H
+#endif  // FMT_GTEST_EXTRA_H_

test/mock-allocator.h

+/*
+ Mock allocator.
+
+ Copyright (c) 2014, Victor Zverovich
+ All rights reserved.
+
+ Redistribution and use in source and binary forms, with or without
+ modification, are permitted provided that the following conditions are met:
+
+ 1. Redistributions of source code must retain the above copyright notice, this
+    list of conditions and the following disclaimer.
+ 2. Redistributions in binary form must reproduce the above copyright notice,
+    this list of conditions and the following disclaimer in the documentation
+    and/or other materials provided with the distribution.
+
+ THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+ ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
+ ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef FMT_MOCK_ALLOCATOR_H_
+#define FMT_MOCK_ALLOCATOR_H_
+
+#include "gmock/gmock.h"
+
+template <typename T>
+class MockAllocator {
+ public:
+  typedef T value_type;
+  MOCK_METHOD1_T(allocate, T* (std::size_t n));
+  MOCK_METHOD2_T(deallocate, void (T* p, std::size_t n));
+};
+
+template <typename Allocator>
+class AllocatorRef {
+ private:
+  Allocator *alloc_;
+
+ public:
+  typedef typename Allocator::value_type value_type;
+
+  explicit AllocatorRef(Allocator *alloc = 0) : alloc_(alloc) {}
+
+  AllocatorRef(const AllocatorRef &other) : alloc_(other.alloc_) {}
+
+  AllocatorRef& operator=(const AllocatorRef &other) {
+    alloc_ = other.alloc_;
+    return *this;
+  }
+
+#if FMT_USE_RVALUE_REFERENCES
+ private:
+  void move(AllocatorRef &other) {
+    alloc_ = other.alloc_;
+    other.alloc_ = 0;
+  }
+
+ public:
+  AllocatorRef(AllocatorRef &&other) {
+    move(other);
+  }
+
+  AllocatorRef& operator=(AllocatorRef &&other) {
+    assert(this != &other);
+    move(other);
+    return *this;
+  }
+#endif
+
+  Allocator *get() const { return alloc_; }
+
+  value_type* allocate(std::size_t n) { return alloc_->allocate(n); }
+  void deallocate(value_type* p, std::size_t n) { alloc_->deallocate(p, n); }
+};
+
+#endif  // FMT_MOCK_ALLOCATOR_H_

test/util-test.cc

@@ -29,7 +29,14 @@
 #include <climits>
 #include <cstring>
 #include <limits>
+
+#if FMT_USE_TYPE_TRAITS
+# include <type_traits>
+#endif
+
+#include "gmock/gmock.h"
 #include "gtest-extra.h"
+#include "mock-allocator.h"
 #include "util.h"
 
 // Check if format.h compiles with windows.h included.
@@ -42,8 +49,11 @@
 #undef max
 
 using fmt::StringRef;
-using fmt::internal::Value;
 using fmt::internal::Arg;
+using fmt::internal::Value;
+using fmt::internal::MemoryBuffer;
+
+using testing::Return;
 
 namespace {
 
@@ -62,9 +72,241 @@ Arg make_arg(const T &value) {
         fmt::internal::MakeValue<Char>::type(value));
   return arg;
 }
-
 }  // namespace
 
+void CheckForwarding(
+    MockAllocator<int> &alloc, AllocatorRef< MockAllocator<int> > &ref) {
+  int mem;
+  // Check if value_type is properly defined.
+  AllocatorRef< MockAllocator<int> >::value_type *ptr = &mem;
+  // Check forwarding.
+  EXPECT_CALL(alloc, allocate(42)).WillOnce(Return(ptr));
+  ref.allocate(42);
+  EXPECT_CALL(alloc, deallocate(ptr, 42));
+  ref.deallocate(ptr, 42);
+}
+
+TEST(AllocatorTest, AllocatorRef) {
+  testing::StrictMock< MockAllocator<int> > alloc;
+  typedef AllocatorRef< MockAllocator<int> > TestAllocatorRef;
+  TestAllocatorRef ref(&alloc);
+  // Check if AllocatorRef forwards to the underlying allocator.
+  CheckForwarding(alloc, ref);
+  TestAllocatorRef ref2(ref);
+  CheckForwarding(alloc, ref2);
+  TestAllocatorRef ref3;
+  EXPECT_EQ(0, ref3.get());
+  ref3 = ref;
+  CheckForwarding(alloc, ref3);
+}
+
+#if FMT_USE_TYPE_TRAITS
+TEST(BufferTest, NotCopyConstructible) {
+  EXPECT_FALSE(std::is_copy_constructible<fmt::internal::Buffer<char> >::value);
+}
+
+TEST(BufferTest, NotCopyAssignable) {
+  EXPECT_FALSE(std::is_copy_assignable<fmt::internal::Buffer<char> >::value);
+}
+#endif
+
+TEST(MemoryBufferTest, Ctor) {
+  MemoryBuffer<char, 123> buffer;
+  EXPECT_EQ(0u, buffer.size());
+  EXPECT_EQ(123u, buffer.capacity());
+}
+
+#if FMT_USE_RVALUE_REFERENCES
+
+typedef AllocatorRef< std::allocator<char> > TestAllocator;
+
+void check_move_buffer(const char *str,
+                       MemoryBuffer<char, 5, TestAllocator> &buffer) {
+  std::allocator<char> *alloc = buffer.get_allocator().get();
+  MemoryBuffer<char, 5, TestAllocator> buffer2(std::move(buffer));
+  // Move shouldn't destroy the inline content of the first buffer.
+  EXPECT_EQ(str, std::string(&buffer[0], buffer.size()));
+  EXPECT_EQ(str, std::string(&buffer2[0], buffer2.size()));
+  EXPECT_EQ(5, buffer2.capacity());
+  // Move should transfer allocator.
+  EXPECT_EQ(0, buffer.get_allocator().get());
+  EXPECT_EQ(alloc, buffer2.get_allocator().get());
+}
+
+TEST(MemoryBufferTest, MoveCtor) {
+  std::allocator<char> alloc;
+  MemoryBuffer<char, 5, TestAllocator> buffer((TestAllocator(&alloc)));
+  const char test[] = "test";
+  buffer.append(test, test + 4);
+  check_move_buffer("test", buffer);
+  // Adding one more character fills the inline buffer, but doesn't cause
+  // dynamic allocation.
+  buffer.push_back('a');
+  check_move_buffer("testa", buffer);
+  const char *inline_buffer_ptr = &buffer[0];
+  // Adding one more character causes the content to move from the inline to
+  // a dynamically allocated buffer.
+  buffer.push_back('b');
+  MemoryBuffer<char, 5, TestAllocator> buffer2(std::move(buffer));
+  // Move should rip the guts of the first buffer.
+  EXPECT_EQ(inline_buffer_ptr, &buffer[0]);
+  EXPECT_EQ("testab", std::string(&buffer2[0], buffer2.size()));
+  EXPECT_GT(buffer2.capacity(), 5u);
+}
+
+void check_move_assign_buffer(const char *str, MemoryBuffer<char, 5> &buffer) {
+  MemoryBuffer<char, 5> buffer2;
+  buffer2 = std::move(buffer);
+  // Move shouldn't destroy the inline content of the first buffer.
+  EXPECT_EQ(str, std::string(&buffer[0], buffer.size()));
+  EXPECT_EQ(str, std::string(&buffer2[0], buffer2.size()));
+  EXPECT_EQ(5, buffer2.capacity());
+}
+
+TEST(MemoryBufferTest, MoveAssignment) {
+  MemoryBuffer<char, 5> buffer;
+  const char test[] = "test";
+  buffer.append(test, test + 4);
+  check_move_assign_buffer("test", buffer);
+  // Adding one more character fills the inline buffer, but doesn't cause
+  // dynamic allocation.
+  buffer.push_back('a');
+  check_move_assign_buffer("testa", buffer);
+  const char *inline_buffer_ptr = &buffer[0];
+  // Adding one more character causes the content to move from the inline to
+  // a dynamically allocated buffer.
+  buffer.push_back('b');
+  MemoryBuffer<char, 5> buffer2;
+  buffer2 = std::move(buffer);
+  // Move should rip the guts of the first buffer.
+  EXPECT_EQ(inline_buffer_ptr, &buffer[0]);
+  EXPECT_EQ("testab", std::string(&buffer2[0], buffer2.size()));
+  EXPECT_GT(buffer2.capacity(), 5u);
+}
+
+#endif  // FMT_USE_RVALUE_REFERENCES
+
+TEST(MemoryBufferTest, Access) {
+  MemoryBuffer<char, 10> buffer;
+  buffer[0] = 11;
+  EXPECT_EQ(11, buffer[0]);
+  buffer[3] = 42;
+  EXPECT_EQ(42, *(&buffer[0] + 3));
+  const MemoryBuffer<char, 10> &const_buffer = buffer;
+  EXPECT_EQ(42, const_buffer[3]);
+}
+
+TEST(MemoryBufferTest, Resize) {
+  MemoryBuffer<char, 123> buffer;
+  buffer[10] = 42;
+  EXPECT_EQ(42, buffer[10]);
+  buffer.resize(20);
+  EXPECT_EQ(20u, buffer.size());
+  EXPECT_EQ(123u, buffer.capacity());
+  EXPECT_EQ(42, buffer[10]);
+  buffer.resize(5);
+  EXPECT_EQ(5u, buffer.size());
+  EXPECT_EQ(123u, buffer.capacity());
+  EXPECT_EQ(42, buffer[10]);
+}
+
+TEST(MemoryBufferTest, Grow) {
+  MemoryBuffer<int, 10> buffer;
+  buffer.resize(10);
+  for (int i = 0; i < 10; ++i)
+    buffer[i] = i * i;
+  buffer.resize(20);
+  EXPECT_EQ(20u, buffer.size());
+  EXPECT_EQ(20u, buffer.capacity());
+  for (int i = 0; i < 10; ++i)
+    EXPECT_EQ(i * i, buffer[i]);
+}
+
+TEST(MemoryBufferTest, Clear) {
+  MemoryBuffer<char, 10> buffer;
+  buffer.resize(20);
+  buffer.clear();
+  EXPECT_EQ(0u, buffer.size());
+  EXPECT_EQ(20u, buffer.capacity());
+}
+
+TEST(MemoryBufferTest, PushBack) {
+  MemoryBuffer<int, 10> buffer;
+  buffer.push_back(11);
+  EXPECT_EQ(11, buffer[0]);
+  EXPECT_EQ(1u, buffer.size());
+  buffer.resize(10);
+  buffer.push_back(22);
+  EXPECT_EQ(22, buffer[10]);
+  EXPECT_EQ(11u, buffer.size());
+  EXPECT_EQ(15u, buffer.capacity());
+}
+
+TEST(MemoryBufferTest, Append) {
+  MemoryBuffer<char, 10> buffer;
+  const char *test = "test";
+  buffer.append(test, test + 5);
+  EXPECT_STREQ(test, &buffer[0]);
+  EXPECT_EQ(5u, buffer.size());
+  buffer.resize(10);
+  buffer.append(test, test + 2);
+  EXPECT_EQ('t', buffer[10]);
+  EXPECT_EQ('e', buffer[11]);
+  EXPECT_EQ(12u, buffer.size());
+  EXPECT_EQ(15u, buffer.capacity());
+}
+
+TEST(MemoryBufferTest, AppendAllocatesEnoughStorage) {
+  MemoryBuffer<char, 10> buffer;
+  const char *test = "abcdefgh";
+  buffer.resize(10);
+  buffer.append(test, test + 9);
+  EXPECT_STREQ(test, &buffer[10]);
+  EXPECT_EQ(19u, buffer.size());
+  EXPECT_EQ(19u, buffer.capacity());
+}
+
+TEST(MemoryBufferTest, Allocator) {
+  typedef AllocatorRef< MockAllocator<char> > TestAllocator;
+  MemoryBuffer<char, 10, TestAllocator> buffer;
+  EXPECT_EQ(0, buffer.get_allocator().get());
+  testing::StrictMock< MockAllocator<char> > alloc;
+  char mem;
+  {
+    MemoryBuffer<char, 10, TestAllocator> buffer2((TestAllocator(&alloc)));
+    EXPECT_EQ(&alloc, buffer2.get_allocator().get());
+    std::size_t size = 2 * fmt::internal::INLINE_BUFFER_SIZE;
+    EXPECT_CALL(alloc, allocate(size)).WillOnce(Return(&mem));
+    buffer2.reserve(size);
+    EXPECT_CALL(alloc, deallocate(&mem, size));
+  }
+}
+
+TEST(MemoryBufferTest, ExceptionInDeallocate) {
+  typedef AllocatorRef< MockAllocator<char> > TestAllocator;
+  testing::StrictMock< MockAllocator<char> > alloc;
+  MemoryBuffer<char, 10, TestAllocator> buffer((TestAllocator(&alloc)));
+  std::size_t size = 2 * fmt::internal::INLINE_BUFFER_SIZE;
+  std::vector<char> mem(size);
+  {
+    EXPECT_CALL(alloc, allocate(size)).WillOnce(Return(&mem[0]));
+    buffer.resize(size);
+    std::fill(&buffer[0], &buffer[0] + size, 'x');
+  }
+  std::vector<char> mem2(2 * size);
+  {
+    EXPECT_CALL(alloc, allocate(2 * size)).WillOnce(Return(&mem2[0]));
+    std::exception e;
+    EXPECT_CALL(alloc, deallocate(&mem[0], size)).WillOnce(testing::Throw(e));
+    EXPECT_THROW(buffer.reserve(2 * size), std::exception);
+    EXPECT_EQ(&mem2[0], &buffer[0]);
+    // Check that the data has been copied.
+    for (std::size_t i = 0; i < size; ++i)
+      EXPECT_EQ('x', buffer[i]);
+  }
+  EXPECT_CALL(alloc, deallocate(&mem2[0], 2 * size));
+}
+
 TEST(UtilTest, Increment) {
   char s[10] = "123";
   increment(s);
@@ -236,7 +478,7 @@ TEST(ArgTest, MakeArg) {
   Arg arg = make_arg<char>(t);
   EXPECT_EQ(fmt::internal::Arg::CUSTOM, arg.type);
   EXPECT_EQ(&t, arg.custom.value);
-  fmt::Writer w;
+  fmt::MemoryWriter w;
   fmt::BasicFormatter<char> formatter(w);
   const char *s = "}";
   arg.custom.format(&formatter, &t, &s);
@@ -435,14 +677,14 @@ void check_throw_error(int error_code, FormatErrorMessage format) {
   } catch (const fmt::SystemError &e) {
     error = e;
   }
-  fmt::Writer message;
+  fmt::MemoryWriter message;
   format(message, error_code, "test error");
   EXPECT_EQ(message.str(), error.what());
   EXPECT_EQ(error_code, error.error_code());
 }
 
 TEST(UtilTest, FormatSystemError) {
-  fmt::Writer message;
+  fmt::MemoryWriter message;
   fmt::internal::format_system_error(message, EDOM, "test");
   EXPECT_EQ(fmt::format("test: {}", get_system_error(EDOM)), message.str());
   message.clear();
@@ -459,7 +701,7 @@ TEST(UtilTest, SystemError) {
 }
 
 TEST(UtilTest, ReportSystemError) {
-  fmt::Writer out;
+  fmt::MemoryWriter out;
   fmt::internal::format_system_error(out, EDOM, "test error");
   out << '\n';
   EXPECT_WRITE(stderr, fmt::report_system_error(EDOM, "test error"), out.str());
@@ -501,3 +743,5 @@ TEST(UtilTest, ReportWindowsError) {
 }
 
 #endif  // _WIN32
+
+// TODO: test Buffer