[libcxx-commits] [PATCH] D116268: [libc++][ranges] Add indirectly_comparable concept

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__iterator/iter_swap.h:102-103
 
+template <class _I1, class _I2, class _Rp, class _P1 = identity, class _P2 = identity>
+concept indirectly_comparable = indirect_binary_predicate<_Rp, projected<_I1, _P1>, projected<_I2, _P2>>;
+
----------------
Quuxplusone wrote:
> This code looks fine, but why on earth is it in `iter_swap.h`?
> It should be in `__iterator/concepts.h`, or (because I think that would create a cycle) just put it in `__iterator/indirectly_comparable.h`.
> 
> Somewhat tangential query: Is this the last of the `<iterator>` concepts? or are we still missing some?
There are at least `permutable`, `mergable` and `sortable` missing, probably more.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/indirectcallable/indirectinvocable/indirectly_comparable.compile.pass.cpp:18-19
+
+#include "__iterator/readable_traits.h"
+#include "indirectly_readable.h"
+#include "test_iterators.h"
----------------
Quuxplusone wrote:
> Direct inclusion of `<__iterator/readable_traits.h>` will fail the modules build.
> I've never seen `libcxx/test/support/indirectly_readable.h` before but it looks crazy complicated; can we avoid using it here? I think all you actually need for this test would be something like
> ```
> struct Deref {
>     int operator()(int*) const;
> };
> 
> static_assert(!std::indirectly_comparable<int, int, std::less<int>>);  // not dereferenceable
> static_assert(!std::indirectly_comparable<int*, int*, int>);  // not a predicate
> static_assert( std::indirectly_comparable<int*, int*, std::less<int>>);
> static_assert(!std::indirectly_comparable<int**, int*, std::less<int>>);
> static_assert( std::indirectly_comparable<int**, int*, std::less<int>, Deref>);
> static_assert(!std::indirectly_comparable<int**, int*, std::less<int>, Deref, Deref>);
> static_assert(!std::indirectly_comparable<int**, int*, std::less<int>, std::identity, Deref>);
> static_assert( std::indirectly_comparable<int*, int**, std::less<int>, std::identity, Deref>);
> ```
> And then a subsumption test in each direction:
> ```
> template<class F> requires std::indirectly_comparable<int*, char*, F> && true
> constexpr bool subsumes(F f) { return true; }
> 
> template<class F> requires std::indirect_binary_predicate<F, std::projected<int*, std::identity>, std::projected<char*, std::identity>>
> void subsumes(F f);
> 
> template<class F> requires std::indirect_binary_predicate<F, std::projected<int*, std::identity>, std::projected<char*, std::identity>> && true
> constexpr bool is_subsumed(F f) { return true; }
> 
> template<class F> requires std::indirectly_comparable<int*, char*, F>
> void is_subsumed(F f);
> 
> static_assert(subsumes(std::less<int>()));
> static_assert(is_subsumed(std::less<int>()));
> ```
> https://godbolt.org/z/c9rEh8e8c
Why does it make a difference if there is the `&& true` at the end? Is there some "this has more requirements, so it must be stricter"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116268