[libcxx-commits] [PATCH] D112368: [libc++][format] Move iterators when needed.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added a comment.

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)?



================
Comment at: libcxx/include/__format/format_context.h:96
 #endif
-  _LIBCPP_HIDE_FROM_ABI iterator out() { return __out_it_; }
-  _LIBCPP_HIDE_FROM_ABI void advance_to(iterator __it) { __out_it_ = __it; }
+  _LIBCPP_HIDE_FROM_ABI iterator out() { return _VSTD::move(__out_it_); }
+  _LIBCPP_HIDE_FROM_ABI void advance_to(iterator __it) { __out_it_ = _VSTD::move(__it); }
----------------
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112368/new/

https://reviews.llvm.org/D112368