munificent / craftinginterpreters

Repository for the book "Crafting Interpreters"
http://www.craftinginterpreters.com/
Other
9.02k stars 1.06k forks source link

Differences between jlox and clox when working with NaN values #269

Closed MinusKelvin closed 6 years ago

MinusKelvin commented 6 years ago

jlox and clox have different behavior when working with not a number values. This code:

var nan = 0.0/0.0;
print nan;
print nan == nan;
print nan != nan;
print nan > nan;
print nan < nan;
print nan >= nan;
print nan <= nan;

has this output in jlox:

NaN
true
false
false
false
false
false

and this output in clox:

-nan
true
false
false
false
true
true

Furthermore, the fact that NaN == NaN is true is surprising, as the IEEE standard defines NaN == NaN to be false.

jube commented 6 years ago

For <= and >=, in clox, they are compiled as !(a > b) and !(a < b) which is equivalent for integers but not for floats (because of all the special values like nan). So the result of nan <= nan is compiled as !(nan > nan) which is true. In jlox, the interpret use the operators directly so there is no problem.

munificent commented 6 years ago

So, there are a couple of things going on here:

  1. Double.equals() in Java does not honor IEEE 754. Double.NaN == Double.NaN evaluates to false, but Double.NaN.equals(Double.NaN) evaluates to true. jlox uses the latter.

  2. With NaN-tagging enabled, clox just does a straight bit comparison of the two values. Since NaNs have the same bit representation, that returns true.

  3. Like @jube notes, >= and <= desugar to negating < and >.

I could fix 1 by special-casing doubles, but it's annoying. Same for 2. I'd prefer to not change 3 because (1) I already wrote that chapter and, (2) I think it's worth showing desugaring as a technique.

So my current inclination is to just add an aside noting that Lox doesn't fully do the right thing for NaNs and use it as an excuse to talk about how subtle compatibility issues can be. What do you think?

jube commented 6 years ago

I think that's perfectly fine as people interested in IEEE compliance will know how to modify the c interpreter and others won't care.

MinusKelvin commented 6 years ago

The aside is a good idea, especially since language design can have a lot of corner cases most people don't think about that subtly break things (such as upcasting an Integer to a String with Java generics) and it would be good to draw attention to that.