Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Authoritative "correct" way to avoid signed-unsigned warnings when testing a loop variable against size_t

Tags:

c++

The code below generates a compiler warning:

private void test()
{
    byte buffer[100];
    for (int i = 0; i < sizeof(buffer); ++i)
    {
        buffer[i] = 0;
    }
}

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

This is because sizeof() returns a size_t, which is unsigned.

I have seen a number of suggestions for how to deal with this, but none with a preponderance of support and none with any convincing logic nor any references to support one approach as clearly "better." The most common suggestions seem to be:

  1. ignore the warnings
  2. turn off the warnings
  3. use a loop variable of type size_t
  4. use a loop variable of type size_t with tricks to avoid decrementing past zero
  5. cast size_of(buffer) to an int
  6. some extremely convoluted suggestions that I did not have the patience to follow because they involved unreadable code, generally involving vectors and/or iterators
  7. libraries that I cannot load in the AVR / ARM embedded environments I often use.
  8. free functions returning a valid int or long representing the byte count of T
  9. Don't use loops (gotta love that advice)

Is there a "correct" way to approach this?

-- Begin Edit --

The example I gave is, of course, trivial, and meant only to demonstrate the type mismatch warning that can occur in an indexing situation.

#3 is not necessarily the obviously correct answer because size_t carries special risks in a decrementing loop such as for (size_t i = myArray.size; i > 0; --i) (the array may someday have a size of zero).

#4 is a suggestion to deal with decrementing size_t indexes by including appropriate and necessary checks to avoid ever decrementing past zero. Since that makes the code harder to read, there are some cute shortcuts that are not particularly readable, hence my referring to them as "tricks."

#7 is a suggestion to use libraries that are not generalizable in the sense that they may not be available or appropriate in every setting.

#8 is a suggestion to keep the checks readable, but to hide them in a non-member method, sometimes referred to as a "free function."

#9 is a suggestion to use algorithms rather than loops. This was offered many times as a solution to the size_t indexing problem, and there were a lot of upvotes. I include it even though I can't use the stl library in most of my environments and would have to write the code myself.

-- End Edit--

I am hoping for evidence-based guidance or references as to best practices for handling something like this. Is there a "standard text" or a style guide somewhere that addresses the question? A defined approach that has been adopted/endorsed internally by a major tech company? An emulatable solution forthcoming in a new language release? If necessary, I would be satisfied with an unsupported public recommendation from a single widely recognized expert.

None of the options on offer seem very appealing. The warnings drown out other things I want to see. I don't want to miss signed/unsigned comparisons in places where it might matter. Decrementing a loop variable of type size_t with comparison >=0 results in an infinite loop from unsigned integer wraparound, and even if we protect against that with something like for (size_t i = sizeof(buffer); i-->0 ;), there are other issues with incrementing/decrementing/comparing to size_t variables. Testing against size_t - 1 will yield a large positive 'oops' number when size_t is unexpectedly zero (e.g. strlen(myEmptyString)). Casting an unsigned size_t to an integer is a container size problem (not guaranteed a value) and of course size_t could potentially be bigger than an int.

Given that my arrays are of known sizes well below Int_Max, it seems to me that casting size_t to a signed integer is the best of the bunch, but it makes me cringe a little bit. Especially if it has to be static_cast<int>. Easier to take if it's hidden in a function call with some size testing, but still...

Or perhaps there's a way to turn off the warnings, but just for loop comparisons?

like image 911
Craig.Feied Avatar asked Dec 23 '22 20:12

Craig.Feied


2 Answers

I find any of the three following approaches equally good.

  1. Use a variable of type int to store the size and compare the loop variable to it.

    byte buffer[100];
    int size = sizeof(buffer);
    for (int i = 0; i < size; ++i)
    {
        buffer[i] = 0;
    }
    
  2. Use size_t as the type of the loop variable.

    byte buffer[100];
    for (size_t i = 0; i < sizeof(buffer); ++i)
    {
        buffer[i] = 0;
    }
    
  3. Use a pointer.

    byte buffer[100];
    byte* end = buffer + sizeof(buffer)
    for (byte* p = buffer; p < end; ++p)
    {
        *p = 0;
    }
    

If you are able to use a C++11 compiler, you can also use a range for loop.

byte buffer[100];
for (byte& b : buffer)
{
    b = 0;
}
like image 197
R Sahu Avatar answered Dec 26 '22 10:12

R Sahu


The most appropriate solution will depend entirely on context. In the context of the code fragment in your question the most appropriate action is perhaps to have type-agreement - the third option in your bullet list. This is appropriate in this case because the usage of i throughout the code is only to index the array - in this case the use of int is inappropriate - or at least unnecessary.

On the other hand if i were an arithmetic object involved in some arithmetic expression that was itself signed, the int might be appropriate and a cast would be in order.

I would suggest that as a guideline, a solution that involves the fewest number of necessary type casts (explicit of implicit) is appropriate, or to look at it another way, the maximum possible type agreement. There is not one "authoritative" rule because the purpose and usage of the variables involved is semantically rather then syntactically dependent. In this case also as has been pointed out in other answers, newer language features supporting iteration may avoid this specific issue altogether.

To discuss the advice you say you have been given specifically:

  • ignore the warnings

Never a good idea - some will be genuine semantic errors or maintenance issues, and by teh time you have several hundred warnings you are ignoring, how will you spot the one warning that is and issue?

  • turn off the warnings

An even worse idea; the compiler is helping you to improve your code quality and reliability. Why would you disable that?

  • use a loop variable of type size_t

In this precise example, that is exactly why you should do; exact type agreement should always be the aim.

  • use a loop variable of type size_t with tricks to avoid decrementing past zero

This advice is irrelevant for the trivial example given. Moreover I presume that by "tricks" the adviser in fact means checks or just correct code. There is no need for "tricks" and the term is entirely ambiguous - who knows what the adviser means? It suggests something unconventional and a bit "dirty", when there is not need for any solution with such attributes.

  • cast size_of(buffer) to an int

This may be necessary if the usage of i warrants the use of int for correct semantics elsewhere in the code. The example in the question does not, so this would not be an appropriate solution in this case. Essentially if making i a size_t here causes type agreement warnings elsewhere that cannot themselves be resolved by universal type agreement for all operands in an expression, then a cast may be appropriate. The aim should be to achieve zero warnings an minimum type casts.

  • some extremely convoluted suggestions that I did not have the patience to follow, generally involving vectors and/or iterators

If you are not prepared to elaborate or even consider such advice, you'd have better omitted the "advice" from your question. The use of STL containers in any case is not always appropriate to a large segment of embedded targets in any case, excessive code size increase and non-deterministic heap management are reasons to avoid on many platforms and applications.

  • libraries that I cannot load in an embedded environment.

Not all embedded environments have equal constraints. The restriction is on your embedded environment, not by any means all embedded environments. However the "loading of libraries" to resolve or avoid type agreement issues seems like a sledgehammer to crack a nut.

  • free functions returning a valid int or long representing the byte count of T

It is not clear what that means. What id a "free function"? Is that just a non-member function? Such a function would internally necessarily have a type case, so what have you achieved other than hiding a type cast?

  • Don't use loops (gotta love that advice).

I doubt you needed to include that advice in your list. The problem is not in any case limited to loops; it is not because you are using a loop that you have the warning, it is because you have used < with mismatched types.

like image 30
Clifford Avatar answered Dec 26 '22 09:12

Clifford