This is another piece in the ongoing <chrono> saga.
Implement the class hh_mm_ss.
Details
Diff Detail
Event Timeline
| libcxx/include/chrono | ||
|---|---|---|
| 2757 | It seems like this shouldn't be reimplemented inside this class, instead we should put this at namespace scope if we don't already have something like that. | |
| 2765 | Same comment as above regarding duplication. | |
| 2766 | Indentation looks funny (here and elsewhere). Is it just Phab acting up? | |
The __pow10 and __width routines are only called at compile-time; no run-time code at all.
We have a "power of 10" table in charconv, but I don't think that coupling <chrono> and <charconv> is worth the benefit of removing this small routine.
| libcxx/include/chrono | ||
|---|---|---|
| 2766 | It was some tabs. | |
We've discussed that offline, however I want to say it here for posterity. The fact that we have monolithic headers in libc++ sometimes leads us to duplicate code (like here). If instead we had finer grained headers, we could reuse code without fear of adding coupling between unrelated headers.
I was told there was a fear that adding header files would increase compile-times, however I haven't personally seen that claim substantiated. Also, having finer grained headers means that we might be able to pull in less code when we include something.
| libcxx/include/chrono | ||
|---|---|---|
| 617 | When do you plan do update the feature-test macro __cpp_lib_chrono? | |
| 2757 | I'd like it to be promoted so it can be tested on its own. | |
| libcxx/test/std/utilities/time/time.hms/time.hms.members/hours.pass.cpp | ||
| 16 | Can you add a comment in the other related test files saying there's a correspondence table in this file? Something like: // See the table in hours.pass.cpp for correspondence between the magic values used below | |
| libcxx/test/std/utilities/time/time.hms/time.hms.nonmembers/nothing.to.do.pass.cpp | ||
| 11 | What is this testing? | |
| libcxx/include/chrono | ||
|---|---|---|
| 617 | When I get the rest of the calendaring stuff implemented. | |
| libcxx/test/std/utilities/time/time.hms/time.hms.members/hours.pass.cpp | ||
| 16 | I can do that. | |
| libcxx/test/std/utilities/time/time.hms/time.hms.nonmembers/nothing.to.do.pass.cpp | ||
| 11 | Nothing. Due to historical reasons; we try not to have folders with no tests. | |
| libcxx/include/chrono | ||
|---|---|---|
| 2757 | But it's not used anywhere else; nor really do I think it will be. | |
When do you plan do update the feature-test macro __cpp_lib_chrono?