Page MenuHomePhabricator
Differential D134742

[libc++][chrono] Implements formatter duration.
AcceptedPublic
Actions

Authored by Mordante on Sep 27 2022, 8:10 AM.

Details

Reviewers
ldionne
vitaut
Group Reviewers
Restricted Project
Commits
rG719c3dc6f2f7: [libc++][chrono] Implements formatter duration.
Summary

Partially implements:

  • P1361 Integration of chrono with text formatting
  • P2372 Fixing locale handling in chrono formatters
  • LWG3270 Parsing and formatting %j with durations

Completes:

  • P1650R0 std::chrono::days with 'd' suffix
  • LWG3262 Formatting of negative durations is not specified
  • LWG3314 Is stream insertion behavior locale dependent when Period::type is micro?

Diff Detail

Event Timeline

Mordante created this revision.Sep 27 2022, 8:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 27 2022, 8:10 AM
Herald added subscribers: mstorsjo, arichardson. · View Herald Transcript
Harbormaster completed remote builds in B188957: Diff 463239.Sep 27 2022, 8:41 AM
Mordante updated this revision to Diff 463250.Sep 27 2022, 8:57 AM

CI fixes.

Harbormaster completed remote builds in B188966: Diff 463250.Sep 27 2022, 9:27 AM
Mordante updated this revision to Diff 466373.Oct 9 2022, 7:03 AM

Attempts to fix CI.

Harbormaster completed remote builds in B191169: Diff 466373.Oct 9 2022, 7:04 AM
Mordante edited the summary of this revision. (Show Details)Oct 9 2022, 7:08 AM
Mordante removed a parent revision: D134138: [libc++][chrono] Implements formatter month..
Mordante updated this revision to Diff 466374.Oct 9 2022, 7:09 AM

Fix dependencies.

Harbormaster completed remote builds in B191170: Diff 466374.Oct 9 2022, 7:57 AM
Mordante updated this revision to Diff 467192.Oct 12 2022, 10:41 AM

CI fixes.

Harbormaster completed remote builds in B191774: Diff 467192.Oct 12 2022, 12:56 PM
Mordante updated this revision to Diff 468007.Oct 15 2022, 4:01 AM

CI fixes.

Harbormaster completed remote builds in B192339: Diff 468007.Oct 15 2022, 4:30 AM
Mordante updated this revision to Diff 468009.Oct 15 2022, 4:55 AM

CI fixes.

Harbormaster completed remote builds in B192341: Diff 468009.Oct 15 2022, 5:42 AM
Mordante updated this revision to Diff 468013.Oct 15 2022, 5:59 AM

CI fixes.

Harbormaster completed remote builds in B192345: Diff 468013.Oct 15 2022, 6:57 AM
Mordante updated this revision to Diff 468016.Oct 15 2022, 7:05 AM

CI fixes

Harbormaster completed remote builds in B192348: Diff 468016.Oct 15 2022, 7:32 AM
Mordante updated this revision to Diff 468017.Oct 15 2022, 7:37 AM

CI fixes

Harbormaster completed remote builds in B192349: Diff 468017.Oct 15 2022, 8:08 AM
Mordante updated this revision to Diff 468019.Oct 15 2022, 8:17 AM

CI fixes.

Mordante updated this revision to Diff 468023.Oct 15 2022, 8:44 AM

CI fixes.

Harbormaster completed remote builds in B192354: Diff 468023.Oct 15 2022, 9:13 AM
Mordante updated this revision to Diff 468027.Oct 15 2022, 10:00 AM

CI fixes.

Harbormaster completed remote builds in B192358: Diff 468027.Oct 15 2022, 10:49 AM
Mordante updated this revision to Diff 468061.Oct 16 2022, 4:07 AM

Rebased enable all CI builders and minor polishing.

Mordante added a child revision: D136037: [libc++][chrono] Implements formatter weekday..Oct 16 2022, 5:22 AM
Harbormaster completed remote builds in B192385: Diff 468061.Oct 16 2022, 5:32 AM
Mordante updated this revision to Diff 468065.Oct 16 2022, 5:57 AM

CI fixes.

Harbormaster completed remote builds in B192389: Diff 468065.Oct 16 2022, 6:03 AM
Mordante updated this revision to Diff 468066.Oct 16 2022, 6:07 AM

Fixes typo.

