[libcxx-commits] [PATCH] D117638: [libcxx] chrono::month_weekday should not be default constructible

[Casey Carter via Phabricator via libcxx-commits]

CaseyCarter marked an inline comment as done.
CaseyCarter added inline comments.


================
Comment at: libcxx/test/std/utilities/time/time.cal/time.cal.month/time.cal.month.nonmembers/comparisons.pass.cpp:43-45
     for (unsigned i = 1; i < 10; ++i)
-        for (unsigned j = 10; j < 10; ++j)
+        for (unsigned j = 10; j < 13; ++j)
             assert(testComparisons6Values<month>(i, j));
----------------
Quuxplusone wrote:
> For test coverage this //should// say
> ```
> for (int i=1; i <= 12; ++i)
>     for (int j=1; j <= 12; ++j)
>         assert(testComparisons6Values<month>(i, j));
> ```
> (I wildly speculate that it originally ran from 1 to 10 in both loops, but that that didn't pass because all this stuff was broken, so someone changed j=1 to j=10 to nerf the test and then shipped it.)
Ah, I incorrectly assumed from the fact that the ranges of `i` and `j` were disjoint that `testComparisons6Values` expected its first argument to be less than the second. Fixing, but I'll write `< 13` to be consistent with every other loop in these tests.


================
Comment at: libcxx/test/std/utilities/time/time.cal/time.cal.weekday/time.cal.weekday.nonmembers/minus.pass.cpp:32-41
     {
     WD wd{5};
     Ds offset{3};
     if (wd - offset != WD{2}) return false;
     if (wd - WD{2} != offset) return false;
     }
 
----------------
Quuxplusone wrote:
> 
Missed this comment. Fix incoming.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117638