Is this an unavoidable signed and unsigned integer comparison?

Question

Probably not, but I can't think of a good solution. I'm no expert in C++ yet.

Recently I've converted a lot of ints to unsigned ints in a project. Basically everything that should never be negative is made unsigned. This removed a lot of these warnings by MinGW:

warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

I love it. It makes the program more robust and the code more descriptive. However, there is one place where they still occur. It looks like this:

unsigned int subroutine_point_size = settings->get<unsigned int>("subroutine_point_size");
...
for(int dx = -subroutine_point_size;dx <= subroutine_point_size;dx++) //Fill pixels in the point's radius.
{
    for(int dy = -subroutine_point_size;dy <= subroutine_point_size;dy++)
    {
        //Do something with dx and dy here.
    }
}

In this case I can't make dx and dy unsigned. They start out negative and depend on comparing which is lesser or greater.

I don't like to make subroutine_point_size signed either, though this is the lesser evil. It indicates a size of a kernel in a pass over an image, and the kernel size can't be negative (it's probably unwise for a user ever to set this kernel size to anything more than 100 but the settings file allows for numbers up to 2^32 - 1).

So it seems there is no way to cast any of the variables to fix this. Is there a way to get rid of this warning and solve this neatly?

We're using C++11, compiling with GCC for Windows, Mac and various Unix distributions.

Answer 1

Cast the variables to a long int or long long int type giving at the same time the range of unsigned int (0..2^32-1) and sign.

Answer 2

You're making a big mistake.

Basically you like the name "unsigned" and you intend it to mean "not negative" but this is not what is the semantic associated to the type.

Consider the statement:

adding a signed integer and an unsigned integer the result is unsigned

Clearly it makes no sense if you consider the term "unsigned" as "not negative", yet this is what the language does: adding -3 to the unsigned value 2 you will get a huge nonsense number instead of the correct answer -1.

Indeed the choice of using an unsigned type for the size of containers is a design mistake of C++, a mistake that is too late to fix now because of backward compatibility. By the way the reason it happened has nothing to do with "non-negativeness", but just with the ability to use the 16th bit when computers were that small (i.e. being able to use 65535 elements instead of 32767). Even back then I don't think the price of wrong semantic was worth the gain (if 32767 is not enough now then 65535 won't be enough quite soon anyway).

Do not repeat the same mistake in your programs... the name is irrelevant, what counts is the semantic and for unsigned in C and C++ it is "member of the Z_n modulo ring with n=2^k ".

You don't want the size of a container to be the member of a modulo ring. Do you?

Answer 3

Firstly, without knowing the range of values that will be stored in the variables, your claim that changing signed to unsigned variables is unsubstantiated - there are circumstances where that claim is false.

Second, the compiler is not issuing a warning only as a result of changing variables (and I assume calls of template functions like settings.get()) to be unsigned. It is warning about the fact you have expressions involving both signed and unsigned variables. Compilers typically issue warnings about such expressions because - in practice - they are more likely to indicate a programming error or to potentially involve some behaviour that the programmer may not have anticipated (e.g. instances of undefined behaviour, expressions where a negative result is expected but a large positive result is what will occur, etc).

A rule of thumb is that, if you need to have expressions involving both signed and unsigned types, you are better off making all the relevant variables signed. While there are exceptions where that rule of thumb isn't needed, you wouldn't have asked this question if you understood how to decide that.

On that basis, I suggest the most appropriate action is to unwind your changes.

Answer 4

Instead of the current

for(int dx = -subroutine_point_size;dx <= subroutine_point_size;dx++) //Fill pixels in the point's radius.

you can do this:

for(int dx = -int(subroutine_point_size);dx <= int(subroutine_point_size);dx++) //Fill pixels in the point's radius.

where the first int cast is ⁽¹⁾technically redundant, but is there for consistency, because the second cast removes the signed/unsigned warning that presumably is the issue here.

However, I strongly advise you to undo the work of converting signed to unsigned types everywhere. A good rule of thumb is to use signed types for numbers, and unsigned types for bit level stuff. That avoids the problems with wrap-around due to implicit conversions, where e.g. std:.string("Bah").length() < -5 is guaranteed (very silly), and because it does away with actual problems, it also reduces spurious warnings.

Note that you can just define a suitable name, where you want to indicate that some value will never be negative.

---

^{1) Technically redundant in practice, for two's complement representation of signed integers, with no trapping inserted by the compiler. As far as I know no extant C++ compiler behaves otherwise.}