Fix conversion warnings (#989)

* Fix sign conversion warnings

* Ignore unused local typedefs

* Remove cast reported as useless on GCC

* Remove warning on unused variable

* Enable conversion warning checking for pedantic build

* Fix sign-conversion warnings in headers

CMakeLists.txt

@@ -74,10 +74,11 @@ if (CMAKE_CXX_COMPILER_ID MATCHES "GNU")
       -Wcast-align -Wnon-virtual-dtor
       -Wctor-dtor-privacy -Wdisabled-optimization
       -Winvalid-pch -Woverloaded-virtual
-      -Wno-ctor-dtor-privacy -Wno-dangling-else
-      -Wno-format-nonliteral -Wno-sign-conversion -Wno-shadow)
+      -Wconversion
+      -Wno-ctor-dtor-privacy -Wno-format-nonliteral -Wno-shadow)
   if (NOT CMAKE_CXX_COMPILER_VERSION VERSION_LESS 4.6)
-      set(PEDANTIC_COMPILE_FLAGS ${PEDANTIC_COMPILE_FLAGS} -Wnoexcept)
+      set(PEDANTIC_COMPILE_FLAGS ${PEDANTIC_COMPILE_FLAGS} -Wnoexcept
+         -Wno-dangling-else -Wno-unused-local-typedefs)
   endif ()
   if (NOT CMAKE_CXX_COMPILER_VERSION VERSION_LESS 5.0)
       set(PEDANTIC_COMPILE_FLAGS ${PEDANTIC_COMPILE_FLAGS} -Wdouble-promotion
@@ -92,7 +93,7 @@ if (CMAKE_CXX_COMPILER_ID MATCHES "GNU")
 endif ()
 
 if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
-  set(PEDANTIC_COMPILE_FLAGS -Wall -Wextra -pedantic)
+  set(PEDANTIC_COMPILE_FLAGS -Wall -Wextra -pedantic -Wconversion -Wno-sign-conversion)
   check_cxx_compiler_flag(-Wzero-as-null-pointer-constant HAS_NULLPTR_WARNING)
   if (HAS_NULLPTR_WARNING)
     set(PEDANTIC_COMPILE_FLAGS ${PEDANTIC_COMPILE_FLAGS}

include/fmt/chrono.h

@@ -250,7 +250,7 @@ struct chrono_formatter {
   void write(int value, int width) {
     typedef typename int_traits<int>::main_type main_type;
     main_type n = to_unsigned(value);
-    int num_digits = static_cast<int>(internal::count_digits(n));
+    int num_digits = internal::count_digits(n);
     if (width > num_digits)
       out = std::fill_n(out, width - num_digits, '0');
     out = format_decimal<char_type>(out, n, num_digits);
@@ -415,7 +415,7 @@ struct formatter<std::chrono::duration<Rep, Period>, Char> {
     if (begin == end) return begin;
     begin = internal::parse_width(begin, end, handler);
     end = parse_chrono_format(begin, end, internal::chrono_format_checker());
-    format_str = basic_string_view<Char>(&*begin, end - begin);
+    format_str = basic_string_view<Char>(&*begin, internal::to_unsigned(end - begin));
     return end;
   }
 

include/fmt/color.h

@@ -246,7 +246,8 @@ struct color_type {
   }
   FMT_CONSTEXPR color_type(rgb rgb_color) FMT_NOEXCEPT
     : is_rgb(true), value{} {
-    value.rgb_color = (rgb_color.r << 16) + (rgb_color.g << 8) + rgb_color.b;
+    value.rgb_color = (static_cast<uint32_t>(rgb_color.r) << 16)
+       | (static_cast<uint32_t>(rgb_color.g) << 8) | rgb_color.b;
   }
   FMT_CONSTEXPR color_type(terminal_color term_color) FMT_NOEXCEPT
     : is_rgb(), value{} {
@@ -395,22 +396,22 @@ struct ansi_color_escape {
     // sequence.
     if (!text_color.is_rgb) {
       bool is_background = esc == internal::data::BACKGROUND_COLOR;
-      uint8_t value = text_color.value.term_color;
+      uint32_t value = text_color.value.term_color;
       // Background ASCII codes are the same as the foreground ones but with
       // 10 more.
       if (is_background)
-        value += 10;
+        value += 10u;
 
       std::size_t index = 0;
       buffer[index++] = static_cast<Char>('\x1b');
       buffer[index++] = static_cast<Char>('[');
 
-      if (value >= 100) {
+      if (value >= 100u) {
         buffer[index++] = static_cast<Char>('1');
-        value %= 100;
+        value %= 100u;
       }
-      buffer[index++] = static_cast<Char>('0' + value / 10);
-      buffer[index++] = static_cast<Char>('0' + value % 10);
+      buffer[index++] = static_cast<Char>('0' + value / 10u);
+      buffer[index++] = static_cast<Char>('0' + value % 10u);
 
       buffer[index++] = static_cast<Char>('m');
       buffer[index++] = static_cast<Char>('\0');
@@ -452,7 +453,7 @@ struct ansi_color_escape {
   FMT_CONSTEXPR operator const Char *() const FMT_NOEXCEPT { return buffer; }
 
 private:
-  Char buffer[7 + 3 * 4 + 1];
+  Char buffer[7u + 3u * 4u + 1u];
 
   static FMT_CONSTEXPR void to_esc(uint8_t c, Char *out,
                                    char delimiter) FMT_NOEXCEPT {

include/fmt/format-inl.h

@@ -172,7 +172,7 @@ void format_error_code(internal::buffer &out, int error_code,
     abs_value = 0 - abs_value;
     ++error_code_size;
   }
-  error_code_size += internal::count_digits(abs_value);
+  error_code_size += internal::to_unsigned(internal::count_digits(abs_value));
   writer w(out);
   if (message.size() <= inline_buffer_size - error_code_size) {
     w.write(message);
@@ -464,14 +464,14 @@ FMT_FUNC fp get_cached_power(int min_exponent, int &pow10_exponent) {
 }
 
 FMT_FUNC bool grisu2_round(
-    char *buf, ptrdiff_t &size, size_t max_digits, uint64_t delta,
+    char *buf, int &size, int max_digits, uint64_t delta,
     uint64_t remainder, uint64_t exp, uint64_t diff, int &exp10) {
   while (remainder < diff && delta - remainder >= exp &&
         (remainder + exp < diff || diff - remainder > remainder + exp - diff)) {
     --buf[size - 1];
     remainder += exp;
   }
-  if (size > static_cast<ptrdiff_t>(max_digits)) {
+  if (size > max_digits) {
     --size;
     ++exp10;
     if (buf[size] >= '5')
@@ -482,8 +482,8 @@ FMT_FUNC bool grisu2_round(
 
 // Generates output using Grisu2 digit-gen algorithm.
 FMT_FUNC bool grisu2_gen_digits(
-    char *buf, ptrdiff_t &size, uint32_t hi, uint64_t lo, int &exp,
-    uint64_t delta, const fp &one, const fp &diff, size_t max_digits) {
+    char *buf, int &size, uint32_t hi, uint64_t lo, int &exp,
+    uint64_t delta, const fp &one, const fp &diff, int max_digits) {
   // Generate digits for the most significant part (hi).
   while (exp > 0) {
     uint32_t digit = 0;
@@ -507,7 +507,7 @@ FMT_FUNC bool grisu2_gen_digits(
       buf[size++] = static_cast<char>('0' + digit);
     --exp;
     uint64_t remainder = (static_cast<uint64_t>(hi) << -one.e) + lo;
-    if (remainder <= delta || size > static_cast<ptrdiff_t>(max_digits)) {
+    if (remainder <= delta || size > max_digits) {
       return grisu2_round(
             buf, size, max_digits, delta, remainder,
             static_cast<uint64_t>(data::POWERS_OF_10_32[exp]) << -one.e,
@@ -523,7 +523,7 @@ FMT_FUNC bool grisu2_gen_digits(
       buf[size++] = static_cast<char>('0' + digit);
     lo &= one.f - 1;
     --exp;
-    if (lo < delta || size > static_cast<ptrdiff_t>(max_digits)) {
+    if (lo < delta || size > max_digits) {
       return grisu2_round(buf, size, max_digits, delta, lo, one.f,
                           diff.f * data::POWERS_OF_10_32[-exp], exp);
     }
@@ -704,7 +704,7 @@ FMT_FUNC gen_digits_params process_specs(const core_format_specs &specs,
     ++num_digits;
     break;
   }
-  params.num_digits = to_unsigned(num_digits);
+  params.num_digits = num_digits;
   char_counter counter{num_digits};
   grisu2_prettify(params, params.num_digits, exp - num_digits, counter);
   buf.resize(to_unsigned(counter.size));
@@ -739,7 +739,7 @@ FMT_FUNC typename std::enable_if<sizeof(Double) == sizeof(uint64_t), bool>::type
   // hi (p1 in Grisu) contains the most significant digits of scaled_upper.
   // hi = floor(upper / one).
   uint32_t hi = static_cast<uint32_t>(upper.f >> -one.e);
-  int exp = static_cast<int>(count_digits(hi));  // kappa in Grisu.
+  int exp = count_digits(hi);  // kappa in Grisu.
   gen_digits_params params = process_specs(specs, cached_exp + exp, buf);
   fp_value.normalize();
   fp scaled_value = fp_value * cached_pow;
@@ -750,7 +750,7 @@ FMT_FUNC typename std::enable_if<sizeof(Double) == sizeof(uint64_t), bool>::type
   // lo (p2 in Grisu) contains the least significants digits of scaled_upper.
   // lo = supper % one.
   uint64_t lo = upper.f & (one.f - 1);
-  ptrdiff_t size = 0;
+  int size = 0;
   if (!grisu2_gen_digits(buf.data(), size, hi, lo, exp, delta, one, diff,
                          params.num_digits)) {
     buf.clear();

include/fmt/printf.h

@@ -631,7 +631,7 @@ void basic_printf_context<OutputIt, Char, AF>::format() {
 
     format_arg arg = get_arg(it, arg_index);
     if (spec.has(HASH_FLAG) && visit_format_arg(internal::is_zero_int(), arg))
-      spec.flags &= ~internal::to_unsigned<int>(HASH_FLAG);
+      spec.flags = static_cast<uint_least8_t>(spec.flags & (~internal::to_unsigned<int>(HASH_FLAG)));
     if (spec.fill_ == '0') {
       if (arg.is_arithmetic())
         spec.align_ = ALIGN_NUMERIC;

include/fmt/time.h

@@ -122,7 +122,7 @@ struct formatter<std::tm, Char> {
     auto end = it;
     while (end != ctx.end() && *end != '}')
       ++end;
-    tm_format.reserve(end - it + 1);
+    tm_format.reserve(internal::to_unsigned(end - it + 1));
     tm_format.append(it, end);
     tm_format.push_back('\0');
     return end;

test/core-test.cc

@@ -492,7 +492,7 @@ class QString {
 
 inline fmt::basic_string_view<wchar_t> to_string_view(
     const QString &s) FMT_NOEXCEPT {
-  return {reinterpret_cast<const wchar_t *>(s.utf16()),
+  return {s.utf16(),
           static_cast<std::size_t>(s.size())};
 }
 }

test/format-test.cc

@@ -1917,6 +1917,7 @@ TEST(FormatTest, Enum) {
 
 TEST(FormatTest, EnumFormatterUnambiguous) {
   fmt::formatter<TestEnum> f;
+  ASSERT_GE(sizeof(f), 0); // use f to avoid compiler warning
 }
 
 #if FMT_HAS_FEATURE(cxx_strong_enums)