[PATCH] D52697: Bug 39129: Speeding up partition_point/lower_bound/upper_bound/ by using unsigned division by 2 when possible.

[Eric Fiselier via Phabricator]

EricWF added a comment.

Other than the comment about the test, this LGTM.



================
Comment at: test/libcxx/algorithms/half_positive.pass.cpp:31
+
+        _LIBCPP_CONSTEXPR type max_v = std::numeric_limits<type>::max();
+        _LIBCPP_CONSTEXPR type r = std::__half_positive(max_v);
----------------
dyaroshev wrote:
> EricWF wrote:
> > Other libraries run our test suite, so we can't use internal macros.
> > 
> > Could you please write one set of tests that run at runtime, and another set that uses constexpr and static assert.
> > And then guard the constexpr tests under `#if TEST_STD_VER >= 11`.
> > 
> > 
> I can but how they can run these tests? There are public tests right? And these are private tests.
> Other libraries cannot run this test because they do not have __half_positive not because they do not have a macro? Or am I missing smth?
Good point. These tests are specific to libc++. My mistake.

We still don't use `_LIBCPP_CONSTEXPR` to declare constexpr variables in tests. It's just not how that's supposed to be used.
Plus, for the constexpr tests you want to use `static_assert` and not assert.

Testing both runtime and compile time behavior will require a little extra duplication, but it's probably worth it.

(If you're curious here's how I would have written it: https://gist.github.com/EricWF/df7e02ca931075b62caac587b7c0cf27)


https://reviews.llvm.org/D52697