[libcxx-commits] [PATCH] D151265: [libc++] Introduce __for_each_segment and use it in copy/move

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

Would it make sense to add benchmarks? The number in D151274 <https://reviews.llvm.org/D151274> look quite impressive.



================
Comment at: libcxx/include/__algorithm/copy.h:53
+    struct _CopySegment {
+      _OutIter& __result_;
 
----------------
I'm not too thrilled with this part of the changes. Is it possible to remove this helper struct and use a lambda instead?

```
std::__for_each_segment(__first, __last, _CopySegment, [&__result](_Traits::__local_iterator __lfirst, _Traits::__local_iterator __llast) mutable
{
  __result_ = std::__copy<_AlgPolicy>(__lfirst, __llast, std::move(__result_)).second;
});
```
If so the same for `move`.


================
Comment at: libcxx/include/__algorithm/for_each_segment.h:29
+
+  // We are in a single segment, so we might not be at the begin or end
+  if (__sfirst == __slast) {
----------------
ldionne wrote:
> 
Thanks a lot for the comments, that makes understanding the code a lot easier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151265