[libcxx-commits] [PATCH] D98983: [libcxx] adds concepts `std::totally_ordered` and `std::totally_ordered_with`

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added a comment.

This mostly looks good to me. I want to read over the logic part one more time while looking at the standard, but I think this is pretty much good to go. Sorry for being slow with reviewing.



================
Comment at: libcxx/test/std/concepts/concepts.compare/concepts.totallyordered/totally_ordered.pass.cpp:102
+namespace standard_types {
+static_assert(models_totally_ordered<std::array<int, 10> >());
+static_assert(models_totally_ordered<std::deque<int> >());
----------------
Nit: `> >` -> `>>`


================
Comment at: libcxx/test/std/concepts/concepts.compare/concepts.totallyordered/totally_ordered.pass.cpp:115
+struct A {};
+// FIXME(cjdb): uncomment when operator<=> is implemented for each of these types.
+// static_assert(!std::totally_ordered<std::array<A, 10> >);
----------------
I think I brought this up before, but do you want to create a bug for spaceship support and then put a link to it in all the tests that need to be updated? That way you could just do a cmd+f to find all the places that mention the bug ID and need to be updated, rather than searching through all "FIXME" comments. 


================
Comment at: libcxx/test/std/concepts/concepts.compare/concepts.totallyordered/totally_ordered_with.pass.cpp:94
+static_assert(!check_totally_ordered_with<int, int (S::*)() const&&>());
+static_assert(!check_totally_ordered_with < int,
+              int (S::*)() const&& noexcept > ());
----------------
Also weird formatting here and on L92 (among other places around here).


================
Comment at: libcxx/test/std/concepts/concepts.compare/types.h:13
 #include <concepts>
+#include <string>
 #include <type_traits>
----------------
Why do we now need to include <string>?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98983