"integer constant overflow" warning in constexpr

Question

I'm trying to find a constexpr compatible hash function to use for hashing strings in compile-time. The number of strings are really small (<10) and I have a separate check for collisions, so the algorithm can be far from perfect. I found the following version of FNV1A somewhere on the internet:

static constexpr unsigned int Fnv1aBasis = 0x811C9DC5;
static constexpr unsigned int Fnv1aPrime = 0x01000193;

constexpr unsigned int hashFnv1a(const char *s, unsigned int h = Fnv1aBasis)
{
    return !*s ? h : hashFnv1a(s + 1, (h ^ *s) * Fnv1aPrime);
}

But when I compile this in MSVS 2015 I get the following warning:

warning C4307: '*': integral constant overflow

Since there's only one multiplication in the function I would assume the warning comes from (h ^ *s) * Fnv1aPrime. It makes sense since multiplying 0x811C9DC5 (Fnv1aBasis) with just about anything will make a 32-bit integer overflow.

Is there any way to work around this? I've tried a couple of other constexpr functions I've found for hashing strings, but all of them have the same issue.

I don't get this warning with rextester or rise4fun, using /W2 and /W4 respectively. — uh oh somebody needs a pupper
– uh oh somebody needs a pupper, Commented Jun 6, 2016 at 13:35
Consider using uint32_t in place of int (unsigned int might be 16 bits). Also char could be signed or unsigned, and if signed could be 1's complement. — Bathsheba
– Bathsheba, Commented Jun 6, 2016 at 13:37
I could change it to unsigned long long but since it does a multiplication for each character in the string it'll only work if the string is short enough. It also wouldn't work if I actually wanted a 64-bit hash (unless I were to use some 128-bit integer). I would expect compile-time hashing of strings to be a quite common use-case for constexpr, but I can't see how it could ever work if overflows trigger warnings since so many hash algorithms relies on overflows. — user408952
– user408952, Commented Jun 6, 2016 at 13:49

Cheers and hth. - Alf · Accepted Answer · 2016-06-06 18:12:40Z

9

If you don't mind the overflow then just silence the warning. Unsigned integer arithmetic is guaranteed to be modulo 2ⁿ arithmetic where n is the number of bits in the value representation, so this is well-defined no matter what. The warning is a sillywarning; it's warning you that you're using the main feature of unsigned integers.

I find that with a local #pragma warning( disable: 4307 ) for the function, the warning still appears for every use of the function.

This rewrite silences the warning for the 32-bit hash function:

constexpr auto hashFnv1a( char const* s, unsigned h = Fnv1aBasis )
    -> unsigned
{
    return !*s ? h : hashFnv1a(s + 1, static_cast<unsigned>( 1ULL*(h ^ *s) * Fnv1aPrime ));
}

Even extensive googling didn't find any way to disable the sillywarning about overflow of unsigned values while leaving it on for signed ones, so to deal with the 64-bit hash function it appears that the only recourse is to implement a constexpr 64-bit unsigned multiplication function. Since it's constexpr it doesn't matter if it's particularly efficient or not. So:

#include <stdint.h>

namespace b32 {
    static constexpr uint32_t Fnv1aBasis = 0x811C9DC5u;
    static constexpr uint32_t Fnv1aPrime = 0x01000193u;

    constexpr auto hashFnv1a( char const* s, uint32_t h = Fnv1aBasis )
        -> uint32_t
    { return !*s ? h : hashFnv1a(s + 1, static_cast<uint32_t>( 1ULL*(h ^ *s)*Fnv1aPrime )); }
}  // namespace b32

namespace b64 {
    static constexpr uint64_t Fnv1aBasis = 0xCBF29CE484222325uLL;
    static constexpr uint64_t Fnv1aPrime = 0x100000001B3uLL;

    constexpr auto lo( uint64_t x )
        -> uint64_t
    { return x & uint32_t( -1 ); }

    constexpr auto hi( uint64_t x )
        -> uint64_t
    { return x >> 32; }

