[PATCH] D25416: Add Chrono.h - std::chrono support header
Mehdi AMINI via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 14 09:11:36 PDT 2016
mehdi_amini added a comment.
Looks fine to me overall! Thanks.
================
Comment at: unittests/Support/Chrono.cpp:36
+ raw_string_ostream OS(S);
+ OS << system_clock::now();
+
----------------
I initially thought this test (StringConversion ) would be doing something else (`TimePoint` string conversion), but both are valuable so what do you think about another test like:
```
auto Now = system_clock::now();
std::string S1;
{
raw_string_ostream OS(S1);
OS << system_clock::now();
}
TimePoint T(Now);
std::string S2;
{
raw_string_ostream OS(S2);
OS << T;
}
ASSERT_EQ(S1, S2);
}
```
(I don't think you added coverage for `raw_ostream &operator<<(raw_ostream &OS, TimePoint TP) {`, it is likely covered by the `TimeValue` tests (if any), but if we remove TimeValue at some point we may lose the coverage.
https://reviews.llvm.org/D25416
More information about the llvm-commits
mailing list