[libcxx-commits] [PATCH] D121523: [libc++][ranges] Implement ranges::count{, _if}

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const requested changes to this revision.
var-const added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/__algorithm/ranges_count_if.h:49
+  _LIBCPP_HIDE_FROM_ABI constexpr
+  iter_difference_t<_Iterator> operator()(_Iterator __first, _Sentinel __last,
+                                          _Predicate __pred, _Projection __proj = {}) const {
----------------
philnik wrote:
> var-const wrote:
> > Question: hmm, there are no constraints on `iter/range_difference_t` (in the Standard), but the implementation has to make certain assumptions about the type. I wonder if it would be a good idea to have a constraint on that as well (unless this constraint comes from somewhere implicitly). What do you think?
> There are constraints through `input_iterator` -> `input_or_output_iterator` -> `weakly_incrementable`. Specifically `__signed_integer_like`.
Ah, thanks for explaining.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.count/ranges.count.pass.cpp:245
+      int a[] = {5, 5, 4, 3, 2, 1};
+      std::same_as<signed char> auto ret = std::ranges::count(DiffTypeIterator(a), DiffTypeIterator(a + 6), 4);
+      assert(ret == 1);
----------------
Nit: `s/auto/decltype(auto)/` (here and below).


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.count/ranges.count_if.pass.cpp:152
+      auto range = std::ranges::subrange(It(a.data()), Sent(It(a.data() + a.size())));
+      auto ret = std::ranges::count(range, 5);
+      assert(ret == 4);
----------------
This should be `count_if`, not `count`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121523