[libcxx-commits] [PATCH] D130599: [libc++] Implement ranges::remove_copy{, _if}

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/include/algorithm:832
+             indirect_binary_predicate<ranges::equal_to, projected<I, Proj>, const T*>
+    constexpr remove_copy_result<I, O>
+      remove_copy(I first, S last, O result, const T& value, Proj proj = {});                      // Since C++20
----------------
Missing the definition of `remove_copy_result`.


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.remove/ranges_remove_copy.pass.cpp:49
+static_assert(!HasRemoveCopyIt<int*, int*, OutputIteratorNotIndirectlyWritable>);
+static_assert(!HasRemoveCopyIt<int*, int*, OutputIteratorNotInputOrOutputIterator>);
+
----------------
Needs tests for `indirectly_copyable` and `indirect_binary_predicate`.


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.remove/ranges_remove_copy.pass.cpp:176
+
+  // Complexity: Exactly last - first applications of the corresponding predicate and any projection.
+
----------------
Need to count invocations of the predicate as well.


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.remove/ranges_remove_copy_if.pass.cpp:61
+// !input_iterator<I>
+static_assert(!HasRemoveCopyIfIter<InputIteratorNotDerivedFrom>);
+static_assert(!HasRemoveCopyIfIter<cpp20_output_iterator<int*>>);
----------------
This should be made consistent with the other test file.


================
Comment at: libcxx/test/std/algorithms/alg.modifying.operations/alg.remove/ranges_remove_copy_if.pass.cpp:223
 
 constexpr bool test() {
+  withAllPermutationsOfInIterOutIter<std::type_identity_t>();
----------------
These two files should ideally be almost mirror versions of each other.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130599