Riddle

Normally, I try to avoid C++ bashing, so trendy nowadays. It is what it is, but we’ve to use it anyway. Today, however I encountered a funny bug that made me wonder a little bit. Consider the following snippet:

void Lol(float x, bool b)
{
    const float y = x + b ? 1.f : 0.f;
    rde::Console::Printf("Y = %f\n", y);
}
...
Lol(5.f, true);

What will be printed to the screen? Answer is quite simple – 1.0. It’s an obvious programmer’s mistake, but what puzzles me is that MSVC compiles it at /W4 without single warning that something might be fishy. At the same time this piece of code:

bool Cat(int x)
{ 
    return x;
}

Will result in the following: warning C4800: ‘int’ : forcing value to bool ‘true’ or ‘false’ (performance warning)...

I guess it was more of a rant about warnings in MSVC than C++ itself.

Old comments

Gynvael Coldwind 2011-06-03 09:04:05

Hey ;)
OK, I admit I don’t really understand the riddle.
I mean, in my opinion there should be no warning in the first place anyway.
Please note that what’s happening there (i.e. on x+b) is an upgrade of type bool (which according to the standard has the lowest integer rank) to type float. This specific conversion is even covered in the standard - quote from draft N3242 (point “4.9 Floating-integral conversion”):
“If the source type is bool, the value
false is converted to zero and the value true is converted to one.”
So imo it’s quite obvious that x+b is x+(float)b, so the result in the given example is 6.0f. The x+b is wrapped in a conditional statement being x+b?1.f:0.f, so the float part is casted back to bool (see point “5.16 Conditional operator” of the said draft) which is redirected from “4.9 Floating-integral conversions”:
“[ Note: If the destination type is bool, see 4.12. â??end note ]”
, to “4.12 Boolean conversion” of the said draft:
“A prvalue of arithmetic […] can be converted to a
prvalue of type bool. A zero value […] is converted to false;
any other value is converted to true.”
So this is once again fully defined by the standard.
And of course 6.0f is converter to true, so teh final value of y is set to 1.f due to the conditional operator parameters.
So, I repeat the question - why to expect a warning here in any compiler?
That being said I understand that you’re pointing out an inconsistency here - that in the first case a boolfloat conversion doesn’t generate a performance warning, while in the boolint case it does. However, from my understanding of the way the compiler works, the first part is greatly optimized anyway at code generation level: the last cast (float->bool which would cause the performance warning) won’t be present in the code, just a NZ/Z fpu test will be generated (due to conditional operator). Hence, the performance that is lost in the “Cat” function example due to [0->0,anything->1] conversion is not lost in the “Lol” function case. Hence, no warning.
Again, that being said I personally (as a security-related researcher) would like to see warnings on all implicit casts (and maybe even rank promotions) :)
Cheers!

admin 2011-06-03 12:50:18

Good point, you’re perfectly right. Honestly, I didn’t think about it too much and didn’t bother checking the standard. It’s just my gut feeling (even though I’m not a security researcher) after finding it was: “huh, shouldn’t compiler mention we’re casting back & forth here”? Apparently it shouldn’t…
The second point wasn’t really about the performance warning, I’m surprised you get a warning when implicitly converting int->bool, but there’s none when going float->bool, just seems awkward to me. As far as performance goes, generated assembly is pretty close (xor eax, eax + cmp mem, reg vs cmp mem, 0), but that’s another thing.

Mueller 2011-06-03 13:13:58

This is interesting and I could see how this would get overlooked. Reminds me of if (a = 0) instead of if (a == 0).
I would be curious if PC LINT would produce a error/warning message for your riddle?
http://www.gimpel.com/html/pcl.htm

Kristine 2011-06-03 14:32:38

There is talk on the clang developer mailing list of adding a compiler warning to this and similar cases.

admin 2011-06-03 14:32:51

@Mueller: No, it wouldn’t, we use Lint here. Then again, as Gynvael pointed out, it’s probably because it’s a perfectly legal code. IIRC MSVC will warn you about if (a = 0) at /W4.

Kristine 2011-06-03 14:35:18

But every warning a compiler produces is produced for perfectly legal code; otherwise it would be an error. The problem is just that C/C++ allows for constructs that are not desired in most cases (such as “if (a = 0)” or “int i = 0.5”) …

castano 2011-06-03 18:51:49

What a coincidence, we just caught the same bug using PVS-studio. I think it was the only useful warning it produced in our code base. It had never been detected, because it was inside of an assert, as a result it always returned true, which was the expected result anyway.

Maciekp 2011-06-04 12:13:01

I found such a bug in my code last month too :)

Liam 2011-06-09 12:58:36

I have not looked at the standard (no access to it at the moment) but IIRC it does not mention much on floating point promotion.
“So imo itâ??s quite obvious that x+b is x+(float)b, so the result in the given example is 6.0f”
I would not be surprised if the above was not the case and instead the types were both promoted to double instead of float. After a little googling you can see this is the case for C++0x from Pete Becker’s response in the C++ mailing list. http://groups.google.com/group/comp.std.c++/browse_thread/thread/06a2af7db8c0bc86?pli=1