Compilation warning/error with gcc 6.3.0

[pitrou]

We just got this after bumping our vendored copy to the latest git master (052975d):

In file included from /arrow/cpp/src/arrow/vendored/fast_float/parse_number.h:6:0,
                 from /arrow/cpp/src/arrow/vendored/fast_float/fast_float.h:65,
                 from /arrow/cpp/src/arrow/util/value_parsing.cc:23:
/arrow/cpp/src/arrow/vendored/fast_float/digit_comparison.h: In instantiation of ‘arrow_vendored::fast_float::adjusted_mantissa arrow_vendored::fast_float::to_extended(T) [with T = double]’:
/arrow/cpp/src/arrow/vendored/fast_float/digit_comparison.h:92:37:   required from ‘arrow_vendored::fast_float::adjusted_mantissa arrow_vendored::fast_float::to_extended_halfway(T) [with T = double]’
/arrow/cpp/src/arrow/vendored/fast_float/digit_comparison.h:354:48:   required from ‘arrow_vendored::fast_float::adjusted_mantissa arrow_vendored::fast_float::negative_digit_comp(arrow_vendored::fast_float::bigint&, arrow_vendored::fast_float::adjusted_mantissa, int32_t) [with T = double; int32_t = int]’
/arrow/cpp/src/arrow/vendored/fast_float/digit_comparison.h:418:34:   required from ‘arrow_vendored::fast_float::adjusted_mantissa arrow_vendored::fast_float::digit_comp(arrow_vendored::fast_float::parsed_number_string&, arrow_vendored::fast_float::adjusted_mantissa) [with T = double]’
/arrow/cpp/src/arrow/vendored/fast_float/parse_number.h:107:41:   required from ‘arrow_vendored::fast_float::from_chars_result arrow_vendored::fast_float::from_chars_advanced(const char*, const char*, T&, arrow_vendored::fast_float::parse_options) [with T = double]’
/arrow/cpp/src/arrow/util/value_parsing.cc:40:85:   required from here
/arrow/cpp/src/arrow/vendored/fast_float/digit_comparison.h:62:50: error: right shift count >= width of type [-Werror=shift-count-overflow]
       am.power2 = int32_t((bits & exponent_mask) >> binary_format<T>::mantissa_explicit_bits());
                           ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[lemire]

Here is the full code dump...

template <typename T>
fastfloat_really_inline adjusted_mantissa to_extended(T value) noexcept {
  adjusted_mantissa am;
  int32_t bias = binary_format<T>::mantissa_explicit_bits() - binary_format<T>::minimum_exponent();
  if (std::is_same<T, float>::value) {
    constexpr uint32_t exponent_mask = 0x7F800000;
    constexpr uint32_t mantissa_mask = 0x007FFFFF;
    constexpr uint64_t hidden_bit_mask = 0x00800000;
    uint32_t bits;
    ::memcpy(&bits, &value, sizeof(T));
    if ((bits & exponent_mask) == 0) {
      // denormal
      am.power2 = 1 - bias;
      am.mantissa = bits & mantissa_mask;
    } else {
      // normal
      am.power2 = int32_t((bits & exponent_mask) >> binary_format<T>::mantissa_explicit_bits()); // <-------- report is here
      am.power2 -= bias;
      am.mantissa = (bits & mantissa_mask) | hidden_bit_mask;
    }
  } else {
    constexpr uint64_t exponent_mask = 0x7FF0000000000000;
    constexpr uint64_t mantissa_mask = 0x000FFFFFFFFFFFFF;
    constexpr uint64_t hidden_bit_mask = 0x0010000000000000;
    uint64_t bits;
    ::memcpy(&bits, &value, sizeof(T));
    if ((bits & exponent_mask) == 0) {
      // denormal
      am.power2 = 1 - bias;
      am.mantissa = bits & mantissa_mask;
    } else {
      // normal
      am.power2 = int32_t((bits & exponent_mask) >> binary_format<T>::mantissa_explicit_bits());
      am.power2 -= bias;
      am.mantissa = (bits & mantissa_mask) | hidden_bit_mask;
    }
  }

  return am;
}

[pitrou]

Right. Since if constexpr is not available before C++17, it seems a reasonable workaround would be to specialize the function instead of using an inline if to choose the right implementation.

[lemire]

@pitrou Does the error go away if you substitute...

int32_t((bits & exponent_mask) >> binary_format<float>::mantissa_explicit_bits());

for

int32_t((bits & exponent_mask) >> binary_format<T>::mantissa_explicit_bits());

[lemire]

it seems a reasonable workaround would be to specialize the function instead of using an inline if to choose the right implementation.

That would also work, of course.

[lemire]

@pitrou

Would you consider doing a PR? It should be easy and save me the trouble of verifying the fix with you.

[pitrou]

Yes, I'll try to.

[pitrou]

Submitted #118

[lemire]

New release: 3.4.0.