[libcxx-commits] [PATCH] D137476: [libc++] Add utilites for instantiating functions with multiple types

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.copy/ranges.copy.pass.cpp:111
+                                    meta::proxy_cpp20_input_iterator_list<int*>>>{},
+      []<class In, class Out>() { test_sentinels<In, Out>(); });
+
----------------
Mordante wrote:
> philnik wrote:
> > Mordante wrote:
> > > For a unit test this feels too clever for me.
> > What exactly do you mean? Concatenating two cartesian products, the `caretesian_product_t` itself, or maybe something completely different?
> Mainly the `cartesian_product_t`. I'm not sure everybody knows what a Cartesian product is. Note that I'm perfectly fine with the Cartesian product in our benchmarks, there it's easy to observe what it does by looking at the output. Here there is not output. So the question is, are you sure the test is correct?
> 
> So here I feel writing out the code, using helper functions, makes it easier to understand what happens. Which makes it easier to validate whether the test is correct.
> 
> 
I think most people reading libc++ tests know what a cartesian product is. Even if you don't know what it is, it's about as hard to find out as what a `proxy_iterator` is, which you also have to know to understand the test. Having the cartesion product spelled out also makes it easier to find out what the intention of the code is. You don't have to go through multiple functions to find out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137476