Please use GitHub pull requests for new patches. Phabricator shutdown timeline

Paths

Table of Contentst

Differential D110498

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

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

Tags

Subscribers

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

Repository: rG LLVM Github Monorepo

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

Herald added a subscriber: libcxx-commits. · View Herald Transcript

Harbormaster completed remote builds in B125759: Diff 375105.Sep 26 2021, 7:50 AM

Mordante added a child revision: D110499: [libc++][format][5/6] Improve format_to_n..Sep 26 2021, 8:05 AM

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

Rebased and adjusted to changes in __put.

Harbormaster completed remote builds in B127936: Diff 378447.Oct 9 2021, 7:59 AM

Rebased to test with wchar_t changes.

Harbormaster completed remote builds in B129191: Diff 380185.Oct 16 2021, 8:50 AM

Mordante planned changes to this revision.Nov 2 2021, 10:57 AM

Rebased and make changes based on D112361.

Harbormaster completed remote builds in B134140: Diff 387092.Nov 14 2021, 8:49 AM

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
259	Why not `const _CharT*`?
263	Why `&&`-qualified?
libcxx/include/format
367–387	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
263	Not really required here, but to keep consistent with the other "retrieve result" functions in these buffers.
libcxx/include/format
367–387	It's moved so `format_to_n` can use `formatted_size`.

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
367–387	Since `formatted_size` no longer depends on `format_to_n` I moved it to its original location.

Harbormaster completed remote builds in B143405: Diff 400011.Jan 14 2022, 9:51 AM

Rebased.

Herald added a project: Restricted Project. · View Herald TranscriptMar 26 2022, 9:54 AM

Harbormaster completed remote builds in B156416: Diff 418402.Mar 26 2022, 11:13 AM

ldionne accepted this revision.Apr 7 2022, 12:51 PM

ldionne added inline comments.

libcxx/include/__format/buffer.h
250
libcxx/include/format
548	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

Rebase and address review comments.

Harbormaster completed remote builds in B158765: Diff 421610.Apr 8 2022, 2:44 PM

Closed by commit rGfb9a692be5dd: [libc++][format][4/6] Improve formatted_size. (authored by Mordante). · Explain WhyApr 9 2022, 12:36 AM

This revision was automatically updated to reflect the committed changes.

Mordante added a commit: rGfb9a692be5dd: [libc++][format][4/6] Improve formatted_size..

Revision Contents
Changeset List

Path

Size

libcxx/

include/

__format/

23 lines

51 lines

Diff 421699

libcxx/include/__format/buffer.h

Loading...

libcxx/include/format

Loading...