[libcxx-commits] [PATCH] D118003: [libc++] Floyd's improvement to pop_heap

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__algorithm/sift_down.h:83
+    using difference_type = typename iterator_traits<_RandomAccessIterator>::difference_type;
+    _LIBCPP_ASSERT(__len >= 2, "shouldn't be called unless __len >= 2");
+
----------------
EricWF wrote:
> LIBCPP_ASSERT is not meant to guard against internal library logic errors. 
You (or anyone) got any better suggestions? Should this just be a comment `// __len >= 2 at this point`?


================
Comment at: libcxx/include/__algorithm/sift_down.h:101
+        *__hole = std::move(*__child_i);
+        __hole = std::move(__child_i);
+
----------------
EricWF wrote:
> You move __child_i here, but you reference it again at the start of the next loop iteration
Yikes! Thanks, fixed in 2b0ec7ca44ea227665f25b7dc3a3c097f16673cb — but tomorrow I'll think about ways to test for this. I wonder if the test iterators (like `forward_iterator<T>`) should be augmented to globally detect use-after-move.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118003