[libcxx-commits] [PATCH] D126529: [libc++] Implement ranges::find_first_of

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik marked 8 inline comments as done.
philnik added inline comments.


================
Comment at: libcxx/include/__algorithm/ranges_find_first_of.h:43
+    for (; __first1 != __last1; ++__first1) {
+      for (auto __j = __first2; __j != __last2; ++__j) {
+        if (std::invoke(__pred, std::invoke(__proj1, *__first1), std::invoke(__proj2, *__j)))
----------------
var-const wrote:
> Why are we creating a new variable instead of incrementing `__first2`, which would be consistent with `__first1`?
Because we go a few times over `[__first2, __last2)`.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.find.first.of/ranges.find_first_of.pass.cpp:170
+
+  { // check that std::invoke is used
+    struct S1 {
----------------
var-const wrote:
> Great test, can you please check if there are any other range algorithm tests where a similar test would be valuable?
I think I mostly added this test to other ranges algorithms. A few of the first ones might be missing a few tests, but I think that is a problem for later. They should probably be refactored and extended at some point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126529