[libcxx-commits] [PATCH] D92730: [libc++] [LWG3221] Add tests for wrapping operator+(year_month, months).

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

A few minor nits, but otherwise LGTM.



================
Comment at: libcxx/test/std/utilities/time/time.cal/time.cal.ym/time.cal.ym.nonmembers/plus.pass.cpp:41
+    years offset{23};
     if (static_cast<int>((ym         ).year()) !=  1) return false;
     if (static_cast<int>((ym + offset).year()) != 24) return false;
----------------
Since you're cleaning up this code. would it make sense to change these `if`s by `assert`s?


================
Comment at: libcxx/test/std/utilities/time/time.cal/time.cal.ym/time.cal.ym.nonmembers/plus.pass.cpp:66
+
+  static_assert(testConstexprYears(year_month{year{1}, month{1}}), "");
+
----------------
Since the test is C++20, can you remove the empty string?


================
Comment at: libcxx/test/std/utilities/time/time.cal/time.cal.ym/time.cal.ym.nonmembers/plus.pass.cpp:92
+
+  static_assert(testConstexprMonths(year_month{year{1}, month{1}}), "");
+
----------------
Since the test is C++20, can you remove the empty string?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92730