This is an archive of the discontinued LLVM Phabricator instance.

[libc++][format][4/6] Improve formatted_size.
ClosedPublic

Authored by Mordante on Sep 26 2021, 7:49 AM.

Details

Reviewers
ldionne
vitaut
Group Reviewers
Restricted Project
Commits
rGfb9a692be5dd: [libc++][format][4/6] Improve formatted_size.
Summary

Use a specialized "buffer" to count the number of insertions instead of
using a string as storage type.

Depends on D110497.

Diff Detail

Event Timeline

Mordante requested review of this revision.Sep 26 2021, 7:49 AM
Mordante created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptSep 26 2021, 7:49 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Mordante planned changes to this revision.Sep 26 2021, 10:00 AM

Needs to be updated due to the removal of __put in D110495.

Mordante updated this revision to Diff 378447.Oct 9 2021, 7:04 AM

Rebased and adjusted to changes in __put.

Mordante updated this revision to Diff 380185.Oct 16 2021, 8:00 AM

Rebased to test with wchar_t changes.

Mordante planned changes to this revision.Nov 2 2021, 10:57 AM
Mordante updated this revision to Diff 387092.Nov 14 2021, 8:01 AM

Rebased and make changes based on D112361.

In general looks good, just a few minor comments inline. However, the discussion in earlier diffs may affect __formatted_size_buffer API.

libcxx/include/__format/buffer.h
265

Why not const _CharT*?

269

Why &&-qualified?

libcxx/include/format
365–385

I suggest adding __vformatted_size and force-inlining formatted_size to reduce template bloat.

Also why move this block?

Mordante marked an inline comment as done.Nov 25 2021, 11:30 AM
Mordante added inline comments.
libcxx/include/__format/buffer.h
269

Not really required here, but to keep consistent with the other "retrieve result" functions in these buffers.

libcxx/include/format
365–385

It's moved so format_to_n can use formatted_size.

Mordante updated this revision to Diff 400011.Jan 14 2022, 8:08 AM
Mordante marked an inline comment as done.

Rebased and addresses last review comment.

Mordante marked 2 inline comments as done.Jan 14 2022, 8:10 AM
Mordante added inline comments.
libcxx/include/format
365–385

Since formatted_size no longer depends on format_to_n I moved it to its original location.

Herald added a project: Restricted Project. · View Herald TranscriptMar 26 2022, 9:54 AM
ldionne accepted this revision.Apr 7 2022, 12:51 PM
ldionne added inline comments.
libcxx/include/__format/buffer.h
256
libcxx/include/format
546

This applies to other free function calls in this diff as well.

This revision is now accepted and ready to land.Apr 7 2022, 12:51 PM
Mordante marked 2 inline comments as done.Apr 8 2022, 10:22 AM
Mordante updated this revision to Diff 421610.Apr 8 2022, 12:05 PM

Rebase and address review comments.

This revision was automatically updated to reflect the committed changes.