Page MenuHomePhabricator

[libc++][format] Implements LWG3892.
Needs ReviewPublic

Authored by Mordante on Sat, Mar 11, 6:55 AM.

Details

Reviewers
ldionne
vitaut
EricWF
Group Reviewers
Restricted Project
Summary

This LWG issue is based on the discussion regarding

P2733R3 Fix handling of empty specifiers in std::format

This paper was disussed and changed a few times in LEWG during the
Issaquah meeting. The paper was not voted in, instead LEWG asked for
a DR against C++26.

This LWG issue contains the direction voted by LEWG. This issue has not
been voted in yet. However it fixes some of the defencies on the
container based formatting. Without this fix the range-default-formatter
for strings looks bad when used in containers.

The changes of this issue match the intended changes of P27333.

type fmt before after (if changed)

char {} a
char {:?} 'a'
array<char, 1> {} ['a']
array<char, 1> {::} [a]
array<char, 1> {::c} [a]
array<char, 1> {::?} ['a']
map<char, char> {} {a: a} -> {'a': 'a'}
map<char, char> {::} {'a': 'a'}
set<char> {} {'a'}
set<char> {::} {a}
set<char> {::c} {a}
set<char> {::?} {'a'}
tuple<char> {} ('a')
stack<char> {} ['a']
stack<char> {::} [a]
stack<char> {::c} [a]
stack<char> {::?} ['a']
array<array<char, 1>, 1> {} [[a]] -> {'a': 'a'}
array<array<char, 1>, 1> {::} [['a']]
array<array<char, 1>, 1> {:::} [[a]]
array<array<char, 1>, 1> {:::c} [[a]]
array<array<char, 1>, 1> {:::?} [['a']]
array<tuple<char>, 1> {} [(a)] -> [('a')]
tuple<tuple<char>> {} ((a)) -> (('a'))
tuple<array<char, 1>> {} ([a]) -> (['a'])

Note the optimization text as mentioned in the tuple formatter can't be
done. The call to parse may affect the formatter so its state needs to
be preserved.

Diff Detail

Event Timeline

Mordante created this revision.Sat, Mar 11, 6:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptSat, Mar 11, 6:55 AM
Mordante published this revision for review.Sun, Mar 12, 7:10 AM
Mordante added reviewers: ldionne, vitaut.
Herald added a project: Restricted Project. · View Herald TranscriptSun, Mar 12, 7:10 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne accepted this revision as: ldionne.Tue, Mar 14, 8:59 AM

LGTM, it would be great if @vitaut could have a quick look as well.

libcxx/test/support/format.functions.common.h
52
EricWF accepted this revision as: EricWF.Fri, Mar 17, 2:34 PM
EricWF added a subscriber: EricWF.

Nice tests. Big fan of the work done in this patch.

I haven't reviewed it for correctness w.r.t. the format spec, but in general I think it looks good.

libcxx/include/__format/range_formatter.h
265

Is there a reason for always doing this rather than just guarding it in a macro?