[libcxx-commits] [PATCH] D129414: [libc++][ranges][NFC] Consolidate range algorithm tests with non-boolean predicates.

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added a comment.

My plan is to omit these tests from the remaining unimplemented range algorithms going forward, but leave the existing tests be for now (I'll clean them up later once we're done with the initial implementation of the algorithms).

The primary motivation for this patch is to reduce the amount of drudgery when implementing a new algorithm. I also think that having these closely related tests consolidated together makes them more readable.

A potential downside is that we're no longer testing that the algorithms behave correctly. However, I think these checks were redundant -- the probability of an algorithm compiling but producing incorrect results when given e.g. a non-`bool` predicate seems to be essentially zero.



================
Comment at: libcxx/test/std/algorithms/ranges_robust_against_nonbool_predicates.compile.pass.cpp:39
+template <class Func, std::ranges::range Input, class Pred>
+void in_pred(Func&& func, Input& in, Pred& pred) {
+  func(in.begin(), in.end(), pred);
----------------
Originally I tried to have a single overloaded function for each kind of invocation, but I found that there is too much complexity when trying to distinguish between different invocations that have the same number of arguments (especially the unconstrained arguments like predicates, projections and values). Additionally, I think that cataloguing all the different algorithm signatures has some value.


================
Comment at: libcxx/test/std/algorithms/ranges_robust_against_nonbool_predicates.compile.pass.cpp:108
+  // min
+  std::ranges::min(1, 2, binary_pred);
+  std::ranges::min(std::initializer_list<int>{1, 2}, binary_pred);
----------------
I avoided creating helper functions here because their main benefit is taking care of both the `(iterator, sentinel)` and `(range)` overloads which doesn't apply here.


================
Comment at: libcxx/test/std/algorithms/ranges_robust_against_nonbool_predicates.compile.pass.cpp:125
+  //in2_pred(std::ranges::search, in, in2, binary_pred);
+  //std::ranges::search_n(in.begin(), in.end(), count, x, binary_pred);
+  //std::ranges::search_n(in, count, x, binary_pred);
----------------
In general, I avoided creating a helper function unless there were at least 3 uses of it within the file.


================
Comment at: libcxx/test/std/algorithms/ranges_robust_against_omitting_invoke.compile.pass.cpp:159
+    std::array out_transform = {false, true, true};
+    in_out_pred(std::ranges::transform, in, out_transform.begin(), &Foo::unary_pred, &Bar::val);
+  }
----------------
I'm reusing the name "predicate" here even though it's arguably a little hacky.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129414