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

[Hui via Phabricator via libcxx-commits]

huixie90 accepted this revision.
huixie90 added inline comments.


================
Comment at: libcxx/include/__algorithm/partial_sort.h:57
+template <class _AlgPolicy, class _RandomAccessIterator, class _Sentinel>
+inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17
+void __maybe_randomize(_RandomAccessIterator __first, _Sentinel __last) {
----------------
question. why do we need `inline` here as it is already a template (we don't need to worry about ODR violation)?


================
Comment at: libcxx/include/__algorithm/partial_sort.h:37
+_LIBCPP_HIDE_FROM_ABI constexpr
+_RandomAccessIterator __partial_sort_impl(
+    _RandomAccessIterator __first, _RandomAccessIterator __middle, _Sentinel __last, _Compare __comp) {
----------------
var-const wrote:
> huixie90 wrote:
> > A general question on these `__impl` functions. There are lots of double underscore `impl` functions in the algorithms that are shared between different algorithms. e.g. `__copy`. these functions are not constrained and sometimes I am unsure what condition these functions can be used. I guess some of them are not meant to be used outside the file thus omitting any constraint checks. But it is not easy to find out if it is ok to use these `impl` functions. I am wondering if it is a good idea to add some comments or static_assert on these functions?
> I think that these functions generally start their life being intended to use only within their file, but later end up being used from outside. The fact that they aren't constrained might be deliberate -- any checks are supposed to have already been performed by the public interface. Comments are definitely useful; `static_assert`s might be useful as well.
Can we have a naming convention for this? It would be good to distinguish the purpose of `__partial_sort` and `__partial_sort_impl` at the first glance.

 It would be good to distinguish 
1. helper function purely for implementation of the classic algorithm (perhaps with some assumptions that the iterators are classic iterators)
2. helper function purely for the implementation of ranges algorithm (perhaps with assumptions that iterators are c++20 iterators)
2. helper function that is meant to be shared between the classic algorithm and the ranges algorithm
3. helper function can be reused any other algorithms

The reason why I was asking is that when I implemented other algorithms and I'd like to call `copy`, I was puzzled if I should call `std::__copy` or `std::__copy_impl`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128744