“signed/unsigned mismatch” warnings (C4018) with decrementing for loop

Question

What should I do about the "signed/unsigned mismatch" warning in C++ code like this:

for (int i = vector.size() - 1; i >= 0; --i) // OK
{
    if (i < vector.size() / 2) // warning C4018: '<': signed/unsigned mismatch
        // ... 
}

(A contrived example, but it demonstrates the problem.)

How do I deal with "signed/unsigned mismatch" warnings (C4018)? says to use size_t for the loop variable, but this doesn't work for a decrementing loop terminating at 0. It compiles without warning, but the integer overflows (or is it underflows?) at runtime and becomes 4294967295.

There's nothing wrong with this code. Your understanding of its effect is correct. The warning is just some compiler writer's notion of good style, which you can ignore when you know more about your code than the compiler writer does. — Pete Becker, Aug 14 '16 at 19:13
@EMO: Try using `vector.crbegin()` for reverse iterating through the vector. — JVApen, Aug 14 '16 at 19:13
If you want high WTF/minute, you can do `for (unsigned int i = vector.size() - 1; i < vector.size(); --i)` :) — Adam Trhon, Aug 14 '16 at 19:15
Possible duplicate of [What's the best way to do a backwards loop in C/C#/C++?](http://stackoverflow.com/questions/275994/whats-the-best-way-to-do-a-backwards-loop-in-c-c-c) — MrTux, Aug 14 '16 at 20:06
MrTux, yes, there are some good answers there and I didn't find that question before posting mine, but it's unfortunate that the poster chose to mix C++ and C#, because the answers are different. C# doesn't suffer from this problem, since in .NET `Array.Length`, `List.Length`, etc. are signed integers - and it's probably to avoid exactly this kind of issue. — EM0, Aug 15 '16 at 19:24

AnT stands with Russia · Accepted Answer · 2016-08-14T21:25:42.423

Firstly, learn a few well-established idioms that'd allow you to write decrementing cycles using unsigned types. This is an absolutely essential skill in C and C++. E.g.

for ([some unsigned type] i = N; i-- > 0; )
{
  // Process i-th element
}

or

for ([some unsigned type] i = N; i > 0; )
{
  --i;
  // Process i-th element
}

(Note, that the initial value of i is N, not N - 1.)

Or you can even use a "more natural" approach

for ([some unsigned type] i = N - 1; i != -1; --i)
{
  // Process i-th element
}

Secondly, start using the proper unsigned type for vector indexing. In your case

for (auto i = vector.size() - 1; i != -1; --i)

or

for ([your vector type]::size_type i = vector.size() - 1; i != -1; --i)

That will solve your warning issue.

Alternatively, you can choose a "loser's way out" and just suppress the warning by using an explicit cast

if ((unsigned) i < vector.size() / 2)

or just do #pragma warning(disable: 4018).

score 3 · Answer 2 · edited May 23 '17 at 12:08

3

There are several options you could do:

If you use a vector use an iterator for traversing it:

for (auto it = vector.rbegin(); it != vector.rend(); ++it)

Or use a different for statement:

for (size_t i = vector.size(); i-- > 0; )

I also have seen for(size_t i = myArray.size() - 1; i != (size_t)-1; i--) (cf. What's the best way to do a backwards loop in C/C#/C++?)

edited May 23 '17 at 12:08

Community

1
1

answered Aug 14 '16 at 19:13

MrTux

32,350
30
109
146

score 1 · Answer 3 · answered Aug 14 '16 at 19:14

1

or alternatively:

for(size_t j = 0; j < vector.size(); j++){
    size_t i = vector.size() - 1 - j;
    ...
}

answered Aug 14 '16 at 19:14

ewcz

12,819
1
25
47

mike.dld · Answer 4 · 2016-08-14T19:13:04.430

0

Use the proper type (auto, std::size_t, std::vector<...>::size_type) and iterate from vector.size() down to 0 (and not from vector.size() - 1 down to -1) instead:

for (std::size_t i = vector.size(); i > 0; --i)
{
    if (i < vector.size() / 2 + 1)
        // ... 
}

edited Aug 14 '16 at 19:13

answered Aug 14 '16 at 19:12

mike.dld

2,929
20
21

1

Doing this you will skip vector[0] – Adam Trhon Aug 14 '16 at 19:13
@Dadam, no, and who said you'll use `vector[i]` and not `vector[i - 1]`?.. – mike.dld Aug 14 '16 at 19:15
2

Experience. Unless you make a reference to the item at the beginning of the block, you will forget the decrement at least one time – Adam Trhon Aug 14 '16 at 19:18
@Dadam, well, you're making baseless assumptions (since you don't know whether I have experience or not), and this is when there wasn't even a mention of indexing in OP's question... – mike.dld Aug 14 '16 at 19:21
1

Sorry, no offence: it is *my* experience that such small mistakes are done by everybody – Adam Trhon Aug 14 '16 at 19:24
Dadam's right. At the _very_ least, you need to mention this in the answer, or the OP may not be aware. – Lightness Races in Orbit Aug 14 '16 at 20:59
I have to agree with Dadam - while this technically works there is no way I could consider it a good practice. – EM0 Aug 15 '16 at 19:20

score 0 · Answer 5 · answered Aug 14 '16 at 21:51

Me I would write an index range-for adapter.

And write backwards, which adapts a range to iterate in reverse.

This gives us:

for(auto i:backwards(indexes_into(container))){

the result is less error prone, as efficient, and clearer intention at point of use (so long as you trust your utility code).

Solutions to both exist in boost, rangesv3, and various of my SO posts.

“signed/unsigned mismatch” warnings (C4018) with decrementing for loop

5 Answers5

Linked

Related