This is an archive of the discontinued LLVM Phabricator instance.

[libc++][chrono] Add calendar type formatters.
ClosedPublic

Authored by Mordante on Oct 29 2022, 8:55 AM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Commits
rG105fef5dca7e: [libc++][chrono] Add calendar type formatters.
Summary

Some of the calendar types have landed before, this adds the missing
set. Note this does not complete the implementation of the chrono
formatters.

This removes the chrono header for some transitive include in C++17
mode. This is needed to avoid inclusion cycles.

Partially implements:

  • P1361 Integration of chrono with text formatting
  • P2372 Fixing locale handling in chrono formatters

Diff Detail

Event Timeline

Mordante created this revision.Oct 29 2022, 8:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 29 2022, 8:55 AM
Mordante updated this revision to Diff 473414.Nov 5 2022, 3:48 AM

CI fixes and more tests.

Mordante updated this revision to Diff 473422.Nov 5 2022, 5:51 AM

Rebased and more CI fixes.

Mordante updated this revision to Diff 473497.Nov 6 2022, 6:29 AM

CI fixes.

Mordante updated this revision to Diff 473499.Nov 6 2022, 7:39 AM

CI fixes.

Mordante updated this revision to Diff 473503.Nov 6 2022, 10:00 AM

CI fixes.

Mordante updated this revision to Diff 473505.Nov 6 2022, 11:07 AM

CI fixes.

Mordante updated this revision to Diff 479827.Dec 3 2022, 4:18 AM

Rebased, polishing, and improving tests.

Mordante updated this revision to Diff 479829.Dec 3 2022, 4:49 AM

CI fixes.

Mordante updated this revision to Diff 479833.Dec 3 2022, 5:08 AM

CI fixes.

Mordante updated this revision to Diff 479834.Dec 3 2022, 5:26 AM

CI fixes.

Mordante updated this revision to Diff 479848.Dec 3 2022, 9:49 AM

Add more tests.

Mordante updated this revision to Diff 479852.Dec 3 2022, 10:40 AM

CI fixes.

Mordante updated this revision to Diff 479853.Dec 3 2022, 10:50 AM

Trigger CI.

Mordante updated this revision to Diff 479855.Dec 3 2022, 11:12 AM

Reenable complete XI.

Mordante updated this revision to Diff 479859.Dec 3 2022, 11:47 AM

CI fixes.

Mordante updated this revision to Diff 479907.Dec 4 2022, 2:09 AM

CI fixes.

Mordante edited the summary of this revision. (Show Details)Dec 4 2022, 4:44 AM
Mordante updated this revision to Diff 479914.Dec 4 2022, 4:47 AM

CI fixes.

Mordante updated this revision to Diff 479926.Dec 4 2022, 8:24 AM

Rebased and CI fixes.

Mordante updated this revision to Diff 480181.Dec 5 2022, 11:44 AM

CI fixes.

Mordante updated this revision to Diff 480200.Dec 5 2022, 12:12 PM

CI fixes.

Mordante updated this revision to Diff 480495.Dec 6 2022, 8:09 AM

CI fixes.

Mordante updated this revision to Diff 480561.Dec 6 2022, 11:29 AM

Remove debug code.

Mordante updated this revision to Diff 481318.Dec 8 2022, 9:05 AM

Rebased and polishing.

Mordante published this revision for review.Dec 8 2022, 12:01 PM
Mordante added inline comments.
libcxx/include/atomic
2656

These are removed due to breakage see https://buildkite.com/llvm-project/libcxx-ci/builds/15514#0184ddf6-ad35-496a-8808-4b35ec626653

chrono does not affect C++03 so not listed in the release notes.

Herald added a project: Restricted Project. · View Herald TranscriptDec 8 2022, 12:01 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne accepted this revision.Dec 21 2022, 12:37 PM
ldionne added a subscriber: ldionne.
ldionne added inline comments.
libcxx/include/__chrono/convert_to_tm.h
43

Otherwise, it's too likely to become out of date.

47

Do you have a test that would fail if we had that bug?

Answer: yes, it seems you ran into that bug.

libcxx/include/__chrono/formatter.h
586–587

I would remove this from the code since it's just a personal note.

libcxx/include/__chrono/ostream.h
165
libcxx/include/atomic
2656

I don't understand this. Especially since that CI run contains XPASSes, not failures. In other words, it seems like whatever change that was, it actually fixed some tests.

Thanks for explaining, we were mis-detecting the non-lockfree-atomics Lit feature.

libcxx/test/std/time/time.cal/time.cal.md/time.cal.md.nonmembers/ostream.pass.cpp
78

Is this a leftover?

This revision is now accepted and ready to land.Dec 21 2022, 12:37 PM
Mordante marked 4 inline comments as done.Dec 24 2022, 4:04 AM

Thanks for the review!

libcxx/test/std/time/time.cal/time.cal.md/time.cal.md.nonmembers/ostream.pass.cpp
78

No it's something that looks like a bug in the Standard. I have improved the comment.

Mordante updated this revision to Diff 485194.Dec 24 2022, 4:06 AM
Mordante marked an inline comment as done.

Rebased and addresses review comments, just a test before landing.

This revision was automatically updated to reflect the committed changes.