This is an archive of the discontinued LLVM Phabricator instance.

[libc++][test] Adds format string helper.
ClosedPublic

Authored by Mordante on Sep 16 2022, 6:01 AM.

Details

Reviewers
vitaut
ldionne
Group Reviewers
Restricted Project
Commits
rGd23f609d9c33: [libc++][test] Adds format string helper.
Summary

Update the formatter day tests to the new style.
Other test will be done separately.

Diff Detail

Event Timeline

Mordante created this revision.Sep 16 2022, 6:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 16 2022, 6:01 AM
Mordante requested review of this revision.Sep 16 2022, 6:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 16 2022, 6:01 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Can this be accompanied by changes in the tests proper?

I created this for the formatting escaped strings, which will take some time to review. I'm also converting the test for the landed day formatter and the year formatter which will be in review soon. So from that point of view it makes sense to have it separately. If wanted I can squash this with the formatter day changes.

Mordante updated this revision to Diff 460741.Sep 16 2022, 7:02 AM

Addresses review comments.

Mordante edited the summary of this revision. (Show Details)Sep 16 2022, 7:06 AM
Mordante added a reviewer: vitaut.

One minor comment inline otherwise looks great. Much more readable than template operator()!

libcxx/test/std/time/time.syn/formatter_tests.h
33–35 ↗(On Diff #460844)

Why is this a lambda and not a normal function? If it was the latter you could have a locale overload without inventing a new name.

Mordante marked an inline comment as done.Sep 17 2022, 12:00 PM

One minor comment inline otherwise looks great. Much more readable than template operator()!

I agree. I will adjust the other tests in a separate commit.

libcxx/test/std/time/time.syn/formatter_tests.h
33–35 ↗(On Diff #460844)

Good question. Basically since I used that pattern before. I've changed it to a normal function.

Mordante updated this revision to Diff 461024.Sep 17 2022, 12:00 PM
Mordante marked an inline comment as done.

Addresses review comments.

ldionne accepted this revision.Sep 20 2022, 9:29 AM
ldionne added inline comments.
libcxx/test/support/test_format_string.h
11–12
44–45

This will also avoid the comment becoming stale when/if we move to github PRs.

49
This revision is now accepted and ready to land.Sep 20 2022, 9:29 AM
This revision was automatically updated to reflect the committed changes.