[libcxx-commits] [PATCH] D130758: [libc++][ranges] Implement `ranges::rotate`.

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/include/__algorithm/algorithm_family.h:32
 template <>
 struct _AlgFamily<_RangeAlgPolicy> {
   static constexpr auto __move = ranges::move;
----------------
ldionne wrote:
> Doesn't it become possible to remove this entirely now that `__move` accepts a `_AlgPolicy`? I guess we'd have to also accept an `_AlgPolicy` in `__swap_ranges`, and then we can get rid of this and use `__swap_ranges<_AlgPolicy>` and `__move<_AlgPolicy>` instead?
Done. Also made `ranges::swap_ranges` delegate to the internal function.

(I originally hesitated to do that because the ranges and classic versions of `swap_ranges` take a different number of arguments, but it was easy to accommodate for)


================
Comment at: libcxx/include/__algorithm/algorithm_family.h:34-35
   static constexpr auto __move = ranges::move;
+  // TODO: if using the return type of `ranges::swap_ranges`, note that `result.out` can be different from what would
+  // be returned by `std::swap_ranges`.
+  static constexpr auto __swap_ranges = ranges::swap_ranges;
----------------
ldionne wrote:
> Non blocking, but I don't understand the purpose of this TODO. Isn't that already the case with `__move` right above?
Thanks for flagging this, this has encouraged me to dig a little more and now it looks like the return value is equivalent; removed the TODO.

Most of the time, the range algorithm returns the same iterator value as the classic algorithm plus an additional value (very often the last iterator); for example, the classic version would return `out + N`, whereas the ranges version would return `{last, out + N}`. Thus, it's trivial to convert the ranges result type to classic by simply dropping a value.

In this case, however, the return value is specified differently -- classic version returns `last2` and the range version returns `first2 + M`, where `M` is defined as `min(last1 - first1, last2 - first2)`. I do think, however, that the two are actually equivalent.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130758