views:

1935

answers:

11

The Microsoft C compiler warns when you try to compare two variables, and one is signed, and the other is unsigned. For example:

int a;    
unsigned b;

if ( a < b ) { // warning C4018: '&lt;' : signed/unsigned mismatch

}

Has this warning, in the history of the world, ever caught a real bug? Why's it there, anyway?

+3  A: 

You should change a and b to both use signed types, or both use unsigned types. But that may not be practical (e.g. it maybe outside your control).

The warning is there to catch comparisons between a signed integer with a negative value and an unsigned integer -- if the magnitudes of both numbers are small, the former will (incorrectly) be deemed larger than the latter.

j_random_hacker
+2  A: 

binary operators often convert both types to the same before doing the comparison, since one is unsigned, it will also convert the int to unsigned. Normally this won't cause too much trouble but if your int is a negative number, this will cause errors in comparisons.

e.g. -1 is equal to 4294967295 when converted from signed to unsigned, now compare that with 100 (unsigned)

yx
Actually, -1 != 2^32 in binary. That's only true for 32-bit integers.
Adam Hawes
+21  A: 

Oh it has. But the other way around. Ignoring that warning caused a huge headache to me one day. I was writing a function that plotted a graph, and mixed signed and unsigned variables. In one place, i compared a negative number to a unsigned one:

int32_t t; ...
uint32_t ut; ...

if(t < ut) { 
    ...
}

Guess what happened? The signed number got promoted to the unsigned type, and thus was greater at the end, even though it was below 0 originally. It took me a couple of hours until i found the bug.

Johannes Schaub - litb
The solution is far from trivial though.`if (t < 0 || unsigned(t) < ut) ` works, is readable, but takes two comparisons.
MSalters
`if (t < int(ut))` is another option.
dalle
+25  A: 

Never ignore compiler warnings.

Dev er dev
The issue is that I am turning on /Wall, which in the MS compiler spews out many things that don't really matter (I think gcc is better at this).
Steve Hanov
+2 if I could because this is really good advice.
j0rd4n
GCC's -Weffc++ is a pretty good set to ignore, actually. Or Visual C++ complaining that a conditional in a template function is "always true" or "always false" as a result of a template parameter.
Tom
Never ignore compilers. Change your code to remove them. Compiler warnings are there for a purpose, to protect you against possible errors. In this case, it could be a serious error.
gbrandt
MSVC used to emit a stack of warnings just because I included some parts of the STL. I think that's trained a lot of devs to disable warnings or ignore them, even though MS have since fixed it.
Adam Hawes
+2  A: 

The warnings are there for a purpose... They cause you to think hard about your code!

Personally, I would always explicitly cast the signed --> unsigned and unsigned --> signed if possible. By doing this you are ensuring that you take ownership of the transaction and that you know whats going to happen. I realise that it might not always be possible, depending on the project to do this, but always aim for 0 compiler warnings ... it can only help!

TK
A: 

Just one of the many ways in which C allows you to shoot yourself in the foot - you'd better know what you are doing. The C quote is attributed to Bjarne Stroustrup, creator of C++.

gimel
+3  A: 

If you have to ask the question, you do not know enough about whether it is safe to disable it, so the answer is No.

I wouldn't disable it - I don't assume I always know better than the compiler (not least because I often don't), and more particularly because I sometimes make mistakes by oversight when a compiler does not.

Jonathan Leffler
+1 for putting it so succinctly. There's a lot of compiler writers and only one of me. Their collective knowledge (and base of customers) is larger. The warning is there for a reason.
Adam Hawes
A: 

I've been writing code longer than I'd care to admit. From personal experience ignoring seemingly pedantic compiler warnings can sometimes yield very unpleasent results.

If they annoy you and you accept/understand the situation then set a cast and move on.

Eventually these things move from an overlooked nuance into a conscious decision when designing new code. The result leaves less room for mickmouse corner cases to ruin your or your customers day and overall better quality software.

Einstein
Explicit casting will remove the warning, but no matter which way you cast you're adding dragons to your code. If you need to, cast both to a larger signed type (if there is one) and then compare.
Adam Hawes
A: 

I have even configured the compiler to make that warning an compile error. for the reasons all the other guys already mentioned.

If I ever encounter a signed/unsigned mismatch I ask myself why I chose different "signedness". It usually is a design error.

EricSchaefer
A: 

@gimel The explanation about shoot the entire leg of found behind your link is really good for this problem.

-"Someone who avoids the simple problems may simply be heading for a not-so-simple one."

This is actually always true when you convert between different types and you don't check for those values that could hurt you.

/Johan

Update: The correct way to convert from uint to int is to check the values against limits.h, or something like that. (But I seldom do that my self, even thou I know I should... :-)

Johan
A: 

I think its best to convert your UNsigned number into a signed number (before the comparison). Rather than the other way around.

mike jones