Harbormaster completed remote builds in B192390: Diff 468066.Oct 16 2022, 7:42 AM
Mordante published this revision for review.Oct 16 2022, 7:49 AM
Mordante added reviewers: ldionne, vitaut.
Herald added a project: Restricted Project. · View Herald TranscriptOct 16 2022, 7:49 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
ldionne accepted this revision.Oct 18 2022, 8:59 AM

LGTM with comments!

libcxx/include/__chrono/convert_to_tm.h
29–32

Suggestion. _ChronoEntity is kind of hand-wavy, but _ChronoCalendarTimePoint is now wrong with this new patch, so let's change it.

libcxx/test/std/time/time.syn/formatter.duration.pass.cpp
554

Also elsewhere.

665
This revision is now accepted and ready to land.Oct 18 2022, 8:59 AM
Mordante marked 3 inline comments as done.Oct 18 2022, 10:03 AM
Mordante updated this revision to Diff 468609.Oct 18 2022, 10:08 AM

Rebased and addresses review comments.

Harbormaster completed remote builds in B192792: Diff 468609.Oct 18 2022, 11:39 AM
Closed by commit rG719c3dc6f2f7: [libc++][chrono] Implements formatter duration. (authored by Mordante). · Explain WhyOct 18 2022, 11:39 AM
This revision was automatically updated to reflect the committed changes.
Mordante added a commit: rG719c3dc6f2f7: [libc++][chrono] Implements formatter duration..
Mordante mentioned this in rG9f8340eff399: [libc++][chrono] Fixes build..Oct 18 2022, 11:58 AM
h-vetinari added a subscriber: h-vetinari.Oct 18 2022, 10:05 PM
h-vetinari added inline comments.
libcxx/docs/Status/Cxx20Papers.csv
130

Extraneous >>>"",<<<

Mordante mentioned this in rG8c489bd1f168: [libc++][doc] Fixes status pages..Oct 19 2022, 10:27 AM
Mordante marked an inline comment as done.Oct 19 2022, 10:28 AM

@h-vetinari Thanks for spotting the issue, I've pushed a fix.

EricWF reopened this revision.Oct 30 2022, 4:31 PM
EricWF added a subscriber: EricWF.

Please fix the undefined behavior mentioned above.

libcxx/include/__chrono/convert_to_tm.h
43

Does this not overflow for a lot of things?

Like hours(duration_cast<hours>(seconds::max()) + hours(1))?

Instead of reverting to raw arithmetic on ints, use chrono to get the values you want.

See https://godbolt.org/z/GYc7Tzn9W

This revision is now accepted and ready to land.Oct 30 2022, 4:31 PM
Mordante mentioned this in D138826: [libc++][chrono] Fixes formatter duration..Nov 28 2022, 8:35 AM
Mordante added inline comments.Nov 28 2022, 10:41 AM
libcxx/include/__chrono/convert_to_tm.h
43

The value is intended to be time since midnight, but it indeed may overflow.

Unfortunately it's not as simple as you suggest. Casting to hours fails for other small units, for example atto seconds. D138826 addresses the issue.

Revision Contents
Changeset List

PathSize
libcxx/
docs/
Status/
6 lines
4 lines
2 lines
include/
__chrono/
27 lines
188 lines
68 lines
2 lines
8 lines
test/
std/
time/
time.duration/
time.duration.nonmember/
232 lines
time.syn/
1091 lines
utilities/
format/
format.formattable/
2 lines

Diff 468636

View Options

libcxx/docs/Status/Cxx20Issues.csv

Loading...
View Options

libcxx/docs/Status/Cxx20Papers.csv

Loading...
View Options

libcxx/docs/Status/FormatPaper.csv

Loading...
View Options

libcxx/include/__chrono/convert_to_tm.h

Loading...
View Options

libcxx/include/__chrono/formatter.h

Loading...
View Options

libcxx/include/__chrono/ostream.h

Loading...
View Options

libcxx/include/__chrono/parser_std_format_spec.h

Loading...
View Options

libcxx/include/chrono

Loading...
View Options

libcxx/test/std/time/time.duration/time.duration.nonmember/ostream.pass.cpp

Loading...
View Options

libcxx/test/std/time/time.syn/formatter.duration.pass.cpp

Loading...
View Options

libcxx/test/std/utilities/format/format.formattable/concept.formattable.compile.pass.cpp

Loading...