[libcxx-commits] [PATCH] D110738: [libc++] [P1614] Implement the first half of [cmp.alg]: strong_order, weak_order, partial_order.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone marked 4 inline comments as done.
Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/language.support/cmp/cmp.alg/strong_order.pass.cpp:386
+#ifndef TEST_BUGGY_SIGNALING_NAN
+        assert(std::strong_order(ps, pq) == std::strong_ordering::less);
+#endif
----------------
mumbleskates wrote:
> Quuxplusone wrote:
> > @mumbleskates: This is the exact line that compares `quiet_NaN()` against `signaling_NaN()` (and see line 395 too).
> > Notice that we're testing "positive quiet NaN" (which might not be //literally// `quiet_NaN()`) against "positive signaling NaN" (which might not be //literally// `signaling_NaN()`), and lines 361 and 370 are testing "negative quiet" against "negative signaling"; we never literally test the `numeric_limits` values //directly//. The Standard doesn't mandate (AFAIK) whether `numeric_limits<F>::quiet_NaN()` should have a positive or negative sign bit.
> Yeah, this all makes sense. I was mostly thinking in terms of getting this to also run statically, or check at our `strong_ordering` header's compile time whether these basic assumptions about the natural ordering of float payloads hold. Again not sure whether that's within our mandate, but recall that the iec559 standard //recommends//, but does not //require//, that the signaling bit be set to 1 for quiet and 0 for signaling; it //may// be reversed.
> 
> So yes, this also depends on how paranoid we are being. The only arch I have read about doing this for sure is PA-RISC, whose most recently released CPU came out in 2005, and any support for that platform is "out-of-tree" for LLVM.
I think that adding `static_assert` to our actual `libcxx/include/` headers would be a bad idea. Our `libcxx/test/` tests will certainly catch such an issue, on any supported platform; and I would hope that anyone trying to use libc++ on a non-supported platform would at least think to run the tests first, just out of curiosity.

I did notice on Godbolt that apparently MSVC-on-Windows uses `0x7fc00000` for quiet-NaN (as usual) but `0x7fc00001` for signaling-NaN (which is not a signaling NaN on the platforms I'm familiar with).
https://godbolt.org/z/oh6ch1dGa


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110738/new/

https://reviews.llvm.org/D110738