[libcxx-commits] [PATCH] D128115: [libc++][ranges] Implement modifying heap algorithms:

Konstantin Varlamov via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Fri Jun 17 22:58:38 PDT 2022

var-const added inline comments.

Comment at: libcxx/include/__algorithm/make_heap.h:26
 template <class _Compare, class _RandomAccessIterator>
-__make_heap(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compare __comp)
-    typedef typename iterator_traits<_RandomAccessIterator>::difference_type difference_type;
-    difference_type __n = __last - __first;
-    if (__n > 1)
-    {
-        // start from the first parent, there is no need to consider children
-        for (difference_type __start = (__n - 2) / 2; __start >= 0; --__start)
-        {
-            _VSTD::__sift_down<_Compare>(__first, __comp, __n, __first + __start);
-        }
+void __make_heap(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compare __comp) {
Note: a few of these existing internal functions were missing the `_LIBCPP_HIDE_FROM_ABI` annotation. I presume adding it won't cause any problems?

Comment at: libcxx/include/__algorithm/make_heap.h:41
+void __make_heap_impl(_RandomAccessIterator __first, _RandomAccessIterator __last, _Compare& __comp) {
+  using _Comp_ref = typename __comp_ref_type<_Compare>::type;
+  std::__make_heap<_Comp_ref>(std::move(__first), std::move(__last), __comp);
The main purpose of this function is to avoid leaking the `__comp_ref_type` quirk from this file (similarly for the other heap headers).

Comment at: libcxx/include/__algorithm/ranges_pop_heap.h:42
+  _Iter __pop_heap_fn_impl(_Iter __first, _Sent __last, _Comp& __comp, _Proj& __proj) {
+    _LIBCPP_ASSERT(__first != __last, "The heap given to pop_heap must be non-empty");
+    auto __last_iter = ranges::next(__first, __last);
Note: the Standard specifies that the range has to be non-empty in [[ http://eel.is/c++draft/pop.heap#2 | pop.heap ]]:
Preconditions: The range [first, last) is a valid **non-empty** heap
However, our existing non-ranges version of `pop_heap` is simply a no-op for an empty range, and the same is true for GCC and MSVC. Please let me know whether you think being strict with this check adds any benefit.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list