[libcxx-commits] [PATCH] D129887: [libc++][chrono] Uses operator<=> in the calendar.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/include/__chrono/year_month.h:62-80
 _LIBCPP_HIDE_FROM_ABI inline constexpr
 bool operator!=(const year_month& __lhs, const year_month& __rhs) noexcept
 { return !(__lhs == __rhs); }
 
 _LIBCPP_HIDE_FROM_ABI inline constexpr
 bool operator< (const year_month& __lhs, const year_month& __rhs) noexcept
 { return __lhs.year() != __rhs.year() ? __lhs.year() < __rhs.year() : __lhs.month() < __rhs.month(); }
----------------
Is this an omission?


================
Comment at: libcxx/test/std/time/time.cal/time.cal.ym/time.cal.ym.nonmembers/comparisons.pass.cpp:53-56
+  // Use a maximum to avoid hitting the maximum number of constexpr steps.
+  // Another solution would be increment the number of step, but that leads to
+  // long compilation times.
+  int max = std::is_constant_evaluated() ? 1010 : 2000;
----------------
Let's just always use 1010, I don't think we gain any fundamental coverage by doing more than that (do you agree?).

And in fact, we probably also want to test boundary values (at least `year{0}` and `year{1}`) if we don't already.


================
Comment at: libcxx/test/std/time/time.cal/time.cal.ymd/time.cal.ymd.nonmembers/comparisons.pass.cpp:102
+  // long compilation times.
+  int max = std::is_constant_evaluated() ? 1010 : 2000;
+
----------------
Same comment about number of steps and boundary values.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129887