This is an archive of the discontinued LLVM Phabricator instance.

[libc++][chrono] Implements formatter year.
ClosedPublic

Authored by Mordante on Sep 11 2022, 3:16 AM.

Details

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

Partially implements:

  • P1361 Integration of chrono with text formatting

Diff Detail

Event Timeline

Mordante created this revision.Sep 11 2022, 3:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 11 2022, 3:16 AM
Mordante updated this revision to Diff 459345.Sep 11 2022, 3:45 AM

Attempts to fix CI.

Mordante updated this revision to Diff 459352.Sep 11 2022, 4:28 AM

Fixes CI.

Mordante updated this revision to Diff 459376.Sep 11 2022, 8:33 AM

Attempts to fix the CI.

Mordante updated this revision to Diff 459378.Sep 11 2022, 9:15 AM

Fixing CI.

Mordante updated this revision to Diff 459473.Sep 12 2022, 8:44 AM

Attempts to fix CI.

Mordante updated this revision to Diff 459500.Sep 12 2022, 10:06 AM

Removes accidentally duplicated line.

Mordante updated this revision to Diff 460295.Sep 14 2022, 10:28 PM

Attempts to fix CI.

Mordante updated this revision to Diff 460641.Sep 15 2022, 10:43 PM

Attempts to fix CI, only testing the affected ones.

Mordante updated this revision to Diff 460650.Sep 15 2022, 11:14 PM

Readd removed CI runners.

Mordante updated this revision to Diff 460979.Sep 17 2022, 2:09 AM

Avoids templates in tests.

Mordante published this revision for review.Sep 17 2022, 3:31 AM
Mordante added reviewers: ldionne, vitaut.
Herald added a project: Restricted Project. · View Herald TranscriptSep 17 2022, 3:31 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Mordante updated this revision to Diff 462542.Sep 23 2022, 10:19 AM

Trigger CI.

Mordante updated this revision to Diff 462726.Sep 25 2022, 6:36 AM

Rebased to trigger CI.

Mordante updated this revision to Diff 462728.Sep 25 2022, 7:16 AM

Fixes CI.

Mordante updated this revision to Diff 462732.Sep 25 2022, 8:16 AM

Attempts to fix the CI.

Mordante updated this revision to Diff 462737.Sep 25 2022, 8:43 AM

Fixes CI.

Mordante updated this revision to Diff 462739.Sep 25 2022, 9:49 AM

CI fixes.

Mordante updated this revision to Diff 462743.Sep 25 2022, 11:31 AM

Attempts to fix CI.

Mordante updated this revision to Diff 462912.Sep 26 2022, 8:14 AM

Polishing.

ldionne accepted this revision.Oct 4 2022, 9:13 AM

LGTM.

libcxx/include/__chrono/formatter.h
61–63

Since this is used in a single place, any reason not to inline it where it's used?

68

Let's perform the tm => int-from-year-0 conversion in the caller of this function instead. Applies above and below too.

71

I don't understand this comment.

80
136–139
libcxx/test/libcxx/transitive_includes/cxx20.csv
79

This should go away once you rebase onto main with the updated transitive includes tests.

libcxx/test/std/time/time.cal/time.cal.year/time.cal.year.nonmembers/ostream.pass.cpp
11

Here and in the other tests.

This revision is now accepted and ready to land.Oct 4 2022, 9:13 AM
Mordante marked 5 inline comments as done.Oct 4 2022, 10:07 AM

Thanks for the review!

libcxx/include/__chrono/formatter.h
61–63

As discussed I prefer to keep code in switch statements short. However since this is a one-liner I moved it.

Mordante updated this revision to Diff 465068.Oct 4 2022, 10:16 AM
Mordante marked an inline comment as done.

Rebased and addresses review comments.

This revision was automatically updated to reflect the committed changes.