    constexpr auto mulu64( uint64_t a, uint64_t b )
        -> uint64_t
    {
        return 0
            + (lo( a )*lo( b ) & uint32_t(-1))
            +   (
                    (
                        (
                            (
                                (
                                    hi( lo( a )*lo( b ) ) +
                                    lo( a )*hi( b )
                                )
                                & uint32_t(-1)
                            )
                            + hi( a )*lo( b )
                        )
                        & uint32_t(-1)
                    )
                    << 32
                );
    }

    constexpr auto hashFnv1a( char const* s, uint64_t h = Fnv1aBasis )
        -> uint64_t
    { return !*s ? h : hashFnv1a( s + 1, mulu64( h ^ *s, Fnv1aPrime ) ); }
}  // namepace b64

#include <assert.h>
#include <iostream>
using namespace std;
auto main()
    -> int
{
    constexpr auto x = b64::mulu64( b64::Fnv1aBasis, b64::Fnv1aPrime );

    #ifdef _MSC_VER
    #   pragma warning( push )
    #   pragma warning( disable: 4307 )
        constexpr auto y = b64::Fnv1aBasis*b64::Fnv1aPrime;
    #   pragma warning( pop )
    #else
        constexpr auto y = b64::Fnv1aBasis*b64::Fnv1aPrime;
    #endif

    cout << x << endl;
    cout << y << endl;
    assert( x == y );

    static constexpr const char* const s = "blah!";
    constexpr unsigned xs = b32::hashFnv1a( s );
    constexpr uint64_t ys = b64::hashFnv1a( s );

    int a[1 + xs%2];  (void) a;
    int b[1 + ys%2];  (void) b;
}

edited Jun 6, 2016 at 18:12

answered Jun 6, 2016 at 14:02

Cheers and hth. - Alf

146k15 gold badges218 silver badges342 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

user408952 Over a year ago

The function is in a header and putting a #pragma warning push + disable + pop around the function doesn't to anything. It looks like I have to add it around all the places that call the function in a constexpr way, which is far from ideal. I don't want to disable the warning globally (or everywhere after including the header).

Cheers and hth. - Alf Over a year ago

@user408952: okay, let me look at it

Cheers and hth. - Alf Over a year ago

@user408952: I updated the answer, killing that warning also for the 64-bit hasher. Which is the one I think you should be using. ;-)

Justin Time - Reinstate Monica Over a year ago

I honestly find it really odd that this warning isn't off by default, considering that some of the ones that are (e.g., C4265, "probably-polymorphic class has non-virtual dtor") are more useful than "warning: unsigned wraparound".

coyotte508 · Accepted Answer · 2016-06-06 15:54:42Z

4

You can explicitly cast to unsigned long long and back, as follows:

constexpr unsigned int hashFnv1b(const char *s, unsigned int h = Fnv1aBasis)
{
    return !*s
           ? h
           : hashFnv1b(
               s + 1,
               static_cast<unsigned int>(
                 (h ^ *s) * static_cast<unsigned long long>(Fnv1aPrime)));
}

This stops the warning in my live demo (line 20 triggers it, line 21 does not).

edited Jun 6, 2016 at 15:54

coyotte508

9,8406 gold badges48 silver badges66 bronze badges

answered Jun 6, 2016 at 15:42

mindriot

5,6981 gold badge27 silver badges34 bronze badges

Comments

Dan Olson · Accepted Answer · 2017-07-24 21:55:15Z

3

Another way is to wrap it in a macro and use __pragma to shut down the warning:

#include <type_traits>
#if _MSC_VER
#define FNV_HASH( str ) \
  __pragma( warning( push ) ) \
  __pragma( warning( disable: 4307 ) ) \
  std::integral_constant<uint64_t, hashFnv1a( str )>::value \
  __pragma( warning( pop ) )
#else
#define FNV_HASH( str ) std::integral_constant<uint64_t, hashFnv1a( str )>::value
#endif

The std::integral_constant forces the compiler into compile-time evaluating the expression, whereas it's optional outside of compile-time contexts otherwise.

This is a bit easier for the 64-bit version than implementing your own constexpr 64-bit multiply.

answered Jul 24, 2017 at 21:55

Dan Olson

23.5k4 gold badges45 silver badges56 bronze badges

Collectives™ on Stack Overflow

"integer constant overflow" warning in constexpr

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related