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

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone accepted this revision as: Quuxplusone.
Quuxplusone 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));
----------------
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.)


================
Comment at: libcxx/test/std/utilities/time/time.cal/time.cal.wdidx/time.cal.wdidx.nonmembers/streaming.pass.cpp:9
 // UNSUPPORTED: c++03, c++11, c++14, c++17
 // XFAIL: *
 
----------------
You missed this `XFAIL: *`.
But, looking at these XFAIL'ed tests, I'm noticing that none of them are good. They just print something to std::cout (!) and then exit without asserting anything (!!). Do we want to just delete them, and ask whoever eventually implements <chrono> to write their own 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;
     }
 
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117638