55

This question extends Why use !! when converting int to bool?.

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

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

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)

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

What's the deal, yo? Why is that a performance warning? I thought it'd be less efficient to 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?

bcsb1001
  • 2,834
  • 3
  • 24
  • 35
bobobobo
  • 64,917
  • 62
  • 258
  • 363

6 Answers6

46

There's a discussion on Microsoft Connect about this (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.

Community
  • 1
  • 1
Michael Burr
  • 333,147
  • 50
  • 533
  • 760
  • 3
    I too think this warning is useless, but then again I am biased agains MS VC ;-) – Gunther Piez Dec 04 '09 at 16:24
  • 5
    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 Dec 04 '09 at 17:40
  • 4
    Also `return this->parentNode != 0` is more error prone due to possible typo `return this->parentNode = 0`. – bobobobo Mar 06 '10 at 17:00
  • 11
    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 Oct 20 '10 at 13:04
  • 2
    it exists for histerical raisins. Some old non confromant compilers (likely VStudio v. < 5.0) were just casting to bool in boolean contexts, which was fast. But it is also dangerous and non-standard (cf my answer for why). So they changed it to the `!= 0` comparison, which is a slower OPCODE than the cast that were in place before in the code generator. So the guy who did that, also output a warning... to warn old code bases maintainer clients that something changed to slower with the new compiler. – v.oddou Dec 10 '13 at 05:27
  • 2
    If the warning is to catch bugs, then it is misworded. – Jim Balter Feb 14 '14 at 00:15
  • Even if the shown cases are bugs often enough to merit a warning, C4800 also warns when the conversion is done in a boolean conversion context, i.e. when `explicit operator bool` or the safe boolean conversion idiom is allowed to kick in. They went to the trouble to disable this warning for `!!exp`, they ought to go ahead and do the same for explicit conversion contexts. – bames53 May 05 '14 at 21:12
  • Note: This warning was removed with Visual Studio 2017 – Brandlingo Feb 07 '19 at 16:24
28

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
  • 348,265
  • 75
  • 913
  • 977
  • 1
    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 Mar 06 '10 at 17:02
  • 1
    I prefer to cast by `!!value`. It's shorter and more generic. – Yakov Galka Nov 25 '10 at 09:20
  • 1
    +1 Making the conversion explicit makes your intent obvious to the next programmer that has the read the code, and acknowledges the performance cost of the implicit conversion. – Adrian McCarthy May 16 '12 at 22:19
  • Does not answer the question: "Why is there a performance warning..."? – Jon Jun 23 '14 at 05:27
  • 1
    Adding static_cast(this->parentNode) makes your intent every bit as crystal clear, but that doesn't disable the warning "by design". That seems like a REALLY stupid design! – George Feb 25 '15 at 11:27
  • 1
    Some coding standards suggest to switch the operator parameters round NULL == object so the typo NULL = object will cause a compiler error. – Denise Skidmore Apr 21 '16 at 20:08
9

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
  • 10,954
  • 6
  • 44
  • 66
  • 9
    Oh emm gee. 1 clock cycle out of 3,000,000,000? That is too much. – bobobobo Sep 03 '10 at 11:48
  • 2
    In some uses, even 1 clock cycle matters. – quant_dev Feb 29 '16 at 22:34
  • The two are no different actually. The cycle count difference only applies if `parentNode` is a bool to begin with. Since it's an address, it's going to be the native word size, and you can't just compare the lower 8 bits and call it good. If it wasn't an address and was a bool, sure you can use a different instruction for comparing smaller operands and it might be a bit faster (by a few cycles, or pipeline better). – Michael J. Gray Jan 13 '17 at 23:34
9

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.

Gunther Piez
  • 29,760
  • 6
  • 71
  • 103
  • It's not because it's converting a pointer to a bool but rather because it's returning a bool. This same code gets generated if you're comparing two bools, but it uses different registers depending on optimization settings. – Michael J. Gray Jan 13 '17 at 23:40
2

It would be more efficient to write:

bool hasParent()
{
    return  this->parentNode != NULL;
}
Lukáš Lalinský
  • 40,587
  • 6
  • 104
  • 126
Martin York
  • 257,169
  • 86
  • 333
  • 562
1

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

return this->parentNode != 0;
UberJumper
  • 20,245
  • 19
  • 69
  • 87