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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

For now a few comments, I'll have a closer look later.



================
Comment at: libcxx/include/concepts:217
   is_lvalue_reference_v<_Lhs> &&
-  common_reference_with<const remove_reference_t<_Lhs>&, const remove_reference_t<_Rhs>&> &&
+  common_reference_with<__const_lvalue_ref<_Lhs>, __const_lvalue_ref<_Rhs>> &&
   requires (_Lhs __lhs, _Rhs&& __rhs) {
----------------
Minor bikeshed; since the standard type traits start with a verb I would like to do the same here. This "trait" is like `make_signed_t` so I would suggest `__make_const_lvalue_ref`.


================
Comment at: libcxx/test/std/concepts/comparison/concepts.totallyordered/totally_ordered.compile.pass.cpp:45
+static_assert(models_totally_ordered<int>());
+static_assert(models_totally_ordered<double>());
+static_assert(models_totally_ordered<void*>());
----------------
This feels wrong, when a double has a NaN value there's no total order. I wonder whether this is a bug in the Standard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98983