D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 13663 - Comparison of Tuples with floating point fields
Summary: Comparison of Tuples with floating point fields
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: phobos (show other issues)
Version: D2
Hardware: x86_64 Windows
: P1 normal
Assignee: No Owner
URL:
Keywords: pull
: 18832 (view as issue list)
Depends on:
Blocks:
 
Reported: 2014-10-29 09:26 UTC by Ryuichi OHORI
Modified: 2021-01-19 15:38 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Ryuichi OHORI 2014-10-29 09:26:53 UTC
A Tuple with only component nan is greater than itself.

unittest
{
	import std.typecons;
	real x;
	auto t = tuple(x);
	assert (t > t);
	assert (!(t < t));
}
Comment 1 Peter Alexander 2014-12-14 21:17:23 UTC
There is nothing that can be done here. For user defined types (e.g. Tuple), comparison operators are converted to calls to opCmp, i.e.

a < b	a.opCmp(b) < 0
a <= b	a.opCmp(b) <= 0
a > b	a.opCmp(b) > 0
a >= b	a.opCmp(b) >= 0

For NaN vs NaN, the comparison is neither equal, less than, nor greater than. There is nothing that opCmp can return to give the desired semantics.
Comment 2 berni44 2019-12-04 06:14:09 UTC
I cannot even see a place, where this can be clearified in the docs. Maybe in some very general place, like some page on floating point numbers in general.
Comment 3 Simen Kjaeraas 2019-12-04 07:46:06 UTC
Peter's claims in comment 1 are plain false - opCmp can return float, and float.nan for incomparable cases. Here's an implementation of opCmp that does that:

        float opCmp(R)(R rhs)
        if (areCompatibleTuples!(typeof(this), R, "<"))
        {
            static foreach (i; 0 .. Types.length)
            {
                if (field[i] != field[i] || rhs.field[i] != rhs.field[i])
                {
                    return float.nan;
                }
                if (field[i] != rhs.field[i])
                {
                    return field[i] < rhs.field[i] ? -1 : 1;
                }
            }
            return 0;
        }

        /// ditto
        float opCmp(R)(R rhs) const
        if (areCompatibleTuples!(typeof(this), R, "<"))
        {
            static foreach (i; 0 .. Types.length)
            {
                if (field[i] != field[i] || rhs.field[i] != rhs.field[i])
                {
                    return float.nan;
                }
                if (field[i] != rhs.field[i])
                {
                    return field[i] < rhs.field[i] ? -1 : 1;
                }
            }
            return 0;
        }

These are taken directly from std.typecons, and the only change made is they return float, and check if any of the fields are incomparable.
Comment 4 berni44 2019-12-04 11:18:30 UTC
(In reply to Simen Kjaeraas from comment #3)
> opCmp can return float

Oh, I didn't know that trick with returning nan. I'll check your solution and add a PR.
Comment 5 Dlang Bot 2019-12-04 11:54:32 UTC
@berni44 created dlang/phobos pull request #7301 "Fix Issue 13663 - Comparison of Tuples with floating point fields" fixing this issue:

- Fix Issue 13663 - Comparison of Tuples with floating point fields

https://github.com/dlang/phobos/pull/7301
Comment 6 berni44 2019-12-21 09:21:38 UTC
*** Issue 18832 has been marked as a duplicate of this issue. ***
Comment 7 Dlang Bot 2021-01-17 18:12:47 UTC
@berni44 created dlang/phobos pull request #7748 "Fix Issue 13663 - Comparison of Tuples with floating point fields" fixing this issue:

- Fix Issue 13663 - Comparison of Tuples with floating point fields

https://github.com/dlang/phobos/pull/7748
Comment 8 Dlang Bot 2021-01-19 15:38:08 UTC
dlang/phobos pull request #7748 "Fix Issue 13663 - Comparison of Tuples with floating point fields" was merged into master:

- ab2f7cdef4d80964a85440699b358fb95b50b26f by Bernhard Seckinger:
  Fix Issue 13663 - Comparison of Tuples with floating point fields

https://github.com/dlang/phobos/pull/7748