[Issue 13499] (float|double|real).compare functions improperly compare nan

[d-bugmail at puremagic.com]

https://issues.dlang.org/show_bug.cgi?id=13499

Andrei Alexandrescu <andrei at erdani.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrei at erdani.com

--- Comment #4 from Andrei Alexandrescu <andrei at erdani.com> ---
Recent work (https://github.com/dlang/druntime/pull/3208) brought this up
again. Per https://run.dlang.io/is/kCnvwa, all of these asserts currently pass:

void main() {
    import std;
    float[] a = [float.nan], b = [1];
    assert(!(a < b));
    assert(__cmp(a, b) == 0);
    assert(typeid(float[]).compare(&a, &b) < 0); // should be == 0
    assert(!(a > b));
    assert(__cmp(b, a) == 0);
    assert(typeid(float[]).compare(&b, &a) > 0); // should be == 0
}

As per Steve's original report, the same problem exists for individual floating
point numbers, not just arrays thereof.

I speculate the following historical development.

When typeid was introduced, the main client was the built-in dictionary.
(Another client: the now defunct built-in sort.) The dictionary used a
hashtable using a search tree as a collision list. The hashtable used
comparison for equality, but the search tree also needed comparison for
ordering. For dictionaries keyed on FP numbers, it made sense to see NaN as
smaller than any number so as to make it behave somewhat properly. The idea
would be to make sure there are no two NaNs stored in the same hashtable.

After a while, the built-in dictionary was changed to use simple lists for
collision resolutions, so as of right now there is no use of TypeInfo.compare
in dictionaries (and I think in druntime/phobos as a whole).

I should add that duplicate NaNs in a dictionary do occur today. Either that
was never paid attention to, or it ceased to work at some point. Check out :

void main() {
    import std;
    string[double] t;
        t[double.nan] = "nan";
    t[1] = "one";
        t[double.nan] = "nan2";
    writeln(t);
}

You will awesomely see two mappings for NaN.

PROPOSED RESOLUTION:

1. Change typeid().compare() to behave in perfect keeping with the built-in
comparison. This is a breaking change so we shouldn't approach it lightly.
However there is something to be said about the rule of least astonishment and
it is indeed surprising that direct comparison is subtly different from
comparison via runtime information.

2. Fix built-in dictionaries to handle NaN appropriately. This will be trivial
once we have templated dictionaries, but even now we should be able to get it
going by special-casing at runtime. This also is technically a breaking change,
but it breaks something broken.

--