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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

Can you add a release note? This is a nice improvement. I'd also like us to add a libc++ specific test that counts the number of comparisons. It can be pretty basic, but I'd like to regression-lock this improvement.

I tried poking holes into the implementation by grabbing the patch, making small changes (e.g. introducing off-by-ones) and literally every single line I touched would make the tests fail. In other words, I am fairly confident this is correct, not because I trust myself to validate the algorithm itself, but because I think our test coverage in that area is pretty solid.

LGTM with a regression test that checks for the number of comparisons and my `__assert` comment.



================
Comment at: libcxx/include/__algorithm/sift_down.h:13
 #include <__config>
+#include <__debug>
 #include <__iterator/iterator_traits.h>
----------------
You want to include `<__assert>` here instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118003