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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante planned changes to this revision.
Mordante added a comment.

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



================
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); }
----------------
Quuxplusone wrote:
> 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.
I agree, this feels odd. But it's what the Standard mandates. I'm tempted to file an LWG-issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112368