From 33bb6eef5a6ea487381a6e34edc00fc08e625809 Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Tue, 11 Dec 2012 21:47:05 -0800 Subject: [PATCH] Enable and fix warnings. --- CMakeLists.txt | 8 +++++++ format.cc | 37 +++++++++++++----------------- format.h | 61 ++++++++++++++++++++++++++++++++------------------ format_test.cc | 38 +++++++++++++++---------------- 4 files changed, 82 insertions(+), 62 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3ac59fa7..5b4d9c6b 100644 --- a/CMakeLists.txt +++ b/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 () diff --git a/format.cc b/format.cc index 468dd133..cc9d8ec1 100644 --- a/format.cc +++ b/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 -struct GetUnsigned; - -template <> -struct GetUnsigned { - typedef unsigned Type; +struct IntTraits { + typedef T UnsignedType; + static bool IsNegative(T) { return false; } }; template <> -struct GetUnsigned { - typedef unsigned Type; +struct IntTraits { + typedef unsigned UnsignedType; + static bool IsNegative(int value) { return value < 0; } }; template <> -struct GetUnsigned { - typedef unsigned long Type; -}; - -template <> -struct GetUnsigned { - typedef unsigned long Type; +struct IntTraits { + typedef unsigned long UnsignedType; + static bool IsNegative(long value) { return value < 0; } }; template @@ -93,9 +88,9 @@ template void fmt::Formatter::FormatInt(T value, unsigned flags, int width, char type) { int size = 0; char sign = 0; - typedef typename GetUnsigned::Type UnsignedType; + typedef typename IntTraits::UnsignedType UnsignedType; UnsignedType abs_value = value; - if (value < 0) { + if (IntTraits::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(width, size)); out = std::copy(str, str + size, out); - if (width > size) + if (static_cast(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); diff --git a/format.h b/format.h index eba81aac..6810c8fb 100644 --- a/format.h +++ b/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 - Arg(const T &value) - : type(CUSTOM), custom_value(&value), - format(&Formatter::FormatCustomArg) {} + Arg(const T &value) : type(CUSTOM), formatter(0) { + custom.value = &value; + custom.format = &Formatter::FormatCustomArg; + } ~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(width, str.size())); std::copy(str.begin(), str.end(), out); - if (width > str.size()) + if (static_cast(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_)); } diff --git a/format_test.cc b/format_test.cc index 54a72cc5..9148835c 100644 --- a/format_test.cc +++ b/format_test.cc @@ -69,8 +69,8 @@ class TestString { TEST(ArrayTest, Ctor) { fmt::Array 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 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 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; } };