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

Sep 19, 2014

Steven Schveighoffer

Oct 08, 2014

Steven Schveighoffer

Jan 30, 2018

Jan 30, 2018

Sep 08, 2020

Dec 17, 2022

Dec 07

dlangBugzillaToGithub

https://issues.dlang.org/show_bug.cgi?id=13499 Francesco Mecca <me@francescomecca.eu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |me@francescomecca.eu --- Comment #2 from Francesco Mecca <me@francescomecca.eu> --- What do you propose as the right solution? The problem is mainly that the TypeInfo comparison returns an integer that has no real meaning when the comparison is between a number and NaN. Throwing an exception doesn't seem like the best solution to me. --

https://issues.dlang.org/show_bug.cgi?id=13499 --- Comment #3 from Steven Schveighoffer <schveiguy@yahoo.com> --- I don't know what to propose. You are right that there is no valid return. I don't even remember what prompted me to create this bug (wish I had wrote more context here), as it was so long ago. If I had to guess, it was because AA's at the time may have still used a tree to store bucket collisions, and compare was messing up if you used a float as a key type. But these days, that isn't the case, and the builtin sort property is gone, so very little code actually uses TypeInfo.compare. I would guess that perhaps, while this is still a bug, it isn't as significant as it once was. --

September 08, 2020

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

Posted by Andrei Alexandrescu

Permalink

Andrei Alexandrescu

Permalink

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

Andrei Alexandrescu <andrei@erdani.com> changed:

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

--- Comment #4 from Andrei Alexandrescu <andrei@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.

--

https://issues.dlang.org/show_bug.cgi?id=13499 Iain Buclaw <ibuclaw@gdcproject.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Priority|P1 |P3 --

https://issues.dlang.org/show_bug.cgi?id=13499 --- Comment #5 from dlangBugzillaToGithub <robert.schadek@posteo.de> --- THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/17283 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB --

Forums