[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:
> 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?


================
Comment at: libcxx/test/support/type_algorithms.h:134
+using signed_integer_types =
+    type_list<short,
+              int,
----------------
Mordante wrote:
> `signed char` is a a signed integer type too.
Good catch!


================
Comment at: libcxx/test/support/type_algorithms.h:151
+
+using unsigned_integer_types = wrap_list_t<make_unsigned_t, signed_integer_types>;
+
----------------
Mordante wrote:
> This too look a bit clever. Here it's quite obvious, but in a different use-case it might be less obvious.
I think in some cases it makes sense, since i.e. if we add another integral type, like `__int256_t` or something like that, we can't forget to add the unsigned version. The same reasoning applies to the proxy iterator lists. But I agree we should consider whether we use it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137476