Implements the compile-time checking of the formatting arguments.
Completes:
- P2216 std::format improvements
Paths
| Differential D121530
[libc++][format] Implement format-string. ClosedPublic Authored by Mordante on Mar 12 2022, 10:55 AM.
Details
Summary Implements the compile-time checking of the formatting arguments. Completes:
Diff Detail
Event TimelineComment Actions The patch to the headers looks phenomenal. I only reviewed the code. I haven't looked at the spec yet. But the code was gorgeous. Well done. The test needs to be entirely rewritten. The macro stuff is complicated and hard to read. Tests need to be correct upon inspection. That means repeating yourself rather than adding in direction. Also that test file is way too big. Come up with a nice way to split it into different files. Thank you for working on this.
This revision now requires changes to proceed.Mar 12 2022, 3:30 PM
Comment Actions Updated test based on a discussion on Discord. Mordante added inline comments.
Mordante added a parent revision: D122534: [NFC][libc++][format] Prepare unit tests..Mar 26 2022, 1:11 PM Mordante marked an inline comment as done. Comment ActionsMinor improvements and depend on the new patch with the NFC test changes. Mordante added inline comments.
Mordante marked 9 inline comments as done. Comment ActionsAddress review comments.
Mordante retitled this revision from [WIP][libc++][format] Implement format-string. to [libc++][format] Implement format-string..Apr 9 2022, 9:32 AM Mordante added a child revision: D125606: [libc++][format] Improve string formatters.May 14 2022, 6:04 AM
Mordante removed parent revisions: D122534: [NFC][libc++][format] Prepare unit tests., D121514: [libc++][format] Improve format-arg-store.. Comment Actions Should probably be mentioned in the release notes...?
Comment Actions
Good point, thanks!
Comment Actions In general looks good but I would strongly recommend not to instantiate all of formatting path just to validate width/precision (see an inline comment).
This revision now requires changes to proceed.Jun 4 2022, 8:35 AM Comment Actions Thanks for the review! I've addressed some of the smaller comments, will look at the others tomorrow.
Mordante added inline comments.
Comment Actions This LGTM. I think most comments by @vitaut have been applied, and while I have a couple of comments, I think this is good to go.
Comment Actions Thanks for the review!
This revision was landed with ongoing or failed builds. Closed by commit rGaed5ddf8d097: [libc++][format] Implement format-string. (authored by Mordante). · Explain Why This revision was automatically updated to reflect the committed changes. Mordante marked an inline comment as done.
Mordante mentioned this in D127767: [libc++][format] Improves the handle test..Jun 14 2022, 10:29 AM Mordante added inline comments.
Revision Contents
Diff 421737 libcxx/benchmarks/formatter_float.bench.cpp
libcxx/docs/Status/Cxx20Papers.csv
libcxx/include/__format/parser_std_format_spec.h
libcxx/include/format
libcxx/test/std/utilities/format/format.functions/format.locale.pass.cpp
libcxx/test/std/utilities/format/format.functions/format.locale.verify.cpp
libcxx/test/std/utilities/format/format.functions/format.pass.cpp
libcxx/test/std/utilities/format/format.functions/format.verify.cpp
libcxx/test/std/utilities/format/format.functions/format_to.locale.pass.cpp
libcxx/test/std/utilities/format/format.functions/format_to.locale.verify.cpp
libcxx/test/std/utilities/format/format.functions/format_to.pass.cpp
libcxx/test/std/utilities/format/format.functions/format_to.verify.cpp
libcxx/test/std/utilities/format/format.functions/format_to_n.locale.pass.cpp
libcxx/test/std/utilities/format/format.functions/format_to_n.locale.verify.cpp
libcxx/test/std/utilities/format/format.functions/format_to_n.pass.cpp
libcxx/test/std/utilities/format/format.functions/format_to_n.verify.cpp
libcxx/test/std/utilities/format/format.functions/formatted_size.locale.pass.cpp
libcxx/test/std/utilities/format/format.functions/formatted_size.locale.verify.cpp
libcxx/test/std/utilities/format/format.functions/formatted_size.pass.cpp
libcxx/test/std/utilities/format/format.functions/formatted_size.verify.cpp
|
Since all the components are known at compile time why not make format string construction constexpr and make the benchmark more realistic by replacing vformat_to with format_to?