[libcxx-commits] [PATCH] D124328: [libc++] Forward more often to memmove in copy

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const accepted this revision as: var-const.
var-const added a comment.

In D124328#3480095 <https://reviews.llvm.org/D124328#3480095>, @philnik wrote:

> IIUC you suggestion would be to add non-trivial assignment operators, but that would disable the optimization. The optimization should only apply if it's invisible to the user.

Ah, I think you're right.



================
Comment at: libcxx/include/__algorithm/copy.h:105
+pair<reverse_iterator<reverse_iterator<_InIter> >, reverse_iterator<reverse_iterator<_OutIter> > >
+__copy_impl(reverse_iterator<reverse_iterator<_InIter>> __first,
+            reverse_iterator<reverse_iterator<_Sent>> __last,
----------------
philnik wrote:
> var-const wrote:
> > Question: why wouldn't the other `reverse_iter` overload that calls `__unwrap_iter` handle the case?
> With a `reverse_iterator<reverse_iterator<int*>>` the other overload would have `_InIter = reverse_iterator<int*>`. This is only a random access iterator and not a contiguous iterator, so it wouldn't get unwrapped.
Can you please add a comment to that effect in this function? The fact that `__unwrap_iter` only deals with contiguous iterators is really not obvious without looking at its implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124328