[libcxx-commits] [PATCH] D138268: [libcxx] Fix std::equal not accepting volatile types by refactoring __equal_to

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

The implementation changes look pretty much good to me, but the test needs some more work.



================
Comment at: libcxx/include/__algorithm/comp.h:20-22
 // I'd like to replace these with _VSTD::equal_to<void>, but can't because:
 //   * That only works with C++14 and later, and
 //   * We haven't included <functional> here.
----------------
I don't think this comment makes sense anymore. Let's remove it.


================
Comment at: libcxx/include/__algorithm/equal.h:36
 equal(_InputIterator1 __first1, _InputIterator1 __last1, _InputIterator2 __first2) {
-  typedef typename iterator_traits<_InputIterator1>::value_type __v1;
-  typedef typename iterator_traits<_InputIterator2>::value_type __v2;
-  return _VSTD::equal(__first1, __last1, __first2, __equal_to<__v1, __v2>());
+  return _VSTD::equal(__first1, __last1, __first2, __equal_to());
 }
----------------
While we're refactoring the code we might as well replace the `_VSTD`s with `std`.


================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.equal/equal.pass.cpp:29-32
+#if defined(__GNUC__) && !defined(__clang__)
+#  pragma GCC diagnostic push
+#  pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
+#endif
----------------



================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.equal/equal.pass.cpp:47-49
+#if defined(__GNUC__) && !defined(__clang__)
+#  pragma GCC diagnostic pop
+#endif
----------------



================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.equal/equal.pass.cpp:70
 
 
 int main(int, char**)
----------------
I think it would make more sense factor out a function like this
```lang=c++
template <class Iter1, class Iter2 = Iter1>
TEST_CONSTEXPR_CXX20 test_iterator() {
  int range1[] = {0, 1, 2, 3, 4, 5};
  int range2[] = {0, 1, 2, 5, 4, 5};

  assert(std::equal(Iter1(range1), Iter1(range1 + 6), Iter2(range1)));
  assert(!std::equal(Iter1(range1), Iter1(range1 + 6), Iter2(range2)));

#if TEST_STD_VER >= 14
  assert(std::equal(Iter1(range1), Iter1(range1 + 6), Iter2(range1), Iter2(range1 + 6)));
  assert(!std::equal(Iter1(range1), Iter1(range1 + 6), Iter2(range1), Iter2(range1 + 5)));
  assert(!std::equal(Iter1(range1), Iter1(range1 + 6), Iter2(range2), Iter2(range2 + 6)));
#endif
}
```
and then in the `main` just
```lang=c++
test_iterator<cpp17_input_iterator<int*>>();
test_iterator<random_access_iterator<int*>>();
test_iterator<int*>();
test_iterator<const int*>();
etc.
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138268