Integral constant overflow in MSVC

[uecasm]

constexpr auto data = frozen::make_unordered_set<unsigned>({ 5u, 6u });

On MSVC 15.8.5, this reports

warning C4307: '+': integral constant overflow

Which then causes compilation to fail if using warnings as errors. This also occurs with unordered_map, but not with set or map. Oddly the warning goes away if constexpr is removed.

Is this warning benign or worrisome? Can it be fixed inside frozen somewhere? (Unfortunately it doesn't indicate which particular use of + causes the overflow -- it just reports on that line above, not on something inside frozen.)

[uecasm]

It looks like changing the definition of elsa to this fixes it for 32-bit MSVC. I assume that it would be more complicated for 64-bit, since it would have to do a 128-bit add. Unfortunately there doesn't appear to be any way to suppress the warning inside elsa itself, if it's called in a constexpr context, since the warning is raised at the point of use, not at its own code.

#if defined(_MSC_VER) && !defined(_WIN64)
  constexpr std::size_t operator()(T const &value, std::size_t seed) const {
      std::size_t key = seed ^ static_cast<std::size_t>(value);
      key = static_cast<std::size_t>(static_cast<uint64_t>(~key) + (key << 21));
      key = key ^ (key >> 24);
      key = static_cast<std::size_t>(static_cast<uint64_t>(key) + (key << 3) + (key << 8));
      key = key ^ (key >> 14);
      key = static_cast<std::size_t>(static_cast<uint64_t>(key) + (key << 2) + (key << 4));
      key = key ^ (key >> 28);
      key = static_cast<std::size_t>(static_cast<uint64_t>(key) + (key << 31));
      return key;
  }
#else
  constexpr std::size_t operator()(T const &value, std::size_t seed) const {
    std::size_t key = seed ^ static_cast<std::size_t>(value);
    key = (~key) + (key << 21); // key = (key << 21) - key - 1;
    key = key ^ (key >> 24);
    key = (key + (key << 3)) + (key << 8); // key * 265
    key = key ^ (key >> 14);
    key = (key + (key << 2)) + (key << 4); // key * 21
    key = key ^ (key >> 28);
    key = key + (key << 31);
    return key;
  }
#endif

[serge-sans-paille]

I'd say overflow on a plus on unsigned type is not an issue because it's perfectly defined (unlike the signed version). What could be done instead of your cast is to apply a mask, as in (~key + (key << 21)) & ~std::size_t(0), does that silent the warning? or maybe static_caststd::size_t(~key + (key << 21)` ? If either of these work, feel free to open a PR!

[uecasm]

No, those don't help; but I wouldn't really expect them to, since the warning is raised for the + operator itself, not the assignment back into key.

[degski]

@serge-sans-paille clang-cl doesn't warn on this. Maybe just suppress the warning?

[uecasm]

Sadly the warning is raised at point of constexpr variable use, not anywhere inside elsa itself, as I mentioned above. This makes it impossible to suppress the warning inside the library, except by changing the operand types of + as proposed above so that overflow can't occur.

I did file a bug with MS about the warning behaviour, so we can just sit on it and see if anything comes of that.

[serge-sans-paille]

On Thu, Feb 21, 2019 at 10:28:46PM -0800, degski wrote:

@serge-sans-paille clang-cl doesn't warn on this. Maybe just suppress the warning?

You mean, using a pragma, or using a compiler flag? Can we disable warning locally?

[degski]

@serge-sans-paille Yes, something like this, there are various options, more or less portable.

[uecasm]

@degski So, how many different ways do you want me to tell you that pragma suppressions don't work? Because this is the third time I've said that.