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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__algorithm/ranges_count.h:38
+  iter_difference_t<_Ip> operator()(_Ip __first, _Sp __last, const _Tp& __value, _Proj __proj = {}) const {
+    auto __pred = [&](const auto& __e) { return __e == __value; };
+    return ranges::__count_if_impl(std::move(__first), std::move(__last), __pred, __proj);
----------------
Mordante wrote:
> This differs from the wording `invoke(proj, *i) == value for the overloads with a parameter proj but no parameter pred;`
> Does your code generate the same assembly as the wording in the Standard or does it generate extra overhead?
What exactly do you think differs between my implementation and the standard? (https://godbolt.org/z/GvGvT7fxT) At least my `ranges::count` and `std::count` produce for this example the exact same assembly at `-O1`.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.count/ranges.count.pass.cpp:84
+    {
+      int a[] = {1, 2, 3, 4};
+      auto ret = std::ranges::count(a, a + 4, a + 3, [](int& i) { return &i; });
----------------
Mordante wrote:
> To avoid duplication this line could be placed in its parent block.
Arthur asked me to duplicate it in other tests to have the definition near the usage.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.count/ranges.count_if.pass.cpp:68
+    std::same_as<std::ptrdiff_t> auto ret =
+      std::ranges::count_if(It(a), Sent(It(a + 4)), [](int x) mutable { return x == 4; });
+    assert(ret == 1);
----------------
Mordante wrote:
> Why the `mutable`? I see it used at multiple places.
I think that's a leftover from some iteration of this test.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.count/ranges.count_if.pass.cpp:126
+  {
+    // check that 0 is returned with no match
+    {
----------------
Mordante wrote:
> This comment is wrong.
What's wrong with the comment? It checks that 0 is returned when there is no match?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121523