[libcxx-commits] [PATCH] D117817: [libc++][ranges] Implement ranges::mismatch

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__algorithm/ranges_mismatch.h:47
+    while (__first1 != __last1 && __first2 != __last2) {
+      if (!invoke(__pred, invoke(__proj1, *__first1), invoke(__proj2, *__first2)))
+        break;
----------------
Quuxplusone wrote:
> `_VSTD::invoke` throughout; and please add a regression test involving `Holder<Incomplete>*`.
> https://quuxplusone.github.io/blog/2019/09/26/uglification-doesnt-stop-adl/
Using `Holder<Incomplete>*` currently errors in `input_range`, so I won't put a regression test in since fixing `input_range` is out of scope for this PR.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/mismatch/ranges_mismatch.pass.cpp:16-30
+template <class Iter1, class Iter2>
+constexpr void test_iterators(Iter1 begin1, Iter1 end1, Iter2 begin2, Iter2 end2, Iter1 expected1, Iter2 expected2) {
+  using Expected = std::ranges::mismatch_result<Iter1, Iter2>;
+  std::same_as<Expected> auto ret = std::ranges::mismatch(begin1, end1, begin2, end2);
+  assert(ret.in1 == expected1);
+  assert(ret.in2 == expected2);
+}
----------------
Quuxplusone wrote:
> I think this layer of indirection is more confusing than helpful. Strongly recommend inlining these assertions directly into `test_iters`, `test_different_lengths`, and `test_borrowed_range` — I think that will probably even shorten the code! 
Veto on doing this for `test_ranges`. For that I would either have to inline the return type check as well or explicitly name the template parameters. Both of which make the code definitely less readable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117817