When is a bool not a bool (compiler warning C4800)

2
votes

Consider this being compiled in MS Visual Studio 2005 (and probably others):

CPoint point1( 1, 2 );
CPoint point2( 3, 4 );
const bool point1And2Identical( point1 == point2 );            // C4800 warning
const bool point1And2TheSame( ( point1 == point2 ) == TRUE );  // no warning

What the...? Is the MSVC compiler brain-dead? As far as I can tell, TRUE is #defined as 1, without any type information. So by what magic is there any difference between these two lines? Surely the type of the expression inside the brackets is the same in both cases? [This part of the question now satisfactorily answered in the comments just below]

Personally, I think that avoiding the warning by using the == TRUE option is ugly (though less ugly than the != 0 alternative, despite being more strictly correct), and it is better to use #pragma warning( disable:4800 ) to imply "my code is good, the compiler is an ass". Agree?

Note - I have seen all manner of discussion on C4800 talking about assigning ints to bools, or casting a burger combo with large fries (hold the onions) to a bool, and wondering why there are strange results. I can't find a clear answer on what seems like a much simpler question... that might just shine line on C4800 in general.

3
visual-studiovisual-c++
Look for the declaration of operator== related to class CPoint. – aschepler
No - you look for it :-) I couldn't find any magic there at all - operator == returns a BOOL which is a typedef to an int. As far as I can tell, int == TRUE is still an int. – omatai
On built-in types, == results in a bool, not a BOOL. – aschepler
If you definitely want the operator == return BOOL, then you should declare point1And2Idential as BOOL too to avoid the warning. – Gang Yin
True - if you do the same with ints or doubles instead of CPoints, there are no warnings with either form. I suppose that == returns a bool on built-in types since to do anything else would contravene the C++ standard? That makes clear what is going on... but I still think I prefer the #pragma solution, or to change from a const bool assignment construct to an if (...) if possible. – omatai

3 Answers

3
votes

Have a look at this

Typically, this message is caused by assigning int variables to bool variables

TRUE and BOOL are ints, not bool.

You can also check if (point1 == point2) != 0 or use !!(point1 == point2)

EDIT

The !! operations simply converts the result to a bool, and since it is a double negative you will receive the same result.

There has been discussion on this before both here and here.

0
votes

Actually wouldn't you use ASSERT(point1 == point2)? The operator on the CPoint class is explicitly overloaded to return nonzero if the points are equal and 0 if they are not. The famed ASSERT macro kills me, but if you look at how the ASSERT macro expands it would be almost analogous to adding the == TRUE above in your code. Have a look at this link on msdn. Note the example code that explicitly states to not use the equality operator directly against two CPoint objects. I agree that your code appears most correct, but that is why we love C++. Ugh...

0
votes

The answer that now satisfies me is this:

On built-in types, operator == returns bool (as required by C++ standard).

For CPoint and many other of Microsoft's types, operator == returns Microsoft's BOOL, which is typedef'd to an int.

So... the first const bool assignment line tries to assign an int to a const bool, resulting in C4800.

The second const bool assignment line compares an int to the value TRUE, which is typedef'd as 1 (assumed to be an int by the compiler) and so the result of the int == int comparison is a bool. This gets assigned to the const bool without warning.

As for what looks nicest on screen, #pragma is ugly and unecessary. If you don't like the look of the extra != 0, then try this:

#define ISTRUE( x )  ( (x) != 0 )

const bool point1And3Identical( ISTRUE( point1 == point2 ) );    // no warning

Semantically and syntactically this seems like a better way to pretty up the code. You can quietly tuck the #define for ISTRUE() away into a header file so it looks even prettier.