Page MenuHomePhabricator

[libc++][format] Move iterators when needed.
Changes PlannedPublic

Authored by Mordante on Oct 23 2021, 10:14 AM.

Details

Reviewers
ldionne
vitaut
Group Reviewers
Restricted Project
Summary

LWG-3539 was already implemented but not marked as done.
LWG-3567 is implemented in this commit.

Diff Detail

Event Timeline

Mordante requested review of this revision.Oct 23 2021, 10:14 AM
Mordante created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptOct 23 2021, 10:15 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

I think this needs a test case. But I also assume that libc++ hasn't yet done a lot of the work to enable move-only iterators, is that right? How big is that work, and do you think it would be reasonable to block this PR until that work gets done (so that this PR could be updated to include a test case involving a move-only output iterator)?

libcxx/include/__format/format_context.h
96

This is correct per the Standard, but also kinda crazy, in that y = x.out() modifies the value of x. It should have been specified with an rvalue-ref-qualifier, like out() &&, right?
No action required, anyway, since this PR implements what the Standard says to do. Just kibitzing.

Mordante planned changes to this revision.Oct 23 2021, 12:10 PM

I'll add tests. I think I can just use custom move only iterator and don't need to depend on move only iterators.
(In fact I expect the most common iterators for this class are copyable.)

libcxx/include/__format/format_context.h
96

I agree, this feels odd. But it's what the Standard mandates. I'm tempted to file an LWG-issue.

I spend some time implementing tests but see some other issues, which I want to investigate before proceeding with this patch.

libcxx/include/__format/format_context.h
96

I've decided to file an LWG-issue.