2

Today I wanted to write a small function to render integers up to 64bit in binary. There I ran across a problem which is apparently due to silent integer promotion. Below source code shows the problem.

#include <iostream>
#include <string>

using namespace std;

string pretty(uint64_t);
string pretty_correct(uint64_t);

string pretty(uint64_t var)
{
    string result;
    result.reserve(50);
    for ( short i=63; i>=0; --i ) {
        if ( var & (0x1 << i) )
            result.append(1,'|');
        else
            result.append(1,'O');
    }

    return result;
}

string pretty_correct(uint64_t var)
{
    string result;
    result.reserve(50);
    for ( short i=63; i>=0; --i ) {
        if ( var & (static_cast<unsigned short>(0x1 << i)))
            result.append(1,'|');
        else
            result.append(1,'O');
    }

    return result;
}

int main() {
    cout << pretty(12345678901234567890U) << " <- wrong!"<< endl;
    cout << pretty_correct(12345678901234567890U) << " <- right!"<< endl;
}

I am looking for a way for GCC (or maybe Clang) to spit out a warning for this type of problem. All solutions that were presented to me so far involved me somehow presciently being aware that this kind of problem might occur. But the way this occurs now it might introduce subtle bugs into my code, which would be difficult to track down.

Edit: These are the commands I'm using for compilation: g++ -std=c++14 -Wall -Wextra -pedantic-errors -Wconversion -g -o testcase testcase.cpp clang -std=c++14 -Weverything -g -o testcase testcase.cpp -lm -lstdc++

M.B.
  • 45
  • 4

1 Answers1

1

First, your pretty_correct() function is in fact incorrect. The reason is that the expression 0x1 << i yields UB when i >= 32: you can only shift signed by not more than the length of the type (source). Probably this is the warning you expect to see.

There are several ways to fix it. The first one is to add type-qualifier to 1:

if (var & (1ull << i))

Though I would prefer

if ((var >> i) & 1)

as you don't have to think about "must it be 1u or 1ll or whatever".

Now about warnings: no, there are no such warnings neither in GCC nor in Clang. However, Clang sanitizer gives you an option to check it in runtime with -fsanitize=shift:

$ clang++-3.5 -std=c++11 -Wall -Wextra -O2 a.cpp -fsanitize=shift
$ ./a.out 
a.cpp:14:25: runtime error: shift exponent 63 is too large for 32-bit type 'int'
|||O|O||OOO|||||OOOO|O|O||O|OO|O|||O|O||OOO|||||OOOO|O|O||O|OO|O <- wrong!
a.cpp:28:53: runtime error: shift exponent 63 is too large for 32-bit type 'int'
OOOOOOOOOOOOOOOOOOOO|O|O||O|OO|OOOOOOOOOOOOOOOOOOOOO|O|O||O|OO|O <- right!
Community
  • 1
  • 1
Ivan Smirnov
  • 4,365
  • 19
  • 30
  • gcc has `-Wshift-count-overflow` which sometimes works, although apparently it can't figure it out for this particular program – M.M Mar 17 '17 at 08:02
  • @M.M I've seen such errors if rhs is constant. (1 << 100) yields a warning, but {n = 100; 1 << n; } does not. – Ivan Smirnov Mar 17 '17 at 08:05