[libcxx-commits] [PATCH] D128603: [libc++][chrono] Adds operator<=> for day.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/chrono:16
 
+#include <compare> // C++20
+
----------------
We have a test for mandated inclusions, does it need to be updated?


================
Comment at: libcxx/test/support/test_comparisons.h:10-13
+//      XXXX7  tests all seven comparison operators (the seventh is C++20's operator<=>)
+//      XXXXES tests op== and op<=> where both can have different suffixes.
+//      XXXX6  tests all six comparison operators
+//      XXXX2  tests only op== and op!=
----------------
Suggestion:

```
FooOrder<expected-ordering> => All seven comparison operators
FooComparison => All six pre-C++20 comparison operators
FooEqual => operator== and operator!=
```

This can be done in a separate patch. I don't think we actually need to have a separate `FooES` for testing `operator==` and `operator<=>` only, if that's already tested in `FooOrder` (which would then test all 7 operators).


================
Comment at: libcxx/test/support/test_comparisons.h:134-135
+{
+	AssertComparisons6AreNoexcept<T, U>();
+    ASSERT_NOEXCEPT(std::declval<const T&>() <=> std::declval<const U&>());
+}
----------------
Funky indentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128603