views:

495

answers:

7

Extends.

I thought I was being cool when I did something like:

bool hasParent()
{
  return this->parentNode ;
}

Even with a (bool) cast, the warning still doesn't go away.

Where this->parentNode is NULL when there is no parent node.

But I'm getting:

warning C4800: 'Node *' : forcing value to bool 'true' or 'false' (performance warning)

What's the deal, yo? Why is that a performance warning? I thought it'd be more efficient to not write something like:


bool hasParent()
{
  if( this->parentNode )
    return true ;
  else
    return false ;
}

But the second version generates no warnings and the compiler seems a lot happier. Which is faster though?

+3  A: 

It would be more efficient to write:

bool hasParent()
{
    return  this->parentNode != NULL;
}
Martin York
Wrong, the compiler will generate the same code as for the original.
user9876
user9876, who said it would generate more efficient code? It saves typing.
Georg Fritzsche
The writing part is epsilon more efficient. The executing part is epsilon squared more efficient. Calling this a performance warning is just part of the general penny-wise-pound-foolish idiocy about performance.
Mike Dunlavey
I meant in compared to the if {} else {} block. This also removes the warning.
Martin York
@Martin: I know you did. I was just flaming about the original compiler warning. Sorry.
Mike Dunlavey
A: 

I'm pretty sure this is compiler dependent

A: 

Realistically i think they would optimize to the same, you can also try doing this:

return this->parentNode != 0;
UberJumper
+11  A: 

The fact that casting to bool does not make the warning go away is by design:

Casting the expression to type bool will not disable the warning, which is by design.

I would recommend the approach that the MSDN description of warning C4800 recommends:

return this->parentNode != NULL;

this makes it clear that you are returning true if parentNode is not a null pointer and false if parentNode is a null pointer.

James McNellis
+1 for reasons.
Georg Fritzsche
Good answer! From MS standpoint. I'd like to express that I disagree.. in my if statements etc I never check `if( object == NULL )` because of the possibility of typing `if( object = NULL )` and creating a bug this way. I check `if( object )` and `if( !object )` for precisely this reason. `#pragma warning (disable:4800)`, I now write.
bobobobo
A: 

Why is that a performance warning?

The compiler is turning this:

bool hasParent()
{
  return this->parentNode;
}

into:

bool hasParent()
{
  return this->parentNode != 0;
}

This takes about one clock cycle more than you might expect from looking at the code. It's an insignificant performance difference.

I think it's better to write out the != 0 explicitly anyway, as it makes the code clearer as well as silencing the warning.

user9876
Oh emm gee. 1 clock cycle out of 3,000,000,000? That is too much.
bobobobo
+8  A: 

There's a discussion on Microsoft Connect about this (http://stackoverflow.com/questions/206564/what-is-the-performance-implication-of-converting-to-bool-in-c). The example given to Microsoft is:

$ cat -n t.cpp && cl -c -W3 -O2 -nologo -Fa t.cpp
1 bool f1 (int i)
2 {
3 return i & 2;
4 }
5
6 bool f2 (int i)
7 {
8 const bool b = i & 2;
9 return b;
10 }
11
12 bool f3 (int i)
13 {
14 const bool b = 0 != (i & 2);
15 return b;
16 }
t.cpp
t.cpp(3) : warning C4800: 'int' : forcing value to bool 'true' or 'false' (performance warning)
t.cpp(8) : warning C4800: 'int' : forcing value to bool 'true' or 'false' (performance warning)

And Microsoft's response (from the developer responsible for the warning) is:

This warning is surprisingly helpful, and found a bug in my code just yesterday. I think Martin is taking "performance warning" out of context.

It's not about the generated code, it's about whether or not the programmer has signalled an intent to change a value from int to bool. There is a penalty for that, and the user has the choice to use "int" instead of "bool" consistently (or more likely vice versa) to avoid the "boolifying" codegen. The warning is suppressed in the third case below because he's clearly signalled his intent to accept the int->bool transition.

It is an old warning, and may have outlived its purpose, but it's behaving as designed here

So basically the MS developer seems to be saying that if you want to 'cast' an int to bool you should more properly do it by using "return this->parentNode != 0" instead of an implicit or explicit cast.

Personally, I'd be interested to know more about what kind of bugs the warning uncovers. I'd think that this warning wouldn't have a whole lot of value.

Michael Burr
I too think this warning is useless, but then again I am biased agains MS VC ;-)
drhirsch
I think there should be a compiler flag for enabling those useless warnings. This and "most of C and some of C++ standard library is deprecated" warnings have always created an urge in me simply to disable warnings at all when using VC.
UncleBens
Also `return this->parentNode != 0` is more error prone due to possible typo `return this->parentNode = 0`.
bobobobo
bobobobo: Any sane compiler (at least gcc) will warn on that. And if you use "const" correctly, hasParent() will be a const function so it would be a compile error.
user9876
+1  A: 

The compiler needs to generate additional code for converting a pointer to bool. It is basically a comparision against zero and setting the result to one if not zero.

00000000004005e0 <_Z4testPv>:
bool test(void* adr) {
  4005e0:       48 85 ff                test   %rdi,%rdi
  4005e3:       0f 95 c0                setne  %al
    return adr;
}
  4005f8:       c3                      retq

This isn't directly visible from the source, so the compiler thinks this is something the user should be warned about.

drhirsch