Fix issue 20478

[Biotronic]

No description provided.

[dlang-bot]

Thanks for your pull request and interest in making D better, @Biotronic! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	20478	enhancement	Phobos should have a trait to identify NaN-like behavior

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + phobos#7342"

[thewilsonator]

std/traits.d(3936:15)[warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const.
std/traits.d(3945:11)[warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const.

[Biotronic]

BuildKite runs out of memory testing printf, and Pipelines fails in part because my name is weird. I don't see how the former is affected by this PR, so if it is, please tell me how to fix it.

As for fixing my name, I guess I can do that, but it seems somewhere there's a two- or three-level mojibake factory, and it will probably mess up other people's names as well. (my last name is Kjærås. Translating that from UTF-8 to Windows-1252 you get KjærÃ¥s, which is the first version encountered. I'm not entirely sure what the next steps are to get Kjâ€~r†s, but since there's three characters where previously there were two, we've probably visited UTF-8 again before trying another )

[ghost]

BuildKite runs out of memory

Looks unrelated to me. In the last weeks we had several such false positives. If anyone has the ability to rerun these tests, it would be nice to see, if this repeats.

and Pipelines fails in part because my name is weird.

As far as I know, @wilzbach set up the azur pipelines. Maybe he can help here?

[MoonlightSentinel]

See dlang/dmd#10575 for another case like this.

[PetarKirov]

I don't think hasNanComparison is a good name. The right term for this is partial ordering. See e.g. Rust: https://doc.rust-lang.org/std/cmp/trait.PartialOrd.html

[PetarKirov]

@thewilsonator Also this as PR is adding a new symbol @atilaneves should approve it, before it can be merged.

[wilzbach]

BuildKite runs out of memory testing printf, and Pipelines fails in part because my name is weird. I don't see how the former is affected by this PR, so if it is, please tell me how to fix it.

👍

As far as I know, @wilzbach set up the azur pipelines. Maybe he can help here?

I don't know much about the Windows details, but I guess this would be a workaround: dlang/dmd#10719.

[atilaneves]

I read the original issue and it says "it makes sense" to have this but doesn't actually explain why it's needed.

[n8sh]

What is the reason for the recursion instead of isFloatingPoint!(ReturnType!((T1 a, T2 b) => a.opCmp(b)))? If there's some subtlety here, please add a comment explaining it.

[Biotronic]

opCmp could return a type that isn't floating-point but has an opCmp returning float. This tower of opCmp could be multiple layers deep. I don't actually see a good reason to do this, but that doesn't mean a good reason doesn't exist.

[RazvanN7]

The autotester failures will most likely be fixed by a rebase, however, the rationale, as @atilaneves pointed out is still missing.

[RazvanN7]

Closing due to lack of activity. We still have the bugzilla issue and this PR is linked to it. If anyone can provide a rationale for this change and is willing to take it to the finish line, feel free to reopen.