How do I deal with "signed/unsigned mismatch" warnings (C4018)?

Question

I work with a lot of calculation code written in c++ with high-performance and low memory overhead in mind. It uses STL containers (mostly std::vector) a lot, and iterates over that containers almost in every single function.

The iterating code looks like this:

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

But it produces the signed/unsigned mismatch warning (C4018 in Visual Studio).

Replacing int with some unsigned type is a problem because we frequently use OpenMP pragmas, and it requires the counter to be int.

I'm about to suppress the (hundreds of) warnings, but I'm afraid I've missed some elegant solution to the problem.

On iterators. I think iterators are great when applied in appropriate places. The code I'm working with will never change random-access containers into std::list or something (so iterating with int i is already container agnostic), and will always need the current index. And all the additional code you need to type (iterator itself and the index) just complicates matters and obfuscates the simplicity of the underlying code.

Answer 1

It's all in your things.size() type. It isn't int, but size_t (it exists in C++, not in C) which equals to some "usual" unsigned type, i.e. unsigned int for x86_32.

Operator "less" (<) cannot be applied to two operands of different sign. There's just no such opcodes, and standard doesn't specify, whether compiler can make implicit sign conversion. So it just treats signed number as unsigned and emits that warning.

It would be correct to write it like

for (size_t i = 0; i < things.size(); ++i) { /**/ }

or even faster

for (size_t i = 0, ilen = things.size(); i < ilen; ++i) { /**/ }

Answer 2

Ideally, I would use a construct like this instead:

for (std::vector<your_type>::const_iterator i = things.begin(); i != things.end(); ++i)
{
  // if you ever need the distance, you may call std::distance
  // it won't cause any overhead because the compiler will likely optimize the call
  size_t distance = std::distance(things.begin(), i);
}

This a has the neat advantage that your code suddenly becomes container agnostic.

And regarding your problem, if some library you use requires you to use int where an unsigned int would better fit, their API is messy. Anyway, if you are sure that those int are always positive, you may just do:

int int_distance = static_cast<int>(distance);

Which will specify clearly your intent to the compiler: it won't bug you with warnings anymore.

Answer 3

If you can't/won't use iterators and if you can't/won't use std::size_t for the loop index, make a .size() to int conversion function that documents the assumption and does the conversion explicitly to silence the compiler warning.

#include <cassert>
#include <cstddef>
#include <limits>

// When using int loop indexes, use size_as_int(container) instead of
// container.size() in order to document the inherent assumption that the size
// of the container can be represented by an int.
template <typename ContainerType>
/* constexpr */ int size_as_int(const ContainerType &c) {
    const auto size = c.size();  // if no auto, use `typename ContainerType::size_type`
    assert(size <= static_cast<std::size_t>(std::numeric_limits<int>::max()));
    return static_cast<int>(size);
}

Then you write your loops like this:

for (int i = 0; i < size_as_int(things); ++i) { ... }

The instantiation of this function template will almost certainly be inlined. In debug builds, the assumption will be checked. In release builds, it won't be and the code will be as fast as if you called size() directly. Neither version will produce a compiler warning, and it's only a slight modification to the idiomatic loop.

If you want to catch assumption failures in the release version as well, you can replace the assertion with an if statement that throws something like std::out_of_range("container size exceeds range of int").

Note that this solves both the signed/unsigned comparison as well as the potential sizeof(int) != sizeof(Container::size_type) problem. You can leave all your warnings enabled and use them to catch real bugs in other parts of your code.

Answer 4

C++20 has now std::cmp_less

In c++20, we have the standard constexpr functions

std::cmp_equal
std::cmp_not_equal
std::cmp_less
std::cmp_greater
std::cmp_less_equal
std::cmp_greater_equal

added in the <utility> header, exactly for this kind of scenarios.

Compare the values of two integers t and u. Unlike builtin comparison operators, negative signed integers always compare less than (and not equal to) unsigned integers: the comparison is safe against lossy integer conversion.

That means, if (due to some wired reasons) one must use the i as integer, the loops, and needs to compare with the unsigned integer, that can be done:

#include <utility> // std::cmp_less

for (int i = 0; std::cmp_less(i, things.size()); ++i)
{
    // ...
}

---

This also covers the case, if we mistakenly static_cast the -1 (i.e. int)to unsigned int. That means, the following will not give you an error:

static_assert(1u < -1);

But the usage of std::cmp_less will

static_assert(std::cmp_less(1u, -1)); // error

Answer 5

You can use:

1. size_t type, to remove warning messages
2. iterators + distance (like are first hint)
3. only iterators
4. function object

For example:

// simple class who output his value
class ConsoleOutput
{
public:
  ConsoleOutput(int value):m_value(value) { }
  int Value() const { return m_value; }
private:
  int m_value;
};

// functional object
class Predicat
{
public:
  void operator()(ConsoleOutput const& item)
  {
    std::cout << item.Value() << std::endl;
  }
};

void main()
{
  // fill list
  std::vector<ConsoleOutput> list;
  list.push_back(ConsoleOutput(1));
  list.push_back(ConsoleOutput(8));

  // 1) using size_t
  for (size_t i = 0; i < list.size(); ++i)
  {
    std::cout << list.at(i).Value() << std::endl;
  }

  // 2) iterators + distance, for std::distance only non const iterators
  std::vector<ConsoleOutput>::iterator itDistance = list.begin(), endDistance = list.end();
  for ( ; itDistance != endDistance; ++itDistance)
  {
    // int or size_t
    int const position = static_cast<int>(std::distance(list.begin(), itDistance));
    std::cout << list.at(position).Value() << std::endl;
  }

  // 3) iterators
  std::vector<ConsoleOutput>::const_iterator it = list.begin(), end = list.end();
  for ( ; it != end; ++it)
  {
    std::cout << (*it).Value() << std::endl;
  }
  // 4) functional objects
  std::for_each(list.begin(), list.end(), Predicat());
}

Answer 6

I can also propose following solution for C++11.

for (auto p = 0U; p < sys.size(); p++) {

}

(C++ is not smart enough for auto p = 0, so I have to put p = 0U....)

Answer 7

I will give you a better idea

for(decltype(things.size()) i = 0; i < things.size(); i++){
                   //...
}

decltype is

Inspects the declared type of an entity or the type and value category of an expression.

So, It deduces type of things.size() and i will be a type as same as things.size(). So, i < things.size() will be executed without any warning

Answer 8

I had a similar problem. Using size_t was not working. I tried the other one which worked for me. (as below)

for(int i = things.size()-1;i>=0;i--)
{
 //...
}

Answer 9

I would just do

int pnSize = primeNumber.size();
for (int i = 0; i < pnSize; i++)
    cout << primeNumber[i] << ' ';