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?