Warnings Series - autological out of range

In this series of posts I'm discussing how compiling with alternative compilers can help to catch errors at development time. In the last post I discussed how to detect uninitialized errors. In this post, I'm going to look at some logic errors related to using incorrect types. The type of error in this post was encountered when consuming a library and ended up in production code.

Background

For this example, we have a library that returns a result as a uint8_t. The values to return come from defines in the header file.

1
2
3
4
5
6
// coin.h 
#define COIN_HEAD 0
#define COIN_TAIL 1
#define COIN_ERR 0xFFFF
 
uint8_t get_coin(const std::string& coin);

Autological Out of Range

The code to consume the above header file is as follows:

1
2
3
4
5
6
7
8
9
// game.cpp
int main() {
    auto c = get_coin("head");
    if(c != COIN_ERR) {
        std::cout << "valid coin " << c << std::endl;
    }
 
    return 0;
}

In the above code, we get the number representation of a coin face based on the string. If the string is valid, it is printed, otherwise we do nothing.

This code compiles with no warnings on GCC with -Wall

Problems with the code

The problem with the above example is that if you pass an unknown string into get_coin, the return will convert the COIN_ERR definition from 0xFFFF into 0xFF. When you then do the comparison c != COIN_ERR, it is always true because c can never be equal to 0xFFFF. This is caused because COIN_ERR and the return value of get_coin are incompatible.

Catching the error

Clang

When compiling with Clang and any warning level you will get the following warning:

1
2
3
4
<source>:35:10: warning: comparison of constant 65535 with expression of type 'unsigned char' is always true [-Wtautological-constant-out-of-range-compare]
    if(c != COIN_ERR) {
       ~ ^  ~~~~~~~~
1 warning generated.

GCC

As mentioned, on GCC with the compiler flags -Wall, there are no errors in the code. Enabling other warnings with -Wextra and -pedantic fails to trigger a warning.

MSVC

The warning failed to trigger on MVSC with any warning level.

Catching early

If we go back and look at the library code that this came from, the definition of get_coin is as follows:

1
2
3
4
5
6
7
8
9
10
11
// coin.cpp
uint8_t get_coin(const std::string& coin)
{
    if(coin == "head") {
        return COIN_HEAD;
    } else if(coin == "tail") {
        return COIN_TAIL;
    }
 
    return COIN_ERR;
}

If you recompile the library, this code will produce a warning on GCC, Clang, and MSVC. The Clang version of this warning is:

1
2
3
4
5
<source>:29:12: warning: implicit conversion from 'int' to 'uint8_t' (aka 'unsigned char') changes value from 65535 to 255 [-Wconstant-conversion]
    return COIN_ERR;
    ~~~~~~ ^~~~~~~~
<source>:16:18: note: expanded from macro 'COIN_ERR'
#define COIN_ERR 0xFFFF

If the original library author had paid attention to the warnings, they could have correctly caught the incompatibility in the code and fixed it before it reached our final application. This shows one of the reasons to aim for warning free code across multiple compilers.

Comparing the errors

To view the errors from each compiler you can use godbolt to compare the output.