[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