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

[Mark de Wever via Phabricator via libcxx-commits]

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

LGTM modulo some nits. Make sure it passes the CI before committing. (I know the CI isn't working at the moment.)



================
Comment at: libcxx/docs/Status/RangesPaper.csv:66
 | [iterator.cust.swap]",Zoe Carver,✅
-[common.alg.req]: pt. 3,indirectly_comparable,[projected],Louis Dionne,Not started
+[common.alg.req]: pt. 3,indirectly_comparable,[projected],Louis Dionne,✅
 [common.alg.req]: pt. 4,"| permutable
----------------
Maybe update the name ;-)


================
Comment at: libcxx/include/module.modulemap:597
       module incrementable_traits  { private header "__iterator/incrementable_traits.h" }
+      module indirectly_comparable { private header "__iterator/indirectly_comparable.h" }
       module insert_iterator       { private header "__iterator/insert_iterator.h" }
----------------
I don't see the generated test for this header. 


================
Comment at: libcxx/test/std/iterators/iterator.requirements/indirectcallable/indirectinvocable/indirectly_comparable.compile.pass.cpp:21
+};
+
+static_assert( std::indirectly_comparable<int*, int*, std::less<int>>);
----------------
Is there a reason not to include `static_assert(!std::indirectly_comparable<int, int, std::less<int>>);  // not dereferenceable`?
I would like to see the suggested comment copied.


================
Comment at: libcxx/test/std/iterators/iterator.requirements/indirectcallable/indirectinvocable/indirectly_comparable.compile.pass.cpp:31
+  requires std::indirectly_comparable<int*, char*, F>
+           && true
+constexpr bool subsumes(F) { return true; }
----------------
I think this can use comment, it looks like magic. (And yes I read the reason for it above.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116268