[libcxx-commits] [PATCH] D127577: [libc++] Fix std::lower_bound with C++20-hostile iterators

Nikolas Klauser via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Sun Jun 12 09:57:17 PDT 2022

philnik added inline comments.

Comment at: libcxx/include/__iterator/distance.h:13
+#include <__algorithm/generic_algorithm_helper.h>
 #include <__config>
EricWF wrote:
> I would love a more descriptive name for this header.
Do you have a good suggestion? I think `namespace` is a lot worse than what I've got currently.

Comment at: libcxx/include/__iterator/distance.h:109
 typename iterator_traits<_Iter>::difference_type __ranges_distance(_Iter __first, _Sent __second) {
+  if constexpr (__ns == _Namespace::_Ranges) {
EricWF wrote:
> Can we lose this `#ifdef`, since I assume we are not passing `_Ranges` when we don't have complete ranges?
> And if we are passing `_Ranges` we shouldn't be silently falling back to `std::`
`ranges::distance` has to exist for `if constexpr` to be happy. I've added a `static_assert(__ns == _Namespace::_Std)`.

Comment at: libcxx/test/std/algorithms/alg.sorting/alg.binary.search/lower.bound/lower_bound.pass.cpp:25
+template <class Iter>
+concept HasLowerBound = requires(Iter iter, Iter sent) { std::lower_bound(iter, sent, 0); };
EricWF wrote:
> Can we write this without concepts so we can test it in all the dialects it affects?
> Also, I don't think this tests anything. It's my understanding that concepts can't detect failures outside of the immediate context. So because the call itself is well formed, this test will always pass.
What exactly do you mean? C++20 is the only dialect where it is interesting AFAICT. Otherwise we always call `std::advance` and `std::distance` anyways.

For `HasLowerBound` to be true the instantiation of `std::lower_bound` has to be valid. So if `HasLowerBound` is true `int main() { std::lower_bound(iter, sent, 0); }` has to compile.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list