[libcxx-commits] [PATCH] D128744: [libc++][ranges] Implement `ranges::partial_sort`.

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/include/__algorithm/basic_operations.h:30
+template <>
+struct _BasicOperations<_STLClassicAlgorithms> {
+  // iter_swap
----------------
var-const wrote:
> philnik wrote:
> > Isn't this pretty much exactly the same as `_StdIterOps` and `_RangesIterOps` (in `__algorithm/iterator_operations.h`), just with a bit more boiler-plate?
> Sorry, I completely missed that patch. You're right, it's pretty much the same idea. The only difference of any significance is that a tag can be used to make arbitrary decisions -- or rather, passing a generic tag instead of a class containing a set of operations makes it clearer that this is intended to be a generic mechanism, not limited to just iterator operations. I could imagine something like:
> ```
> template <class _Tag, class _Iter>
> decltype(auto) __foo_impl(_Iter __b, _Iter __e) {
>   // ...
>   if constexpr (same_as<_Tag, _RangeAlgorithms>)
>     return __e;
> }
> ```
> (Of course, this could be done as `if constexpr(same_as<_IterOps, _RangesIterOps>)` too, but I think it is a bit less readable)
> 
> In either case, there's definitely duplication here. Let's discuss whether being more generic with tags is worth the boilerplate, and I'll consolidate the approach in this patch (whichever we decide).
> 
> (By the way, the `static constexpr auto advance = ranges::advance;` idea is pretty neat)
I took a stab at consolidating the two -- the only change I did (other than adding new functions) is using the tag approach for reasons outlined above (more generic). Please let me know what you think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128744