Bug 13489 – Boolean semantics of floating point types should use "<> 0"

Floating point numbers can be implicitly cast to bool so they can be used as if() and assert() arguments. In this case NaN evaluates to true. I'm working at Sociomantic where this behaviour didn't reveal a bug and caused us to attempt to spend a monetary amount of $9223372036854.775808: --- import stdc.math: lround, fabs; ulong calcAmount ( double x ) in { assert(x); // succeeds for NaN } body { // do calculation with x return lround(fabs(x)); // returns 9223372036854775808 if x is NaN } --- It happens because currently for a floating point variable x "cast(bool)x" behaves like "(x != 0)". I suggest changing that to "(x <> 0)". Rationale: The difference between "!=" and "<>" is subtle, many programmers aren't aware of it, and even the ones who are do this mistake easily, as it happened to me. "x != 0" is appropriate for integer and pointer types. For floating point types one has always to respect NaN, and "!=" doesn't do that so, unless NaN is anticipated, one should use "<>" instead. This applies in most use cases and, since it is the initial floating point value, NaN is encountered mostly caused by a bug of using a variable without assigning it before. This very notorious bug should be detected by "assert(x)", and I'd be surprised if most existing code that does "if (x)" (or otherwise uses floating point values as boolean) works as it should with NaN.

Also important to note is how easy a refactoring could produce invalid code. If you use integers in your code and if statements, and you decide to switch to floating-point (say to represent a fraction of a second for a time variable) you can easily end up with faulty code like that.

Consider this C++ code: bool foo(long double a) { return a; } DMC++ generates: fld tbyte ptr 4[ESP] fldz fucompp ST(1),ST fstsw AX sahf jne L13 jp L13 xor EAX,EAX jmp short L18 L13: mov EAX,1 L18: ret and g++ generates: fld tbyte ptr 8[RSP] mov EAX,1 fldz fucomip fstp ST setp DL cmovz EAX,DL ret In other words, both of them treat NaN as "TRUE". I fear that if we deviate from this behavior, we'll get subtly broken code that is written by former C++ programmers or that is transliterated from C++. Having cast(bool)d yield different results than d!=0 to me is very surprising behavior. Having cast(bool)d rewritten to be d<>0 is also problematic as Don Clugston is a vocal advocate of having the <> operator removed from D. Issuing a warning or error for if(d) can be done, but then the user simply rewrites it as if(d!=0) and I'm not sure if anything was accomplished. So I'm not sure what the right answer would be. I do strongly suggest that calculations that return money values have sanity checks in them for reasonable dollar values; not just 0, Infinity or NaN checks. An amount of 9 trillion dollars shouldn't have gotten far. Such checks can find a great many more mistakes than NaNs would. Another possibility is to not use lround(), i.e. make your own lround that asserts that its argument is not NaN. Note that lround() in D just defers to the C one, which is underspecified as to what happens with NaN or Infinity arguments. I would not rely on C's lround() for financial calculations without first checking its argument.

(In reply to Walter Bright from comment #2) > Having cast(bool)d yield different results than d!=0 to me is very > surprising behavior. for me too. NaN is clearly not 0, so it's 'true'. but maybe compiler should produce warning telling programmer that he is about to summon a dragon? i.e. for implicit converts to boolean, like `if (d)` and `assert(d)` compiler will tell that "implicit convertsion of double to boolean leads to undesired results, please use explicit cast(bool)". as for '!=' vs '<>'... i don't know. it's easy to emit a warning, but i can't think out sane method to hush the compiler if "!=" is what i really want.

The current D situation seems standard: http://stackoverflow.com/questions/9158567/nan-to-bool-conversion-true-or-false http://stackoverflow.com/questions/15686318/why-do-not-a-number-values-equal-true-when-cast-as-boolean-in-python-numpy

What the relation of signaling NaN and this. Will `if (nan)` raise a signal? Maybe a runtime check is enough. It's clear that the code that raise this problem is buggy, no argue about that, but how to fix it is not the topic of this issue, the topic is trying to find a way to make the bug impossible to even exist, with help from the language :)

(In reply to Leandro Lucarella from comment #5) > What the relation of signaling NaN and this. Will `if (nan)` raise a signal? Or any following use of nan afterwards, like `lround(fabs(x))`. f the > Maybe a runtime check is enough. > > It's clear that the code that raise this problem is buggy, no argue about > that, but how to fix it is not the topic of this issue, the topic is trying > to find a way to make the bug impossible to even exist, with help from the > language :)

(In reply to Walter Bright from comment #2) > Having cast(bool)d rewritten to be d<>0 is also problematic as Don Clugston > is a vocal advocate of having the <> operator removed from D. This is not problematic, the compiler using it internally does not mean it has to be exposed to the user.

> I do strongly suggest that calculations that return money values have sanity checks in them for reasonable dollar values; not just 0, Infinity or NaN checks. Absolutely. It was just an impressing situation where NaN caught me. I don't blame D for not catching this particular bug, and I'm aware there are alternative ways of detecting this bug. My point is: NaN was intentionally chosen as the initial floating point value to detect the common bug of forgetting to initialize a variable (http://dlang.org/faq.html#nan). The standard way to detect bugs is a plain assert(). So assert() should fail with NaN. I suggest the boolean behaviour of <>0 because believe it makes more sense in general, is compatible to !=0 for integers and pointers and would make assert() fail with NaN. > Having cast(bool)d yield different results than d!=0 to me is very > surprising behavior. I agree it can be considered surprising, and that's an essential point of this debate. But at least if d is a negative 0, cast(bool)d is false even though the binary data in d contain a set bit (i.e., if d is a double, cast(bool)*cast(ulong*)&d is true). This can be considered surprising, too. On the other hand, by its nature NaN causes many surprises so I believe we can live with a false boolean result if we have the benefit of catching a well known bug. (PS. Amounts like 80,833,756,980 Vietnamese Đồng do happen so limiting them is far from trivial...)

(In reply to David Eckardt from comment #8) > My point is: NaN was intentionally chosen as the initial floating point > value to detect the common bug of forgetting to initialize a variable > (http://dlang.org/faq.html#nan). The standard way to detect bugs is a plain > assert(). So assert() should fail with NaN. Yeah, this is the key, NaN should behave the same as null, it should explode as soon as it's used. > (PS. Amounts like 80,833,756,980 Vietnamese Đồng do happen so limiting them > is far from trivial...) Yeah, you should have to check the currency and maintain a table of conversion rates. Not a very promising approach ;-)

(In reply to Leandro Lucarella from comment #9) > (In reply to David Eckardt from comment #8) > > My point is: NaN was intentionally chosen as the initial floating point > > value to detect the common bug of forgetting to initialize a variable > > (http://dlang.org/faq.html#nan). The standard way to detect bugs is a plain > > assert(). So assert() should fail with NaN. > > Yeah, this is the key, NaN should behave the same as null, it should explode > as soon as it's used. NaN is designed not to explode when used, but to propagate in that all operations on NaN should yield NaN as a result. Changing this would be not only a drastic change to the language, but would cause D to behave very differently from every other language that deals with NaNs. I propose instead that, here, the real problem is what lround() did with a NaN value - it returned garbage. I suggest that you ban use of lround() from your system and replace with one that asserts on NaN arguments. > > (PS. Amounts like 80,833,756,980 Vietnamese Đồng do happen so limiting them > > is far from trivial...) > > Yeah, you should have to check the currency and maintain a table of > conversion rates. Not a very promising approach ;-) That's 80 billion, far short of the 9 trillion in the original error. It is essential to have sanity checks on the results of critical calculations. D2 has also recently added core.checkedint with functions in it to do overflow checking addition, subtraction, multiplication and division. These are portable to D1. They replace ad hoc methods that are often misguided or simply wrong.

>> > (PS. Amounts like 80,833,756,980 Vietnamese Đồng do happen so limiting them >> > is far from trivial...) >> >> Yeah, you should have to check the currency and maintain a table of >> conversion rates. Not a very promising approach ;-) >That's 80 billion, far short of the 9 trillion in the original error. It is >essential to have sanity checks on the results of critical calculations. FWIW we advertise some real estate in the €20 million price range. That is 0.4 trillion Đồng. I don't see any reason why the Đồng couldn't collapse even further, and push it into the trillions. We have a long history of our sanity checks being incorrect - the real world is a crazy place! ---- But, back on topic -- I think it is debatable whether assert(NaN) should be false (as originally proposed here), or if it should fail to compile. But I am certain that the current behaviour, that assert(NaN) passes, is wrong. Pretty much all existing code containg assert(f); except where f is known at compile time, is broken code. And this is the point -- it's a place in the language where something that looks completely innocent, is actually a landmine. We can either make it have the behaviour that the author clearly intended, (ie, assert( !(f == 0) ); <--- Look Mum, no NCEG operators! ) or we could statically disallow implicit conversion from float to bool unless we can prove that the float isn't NaN. I mean, is NaN true, or is it false? The answer is NO, it isn't! So a sensible mapping from IEEE float to bool is not possible.

(In reply to Don from comment #11) > But I am certain that the current behaviour, that assert(NaN) passes, is > wrong. Agreed. It was my understanding anyways that anything that operates on NaN produces "false" anyways. I'd expect *both*: assert( NaN); assert(!NaN; to fail, actually, since the fist one is "Are you non 0? No" and the second is "Are you 0? No."

(In reply to monarchdodra from comment #12) > (In reply to Don from comment #11) > > But I am certain that the current behaviour, that assert(NaN) passes, is > > wrong. > > Agreed. It was my understanding anyways that anything that operates on NaN > produces "false" anyways. > > I'd expect *both*: > assert( NaN); > assert(!NaN; > > to fail, actually, since the fist one is "Are you non 0? No" and the second > is "Are you 0? No." I agree with this. One additional thing to point out is that `assert` has already a precedent of being more than just check for 0 - it calls object invariant. Thus meaning of `assert(object)` is closer to "check if this thing is in usable state" and NaN is not a usable state by definition.

(In reply to David Eckardt from comment #0) > ulong calcAmount ( double x ) > in > { > assert(x); // succeeds for NaN > } > body > { > // do calculation with x > return lround(fabs(x)); // returns 9223372036854775808 if x is NaN > } > --- This looks to me like a problem with either lround or fabs, rather than with the boolean semantics of a floating point. Still, I agree that the boolean semantics are an issue. (In reply to Walter Bright from comment #2) > In other words, both of them treat NaN as "TRUE". I fear that if we deviate > from this behavior, we'll get subtly broken code that is written by former > C++ programmers or that is transliterated from C++. Can you give an example of how real-world code might explicitly rely on NaN being boolean true? (In reply to bearophile_hugs from comment #4) > The current D situation seems standard: > > http://stackoverflow.com/questions/9158567/nan-to-bool-conversion-true-or- > false > > http://stackoverflow.com/questions/15686318/why-do-not-a-number-values-equal- > true-when-cast-as-boolean-in-python-numpy The spec snippets pasted there don't mention NaN; as such, it looks to me like an oversight. Furthermore, I have just offered this insight on the C/C++ treatment: But does NaN constitute a value at all? It's similar to NULL in typical database systems. I was explicitly taught that NULL is not a value, but the absence of a value. Unless there's evidence to the contrary, I think we can claim the same about NaN. As such, I argue that this leaves the behaviour of convert NaN to a bool undefined, unless there's something elsewhere in the spec that covers the issue.

@Geod24 created dlang/dmd pull request #10363 "Fix issue 13489: Boolean semantics of floating point types should handle float" fixing this issue: - Fix issue 13489: Boolean semantics of floating point types should handle float https://github.com/dlang/dmd/pull/10363

Walter rejected the PR made to fix this. Closing as "WONTFIX".