[libcxx-commits] [PATCH] D128983: [libcxx][ranges] implement `std::ranges::set_difference`

[Hui via Phabricator via libcxx-commits]

huixie90 added inline comments.


================
Comment at: libcxx/test/std/algorithms/alg.sorting/alg.set.operations/set.difference/ranges_set_difference.pass.cpp:244
+constexpr void withAllInIter1InIter2Iterators() {
+  withAllInIter1Iterators<cpp20_input_iterator<int*>, OutIter>();
+  withAllInIter1Iterators<forward_iterator<int*>, OutIter>();
----------------
var-const wrote:
> Question: does `cpp17_input_iterator` not work? If yes, can you add a short comment indicating why it doesn't work, to make it clear it wasn't forgotten but deliberately omitted?
The algorithm requires the input to be `std::input_iterator` which is a c++ 20 concept. cpp_17_input_iterator might or might not work, depending on whether or not it models C++20 input_iterator concept.

IMO it doesn't add too much in the test, and at the same time it couples the test to the implementation of `cpp_17_input_iterator`. Whether or not model c++ 20's input_iterator might change for that particular class. 

If we really want to test it must work, we should probably create another class `cpp17_and_cpp20_input_iterator`

What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128983