Enable and fix warnings.

CMakeLists.txt

@@ -11,6 +11,10 @@ endif ()
 project(FORMAT)
 
 add_library(format format.cc)
+if (CMAKE_COMPILER_IS_GNUCXX)
+  set_target_properties(format PROPERTIES COMPILE_FLAGS
+    "-Wall -Wextra -pedantic")
+endif ()
 
 # We compile Google Test ourselves instead of using pre-compiled libraries.
 # See the Google Test FAQ "Why is it not recommended to install a
@@ -23,6 +27,10 @@ if (EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/gtest/CMakeLists.txt)
   enable_testing()
   add_executable(format_test format_test.cc gtest/src/gtest_main.cc)
   target_link_libraries(format_test format gtest)
+  if (CMAKE_COMPILER_IS_GNUCXX)
+    set_target_properties(format_test PROPERTIES COMPILE_FLAGS
+      "-Wall -Wextra -pedantic -Wno-long-long -Wno-variadic-macros")
+  endif ()
   add_test(format_test format_test)
 endif ()
 

format.cc

@@ -58,28 +58,23 @@ unsigned ParseUInt(const char *&s) {
   return value;
 }
 
-// Maps an integer type T to its unsigned counterpart.
+// Information about an integer type.
 template <typename T>
-struct GetUnsigned;
-
-template <>
-struct GetUnsigned<int> {
-  typedef unsigned Type;
-};
-
-template <>
-struct GetUnsigned<unsigned> {
-  typedef unsigned Type;
+struct IntTraits {
+  typedef T UnsignedType;
+  static bool IsNegative(T) { return false; }
 };
 
 template <>
-struct GetUnsigned<long> {
-  typedef unsigned long Type;
+struct IntTraits<int> {
+  typedef unsigned UnsignedType;
+  static bool IsNegative(int value) { return value < 0; }
 };
 
 template <>
-struct GetUnsigned<unsigned long> {
-  typedef unsigned long Type;
+struct IntTraits<long> {
+  typedef unsigned long UnsignedType;
+  static bool IsNegative(long value) { return value < 0; }
 };
 
 template <typename T>
@@ -93,9 +88,9 @@ template <typename T>
 void fmt::Formatter::FormatInt(T value, unsigned flags, int width, char type) {
   int size = 0;
   char sign = 0;
-  typedef typename GetUnsigned<T>::Type UnsignedType;
+  typedef typename IntTraits<T>::UnsignedType UnsignedType;
   UnsignedType abs_value = value;
-  if (value < 0) {
+  if (IntTraits<T>::IsNegative(value)) {
     sign = '-';
     ++size;
     abs_value = -value;
@@ -337,13 +332,13 @@ void fmt::Formatter::DoFormat() {
     case STRING: {
       if (type && type != 's')
         ReportUnknownType(type, "string");
-      const char *str = arg.string_value;
-      size_t size = arg.size;
+      const char *str = arg.string.value;
+      size_t size = arg.string.size;
       if (size == 0 && *str)
         size = std::strlen(str);
       char *out = GrowBuffer(std::max<size_t>(width, size));
       out = std::copy(str, str + size, out);
-      if (width > size)
+      if (static_cast<unsigned>(width) > size)
         std::fill_n(out, width - size, ' ');
       break;
     }
@@ -356,7 +351,7 @@ void fmt::Formatter::DoFormat() {
     case CUSTOM:
       if (type)
         ReportUnknownType(type, "object");
-      (this->*arg.format)(arg.custom_value, width);
+      (this->*arg.custom.format)(arg.custom.value, width);
       break;
     default:
       assert(false);

format.h

@@ -151,34 +151,50 @@ class Formatter {
       long double long_double_value;
       const void *pointer_value;
       struct {
-        const char *string_value;
+        const char *value;
         std::size_t size;
-      };
+      } string;
       struct {
-        const void *custom_value;
+        const void *value;
         FormatFunc format;
-      };
+      } custom;
     };
     mutable Formatter *formatter;
 
-    Arg(int value) : type(INT), int_value(value) {}
-    Arg(unsigned value) : type(UINT), uint_value(value) {}
-    Arg(long value) : type(LONG), long_value(value) {}
-    Arg(unsigned long value) : type(ULONG), ulong_value(value) {}
-    Arg(double value) : type(DOUBLE), double_value(value) {}
-    Arg(long double value) : type(LONG_DOUBLE), long_double_value(value) {}
-    Arg(char value) : type(CHAR), int_value(value) {}
-    Arg(const char *value) : type(STRING), string_value(value), size(0) {}
-    Arg(char *value) : type(STRING), string_value(value), size(0) {}
-    Arg(const void *value) : type(POINTER), pointer_value(value) {}
-    Arg(void *value) : type(POINTER), pointer_value(value) {}
-    Arg(const std::string &value)
-    : type(STRING), string_value(value.c_str()), size(value.size()) {}
+    Arg(int value) : type(INT), int_value(value), formatter(0) {}
+    Arg(unsigned value) : type(UINT), uint_value(value), formatter(0) {}
+    Arg(long value) : type(LONG), long_value(value), formatter(0) {}
+    Arg(unsigned long value) : type(ULONG), ulong_value(value), formatter(0) {}
+    Arg(double value) : type(DOUBLE), double_value(value), formatter(0) {}
+    Arg(long double value)
+    : type(LONG_DOUBLE), long_double_value(value), formatter(0) {}
+    Arg(char value) : type(CHAR), int_value(value), formatter(0) {}
+
+    Arg(const char *value) : type(STRING), formatter(0) {
+      string.value = value;
+      string.size = 0;
+    }
+
+    Arg(char *value) : type(STRING), formatter(0) {
+      string.value = value;
+      string.size = 0;
+    }
+
+    Arg(const void *value)
+    : type(POINTER), pointer_value(value), formatter(0) {}
+
+    Arg(void *value) : type(POINTER), pointer_value(value), formatter(0) {}
+
+    Arg(const std::string &value) : type(STRING), formatter(0) {
+      string.value = value.c_str();
+      string.size = value.size();
+    }
 
     template <typename T>
-    Arg(const T &value)
-    : type(CUSTOM), custom_value(&value),
-      format(&Formatter::FormatCustomArg<T>) {}
+    Arg(const T &value) : type(CUSTOM), formatter(0) {
+      custom.value = &value;
+      custom.format = &Formatter::FormatCustomArg<T>;
+    }
 
     ~Arg() {
       // Format is called here to make sure that a referred object is
@@ -321,7 +337,7 @@ void Formatter::FormatCustomArg(const void *arg, int width) {
   std::string str(os.str());
   char *out = GrowBuffer(std::max<std::size_t>(width, str.size()));
   std::copy(str.begin(), str.end(), out);
-  if (width > str.size())
+  if (static_cast<unsigned>(width) > str.size())
     std::fill_n(out + str.size(), width - str.size(), ' ');
 }
 
@@ -359,7 +375,8 @@ class ActiveFormatter : public Formatter::ArgInserter {
   // destructor. Note that the buffer content is not copied because the
   // the buffer in ActiveFormatter is populated when all the arguments
   // are provided.
-  ActiveFormatter(ActiveFormatter &other) : action_(other.action_) {
+  ActiveFormatter(ActiveFormatter &other)
+  : ArgInserter(0), action_(other.action_) {
     other.ResetFormatter();
     ArgInserter::operator=(formatter_(other.formatter_.format_));
   }

format_test.cc

@@ -69,8 +69,8 @@ class TestString {
 
 TEST(ArrayTest, Ctor) {
   fmt::Array<char, 123> array;
-  EXPECT_EQ(0, array.size());
-  EXPECT_EQ(123, array.capacity());
+  EXPECT_EQ(0u, array.size());
+  EXPECT_EQ(123u, array.capacity());
 }
 
 TEST(ArrayTest, Access) {
@@ -88,12 +88,12 @@ TEST(ArrayTest, Resize) {
   array[10] = 42;
   EXPECT_EQ(42, array[10]);
   array.resize(20);
-  EXPECT_EQ(20, array.size());
-  EXPECT_EQ(123, array.capacity());
+  EXPECT_EQ(20u, array.size());
+  EXPECT_EQ(123u, array.capacity());
   EXPECT_EQ(42, array[10]);
   array.resize(5);
-  EXPECT_EQ(5, array.size());
-  EXPECT_EQ(123, array.capacity());
+  EXPECT_EQ(5u, array.size());
+  EXPECT_EQ(123u, array.capacity());
   EXPECT_EQ(42, array[10]);
 }
 
@@ -103,8 +103,8 @@ TEST(ArrayTest, Grow) {
   for (int i = 0; i < 10; ++i)
     array[i] = i * i;
   array.resize(20);
-  EXPECT_EQ(20, array.size());
-  EXPECT_EQ(20, array.capacity());
+  EXPECT_EQ(20u, array.size());
+  EXPECT_EQ(20u, array.capacity());
   for (int i = 0; i < 10; ++i)
     EXPECT_EQ(i * i, array[i]);
 }
@@ -113,20 +113,20 @@ TEST(ArrayTest, Clear) {
   fmt::Array<char, 10> array;
   array.resize(20);
   array.clear();
-  EXPECT_EQ(0, array.size());
-  EXPECT_EQ(20, array.capacity());
+  EXPECT_EQ(0u, array.size());
+  EXPECT_EQ(20u, array.capacity());
 }
 
 TEST(ArrayTest, PushBack) {
   fmt::Array<int, 10> array;
   array.push_back(11);
   EXPECT_EQ(11, array[0]);
-  EXPECT_EQ(1, array.size());
+  EXPECT_EQ(1u, array.size());
   array.resize(10);
   array.push_back(22);
   EXPECT_EQ(22, array[10]);
-  EXPECT_EQ(11, array.size());
-  EXPECT_EQ(15, array.capacity());
+  EXPECT_EQ(11u, array.size());
+  EXPECT_EQ(15u, array.capacity());
 }
 
 TEST(ArrayTest, Append) {
@@ -134,13 +134,13 @@ TEST(ArrayTest, Append) {
   const char *test = "test";
   array.append(test, test + 5);
   EXPECT_STREQ("test", &array[0]);
-  EXPECT_EQ(5, array.size());
+  EXPECT_EQ(5u, array.size());
   array.resize(10);
   array.append(test, test + 2);
   EXPECT_EQ('t', array[10]);
   EXPECT_EQ('e', array[11]);
-  EXPECT_EQ(12, array.size());
-  EXPECT_EQ(15, array.capacity());
+  EXPECT_EQ(12u, array.size());
+  EXPECT_EQ(15u, array.capacity());
 }
 
 TEST(FormatterTest, Escape) {
@@ -363,7 +363,7 @@ void CheckUnknownTypes(
   const char *special = ".0123456789}";
   for (int c = CHAR_MIN; c <= CHAR_MAX; ++c) {
     if (std::strchr(types, c) || std::strchr(special, c) || !c) continue;
-    sprintf(format, "{0:1%c}", c, type_name);
+    sprintf(format, "{0:1%c}", c);
     if (std::isprint(c))
       sprintf(message, "unknown format code '%c' for %s", c, type_name);
     else
@@ -525,7 +525,7 @@ TEST(FormatterTest, FormatStringFromSpeedTest) {
 
 TEST(FormatterTest, FormatterCtor) {
   Formatter format;
-  EXPECT_EQ(0, format.size());
+  EXPECT_EQ(0u, format.size());
   EXPECT_STREQ("", format.data());
   EXPECT_STREQ("", format.c_str());
   EXPECT_EQ("", format.str());
@@ -618,7 +618,7 @@ TEST(ActiveFormatterTest, ArgLifetime) {
 
 struct PrintError {
   void operator()(const fmt::Formatter &f) const {
-    //std::cerr << "Error: " << f.str() << std::endl;
+    std::cerr << "Error: " << f.str() << std::endl;
   }
 };