[libcxx-commits] [PATCH] D123681: [libc++] Implement ranges::equal

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

In general looks good, but some minor issues.



================
Comment at: libcxx/include/__algorithm/ranges_equal.h:93
+    if constexpr (sized_range<_Range1> && sized_range<_Range2>) {
+      if (ranges::size(__range1) != ranges::size(__range2))
+        return false;
----------------
Interesting that the standard uses `distance` instead of `size`.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.equal/ranges.equal.pass.cpp:89
+    }
+  }
+
----------------
Please add tests with the same size, but different values.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.equal/ranges.equal.pass.cpp:106
+      assert(ret);
+    }
+  }
----------------
Here too I like to see a test which returns `false` for a `search`.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.equal/ranges.equal.pass.cpp:196
+  { // check that the complexity requirements are met
+    { // different size
+      {
----------------
This misses the tests when the input isn't a `size_sentinel_for` or a `sized_range`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123681