[libcxx-commits] [PATCH] D148416: [libc++][spaceship] Implement `operator<=>` for `multiset` and `set`

[Mark de Wever via Phabricator via libcxx-commits]

Mordante accepted this revision.
Mordante added a comment.
This revision is now accepted and ready to land.

LGTM modulo one nit.



================
Comment at: libcxx/test/support/test_comparisons.h:258-259
 
+constexpr auto UnorderableIntMin = std::numeric_limits<int>::min();
+constexpr auto UnorderableIntMax = std::numeric_limits<int>::max();
+
----------------
Unlike `constexpr` functions `constexpr` variable are not `inline`.


================
Comment at: libcxx/test/support/test_comparisons.h:265
   friend constexpr std::partial_ordering operator<=>(PartialOrder lhs, PartialOrder rhs) {
-    if (lhs.value == std::numeric_limits<int>::min() || rhs.value == std::numeric_limits<int>::min())
+    if (lhs.value == UnorderableIntMin || rhs.value == UnorderableIntMin)
+      return std::partial_ordering::unordered;
----------------
I actually feel `std::numeric_limits<int>::min()` is more readable. Especially since the name kind of indicates it's not a plain `std::numeric_limits<int>::min()`. So I really like this part reverted and use the numeric_limits directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148416