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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added inline comments.
Herald added a project: All.


================
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); }
----------------
Mordante wrote:
> Mordante wrote:
> > 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.
> I've decided to file an LWG-issue.
I was writing an LWG issue, however I see some issues. The function `out` is called in the requirements tables http://eel.is/c++draft/tab:formatter.basic, which would require a call to `std::move`, the same for http://eel.is/c++draft/format.context#example-1.

So it might be confusing only the `out_` member is moved-from, but `arg` can be called.

Do you still feel it's worth the effort to file an issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112368