This is an archive of the discontinued LLVM Phabricator instance.

[NFC][libc++][format] Prepare unit tests.
ClosedPublic

Authored by Mordante on Mar 26 2022, 12:55 PM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Commits
rG7277b00a29f0: [NFC][libc++][format] Prepare unit tests.
Summary

Before implementing P2216's format-string adjust the unit tests.
After P2216 the format* functions require a compile-time string literal.
This changes prepares the tests.

Diff Detail

Event Timeline

Mordante created this revision.Mar 26 2022, 12:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 26 2022, 12:55 PM
Mordante requested review of this revision.Mar 26 2022, 12:55 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 26 2022, 12:55 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Mordante planned changes to this revision.Mar 26 2022, 1:14 PM
Mordante updated this revision to Diff 418444.Mar 27 2022, 5:46 AM

Attempt to CI and some cleanups.

Mordante retitled this revision from [WIP][NFC][libc++][format] Prepare unit tests. to [NFC][libc++][format] Prepare unit tests..Apr 5 2022, 8:57 AM
Mordante edited the summary of this revision. (Show Details)
Mordante updated this revision to Diff 420533.Apr 5 2022, 8:57 AM

Rebased to trigger CI

ldionne accepted this revision.Apr 7 2022, 12:31 PM
ldionne added a subscriber: ldionne.

LGTM with some comments.

libcxx/test/std/utilities/format/format.functions/formatted_size.pass.cpp
27–28

I would prefer if the tests could include what they use -- here we're using string_literal but we haven't included string_literal.h. I think we should not rely on the transitive include via format_tests.h.

libcxx/test/support/string_literal.h
48–51

Should those be private?

This revision is now accepted and ready to land.Apr 7 2022, 12:31 PM
Mordante marked an inline comment as done.Apr 7 2022, 1:04 PM

Thanks for the review!

libcxx/test/std/utilities/format/format.functions/formatted_size.pass.cpp
27–28

In this case the test really requires format_tests.h to be included. But I agree it's good practice not to depend on transitive includes, so I'll fix it.

libcxx/test/support/string_literal.h
48–51

No that doesn't compile. I tried that, hence the comment at comment on line 27. I haven't looked at the exact rules of consteval, but I have a strong suspicion the class needs to be an aggregate.

Mordante marked 2 inline comments as done.Apr 8 2022, 8:11 AM
This revision was automatically updated to reflect the committed changes.