[libcxx-commits] [PATCH] D145881: [libc++][spaceship] Implement `operator<=>` for `duration`

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.
Herald added a subscriber: mikhail.ramalho.

Thanks for working on this! A few comments.



================
Comment at: libcxx/docs/Status/SpaceshipProjects.csv:55
 | `[range.elements.iterator] <https://wg21.link/range.elements.iterator>`_,| ranges::elements_view::iterator,[concepts.cmp],Hui Xie,|Complete|
-| `[time.duration.comparisons] <https://wg21.link/time.duration.comparisons>`_, "chrono::duration", None, Mark de Wever, |Not Started|
+| `[time.duration.comparisons] <https://wg21.link/time.duration.comparisons>`_, `chrono::duration <https://reviews.llvm.org/D145881>`_, None, Hristo Hristov, |Complete|
 | `[time.point.comparisons] <https://wg21.link/time.point.comparisons>`_, "chrono::time_point", None, Mark de Wever, |Not Started|
----------------
Please reach out to us on Discord when you want to work on patches that are assigned to people, me in this case.
Some of the patches in this list already have patches locally that are block for various reasons.



================
Comment at: libcxx/test/std/time/time.duration/time.duration.comparisons/compare.three_way.pass.cpp:26
+
+int main(int, char**) {
+    {
----------------
Please add runtime tests too.
I'm quite sure you already used our
```
  test();
  static_assert(test());
```
pattern in other patches. If not I can point you to examples.


================
Comment at: libcxx/test/std/time/time.duration/time.duration.comparisons/compare.three_way.pass.cpp:65
+    constexpr std::chrono::duration<int, std::ratio<2, 3>> s1(9);
+    constexpr std::chrono::duration<double, std::ratio<3, 5>> s2(10);
+    static_assert((s1 <=> s2) == std::strong_ordering::equal);
----------------
Nit I think it's safer to use less or greater when comparing floating point values.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145881