[libcxx-commits] [PATCH] D150146: [libc++] Implement ranges::starts_with

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik requested changes to this revision.
philnik added a comment.
This revision now requires changes to proceed.

Looks pretty good already. Please make sure the CI is green.



================
Comment at: libcxx/include/__algorithm/ranges_starts_with.h:11
+#define _LIBCPP___ALGORITHM_RANGES_STARTS_WITH_H
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
----------------
You have to `#include <__config>`.


================
Comment at: libcxx/include/__algorithm/ranges_starts_with.h:49
+    requires indirectly_comparable<_Iter1, _Iter2, _Pred, _Proj1, _Proj2>
+  _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr bool operator()(
+      _Iter1 __first1,
----------------
Please update the `nodiscard_extension` tests!


================
Comment at: libcxx/include/__algorithm/ranges_starts_with.h:57
+      _Proj2 __proj2 = {}) const {
+    return __starts_with_impl(
+        std::move(__first1), std::move(__last1), std::move(__first2), std::move(__last2), __pred, __proj1, __proj2);
----------------
I think we should just forward this for `ranges::equal` (for random access ranges) or `ranges::mismatch` (otherwise) to grab any optimizations that are done there.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.starts_with/ranges.starts_with.pass.cpp:12
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+// UNSUPPORTED: libcpp-has-no-incomplete-ranges
+
----------------
This isn't a relevant flag anymore.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.starts_with/ranges.starts_with.pass.cpp:32
+
+template <class Iter1, class Sent1, class Iter2, class Sent2 = Iter2>
+constexpr void test_iterators() {
----------------
Looks like clang-format does weird things again. Please update the formatting.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.starts_with/ranges.starts_with.pass.cpp:193-201
+  test_iterators2<forward_iterator<int*>>();
+  test_iterators2<forward_iterator<int*>, sized_sentinel<forward_iterator<int*>>>();
+  test_iterators2<bidirectional_iterator<int*>>();
+  test_iterators2<bidirectional_iterator<int*>, sized_sentinel<bidirectional_iterator<int*>>>();
+  test_iterators2<random_access_iterator<int*>>();
+  test_iterators2<random_access_iterator<int*>, sized_sentinel<random_access_iterator<int*>>>();
+  test_iterators2<contiguous_iterator<int*>>();
----------------
This can be simplified now. See `ranges.copy.pass.cpp` for an example.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.starts_with/ranges.starts_with.pass.cpp:203-223
+  { // check that std::invoke is used
+    struct S {
+      int i;
+
+      constexpr S identity() { return *this; }
+
+      constexpr bool compare(const S& s) { return i == s.i; }
----------------
We have a few `robust_against_foo`tests for this and other things. Please make sure you update them.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.starts_with/ranges.starts_with.pass.cpp:233
+}
\ No newline at end of file

----------------
Newline!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150146