[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:
> Quuxplusone wrote:
> > 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`?
> Write a test that would fail were this to be called (failing only under ASAN or MSAN is fine).
> Write a test that would fail were this to be called

@EricWF: I don't understand that comment. My intention is that `__floyd_sift_down` is //never// called with `__len < 2`. If it were ever called with `__len < 2`, bad stuff would happen. In other words, what we really mean here is something like `[[pre: __len >= 2]]` or `__builtin_assume(len >= 2)`, but libc++ traditionally hasn't used //those//, so I just reached for `_LIBCPP_ASSERT` as the handiest documentation tool. As I suggested above, I could just replace it with a comment `// Precondition: __len >= 2`, but I'm soliciting better suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118003