[libcxx-commits] [PATCH] D127211: [libc++] Implement ranges::{reverse, rotate}_copy

Konstantin Varlamov via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Jul 7 17:53:11 PDT 2022


var-const requested changes to this revision.
var-const added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/__algorithm/ranges_reverse_copy.h:51
+  reverse_copy_result<borrowed_iterator_t<_Range>, _OutIter> operator()(_Range&& __range, _OutIter __result) const {
+    auto __ret = ranges::copy(__range | views::reverse, std::move(__result));
+    return {ranges::next(ranges::begin(__range), ranges::end(__range)), std::move(__ret.out)};
----------------
While this allows for a very elegant implementation, the view is way more heavyweight than a simple eager implementation would be. I'd rather avoid the dependency: it adds compilation time overhead, gives users a way to accidentally depend on something they don't include directly, and it might be less efficient (caching logic looks suspicious in that regard).

Whichever approach we settle upon will likely affect some other algorithms as well, so let's check with @ldionne.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127211



More information about the libcxx-commits mailing list