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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante marked 3 inline comments as done.
Mordante added a comment.

Thanks for the review!



================
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(); }
----------------
ldionne wrote:
> Is this an omission?
Yes, thanks for spotting the issue.


================
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;
----------------
ldionne wrote:
> 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.
I'll change the range to -5 to 5. Same for the next one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129887