What is wrong with my For loops? i get warnings: comparison between signed and unsigned integer expressions [-Wsign-compare]

Question

#include <iostream>
#include <string>
#include <vector>
#include <sstream>


using namespace std;

int main() {

    vector<double> vector_double;
    vector<string> vector_string;
    ...


    while (cin >> sample_string)
        {
            ...
        }

    for(int i = 0; i <= vector_string.size(); i++)
        {
            ....
        }

    for (int i = 0; i < vector_double.size(); i++)
        ....


    return 0;
}

Answer 1

Why is there a warning with -Wsign-compare ?

As the name of the warning, and its text, imply, the issue is that you are comparing a signed and an unsigned integer. It is generally assumed that this is an accident.

In order to avoid this warning, you simply need to ensure that both operands of < (or any other comparison operator) are either both signed or both unsigned.

How could I do better ?

The idiomatic way of writing a for loop is to initialize both the counter and the limit in the first statement:

for (std::size_t i = 0, max = vec.size(); i != max; ++i)

This saves recomputing size() at each iteration.

You could also (and probably should) use iterators instead of indices:

for (auto it = vec.begin(), end = vec.end(); it != end; ++it)

auto here is a shorthand for std::vector<int>::iterator. Iterators work for any kind of containers, whereas indices limit you to C-arrays, deque and vector.

Answer 2

It is because the .size() function from the vector class is not of type int but of type vector::size_type

Use that or auto i = 0u and the messages should disappear.

Answer 3

I usually solve it like this:

for(int i = 0; i <= (int)vector_string.size(); i++)

I use the C-style cast because it's shorter and more readable than the C++ static_cast<int>(), and accomplishes the same thing.

There's a potential for overflow here, but only if your vector size is larger than the largest int, typically 2147483647. I've never in my life had a vector that large. If there's even a remote possibility of using a larger vector, one of the answers suggesting size_type would be more appropriate.

I don't worry about calling size() repeatedly in the loop, since it's likely an inline access to a member variable that introduces no overhead.

Answer 4

You get this warning because the size of a container in C++ is an unsigned type and mixing signed/unsigned types is dangerous.

What I do normally is

for (int i=0,n=v.size(); i<n; i++)
    ....

this is in my opinion the best way to use indexes because using an unsigned type for an index (or the size of a container) is a logical mistake.

Unsigned types should be used only when you care about the bit representation and when you are going to use the modulo-(2**n) behavior on overflow. Using unsigned types just because a value is never negative is a nonsense.

A typical bug of using unsigned types for sizes or indexes is for example

// Draw all lines between adjacent points
for (size_t i=0; i<pts.size()-1; i++)
    drawLine(pts[i], pts[i+1]);

the above code is UB when the point array is empty because in C++ 0u-1 is a huge positive number.

The reason for which C++ uses an unsigned type for size of containers is because of an historical heritage from 16-bit computers (and IMO given C++ semantic with unsigned types it was the wrong choice even back then).

Answer 5

int is signed by default - it is equivalent to writing signed int. The reason you get a warning is because size() returns a vector::size_type which is more than likely unsigned.

This has potential danger since signed int and unsigned int hold different ranges of values. signed int can hold values between –2147483648 to 2147483647 while an unsigned int can hold values between 0 to 4294967295 (assuming int is 32 bits).

Answer 6

Your variable i is an integer while the size member function of vector which returns an Allocator::size_type is most likely returning a size_t, which is almost always implemented as an unsigned int of some size.

Answer 7

Make your int i as size_type i.
std::vector::size() will return size_type which is an unsigned int as size cannot be -ve.
The warning is obviously because you are comparing signed integer with unsigned integer.

Answer 8

Answering after so many answers, but no one noted the loop end.. So, here's my full answer:

1. To remove the warning, change the i's type to be unsigned, auto (for C++11), or std::vector< your_type >::size_type
2. Your for loops will seg-fault, if you use this i as index - you must loop from 0 to size-1, inclusive. So, change it to be
   for( std::vector< your_type >::size_type i = 0; i < vector_xxx.size(); ++i )
   (note the <, not <=; my advise is not to use <= with .begin() - 1, because you can have a 0 size vector and you will have issues with that :) ).
3. To make this more generic, as you're using a container and you're iterating through it, you can use iterators. This will make easier future change of the container type (if you don't need the exact position as number, of course). So, I would write it like this:

---

for( std::vector< your_type >::iterator iter = vector_XXX.begin(); 
     iter != vector_XXX.end(); 
     ++iter )
{
    //..
}

Answer 9

Declaring 'size_t i' for me work well.

Answer 10

std::cout << -1U << std::endl;
std::cout << (unsigned)-1 << std::endl;
4294967295

std::cout << 1 << std::endl;
std::cout << (signed)1 << std::endl;
1

std::cout << (unsigned short)-1 << std::endl;
65535

std::cout << (signed)-1U << std::endl;
std::cout << (signed)4294967295 << std::endl;
-1

unsign your index variable

unsigned int index;
index < vecArray.size() // size() would never be negative

Answer 11

Some answers are suggesting using auto, but that won't work as int is the default type deduced from integer literals. Pre c++23 you have to explicitly specify the type std::size_t defined in cstddef header

for(std::size_t i = 0; i <= vector_string.size(); i++)
{
    ....
}

In c++23 the integral literal zu was added, the motivation was indeed to allow the correct type to be deduced.

for(auto i = 0zu; i <= vector_string.size(); i++)
{
    ....
}

But unfortunately no compiler support this feature yet.