[libcxx-commits] [PATCH] D107036: [WIP] [libc++] [P1614] Various unimplemented parts of <compare>. [WIP]

[Christopher Di Bella via Phabricator via libcxx-commits]

cjdb requested changes to this revision.
cjdb added a comment.
This revision now requires changes to proceed.

In D107036#2914445 <https://reviews.llvm.org/D107036#2914445>, @Quuxplusone wrote:

> Has tests now.
> `std::strong_order` still doesn't compare NaNs correctly; we may need a domain expert for this.
> Everything up to `strong_order` is probably ready to upload separately for review:
>
> - [libc++][modularisation] Split up <concepts> into granular headers.
> - [libc++] [p1614] Implement std::compare_three_way_result.
> - [libc++] [p1614] Implement std::three_way_comparable.
> - [libc++] [p1614] Implement std::compare_three_way.
>
> @ldionne: Do you want the above as four PRs, one PR, or something in between?

I'd appreciate it if you could make these dependent PRs to make it easier to review.



================
Comment at: libcxx/include/__concepts/__boolean_testable.h:1
+//===----------------------------------------------------------------------===//
+//
----------------
Please rename this file to `boolean_testable.h`. We decided a while back to not prefix detail headers with underscores.


================
Comment at: libcxx/include/concepts:155-159
 // [concept.derived]
 template<class _Dp, class _Bp>
 concept derived_from =
   is_base_of_v<_Bp, _Dp> &&
   is_convertible_v<const volatile _Dp*, const volatile _Bp*>;
----------------
If you're going to split up `<concepts>`, please entirely split it up. Having some content in `<concepts>` and other stuff in `__concepts` is going to be difficult to track.


================
Comment at: libcxx/test/std/language.support/cmp/cmp.concept/three_way_comparable.compile.pass.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
There are a lot of missing tests for this file. Please reintroduce them.


================
Comment at: libcxx/test/std/language.support/cmp/cmp.concept/three_way_comparable_with.compile.pass.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
There are a lot of missing tests for this file. Please reintroduce them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107036