Search code examples
cfor-loopintegerrelational-operators

For loop: signed / unsigned integer expression WARNING


I have a shape Rectangle which consists of the following parameters:

int16_t x;
int16_t y;
uint16_t w;
uint16_t h;

x and y are int16_t's because the rectangle can also start from a negative position

w and h are uint16_t's because the rectangle can only have a positive width and height. (In theory it could also have a negative width and height but in my program this is not possible, in such a case the coordinates x and y are decremented accordingly)

Now my actual problem: When painting the rectangle (pixel by pixel) I implemented the following loops:

for(int16_t y_counter = y; y_counter < y+h; y_counter++) {
    for(int16_t x_counter = x; x_counter < x+w; x_counter++) {
        // ... draw pixel
    }
}

The problem now is that I have the compiler warning -Wsign-compare enabled which gives me the following warning:

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

Now I am thinking what could be the most elegant (/most correct) solution to avoid this warning.

I came up with the following solution:

for(int16_t y_counter = y; y_counter < y+(int16_t)h; y_counter++) {
    for(int16_t x_counter = x; x_counter < x+(int16_t)w; x_counter++) {
        // ... draw pixel
    }
}

This eliminates the warning. But I have two concerns:

  • It doesn't look nice and therefore not very well readable
  • It is very unprobable but a rectangle could be starting at a negative coordinate and be larger then what a int16_t can hold. In such a case this would lead to an overflow

I know I could just ignore/disable this warning or leave it as it is but I was wondering if somebody came up with an elegant solution


Solution

  • It doesn't look nice and therefore not very well readable

    Nah it's fine. Explicit casts are easy to read. It's probably a worse offense to readability to use a verbose name for the loop iterator, you could as well have named them i and j since the "counter" part doesn't really add any valuable information. This is perfectly readable:

    for(int16_t i=y; i < y + (int16_t)h; i++) {
        for(int16_t j=x; j < x + (int16_t)w; j++) {
            // ... draw pixel (i, j)
        }
    }
    

    Or if you are overly pedantic about keeping expressions simple:

    int16_t y_limit = y + (int16_t)h;
    int16_t x_limit = x + (int16_t)w
    
    for(int16_t i=y; i < y_limit; i++) {
        for(int16_t j=x; j < x_limit; j++) {
            // ... draw pixel (i, j)
        }
    }
    

    It is very unprobable but a rectangle could be starting at a negative coordinate and be larger then what a int16_t can hold. In such a case this would lead to an overflow

    If you worry about that, you probably shouldn't have been using small integer types to begin with. They do come with an implicit promotion to int on 32/64 bit system, which is unfortunate. Switching everything to int32_t would probably have been a sensible fix to that and also resolves the warning.