[libcxx-commits] [PATCH] D145285: [libc++] Refactor __less

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

In D145285#4176202 <https://reviews.llvm.org/D145285#4176202>, @EricWF wrote:

> This cleanup seems incomplete. Why keep `__less` as a template at all given it's never actually used as one?

If you block other reviews on there not being a description, at least read it.



================
Comment at: libcxx/include/__algorithm/includes.h:64
 includes(_InputIterator1 __first1, _InputIterator1 __last1, _InputIterator2 __first2, _InputIterator2 __last2) {
-  return std::includes(
-      std::move(__first1),
-      std::move(__last1),
-      std::move(__first2),
-      std::move(__last2),
-      __less<typename iterator_traits<_InputIterator1>::value_type,
-             typename iterator_traits<_InputIterator2>::value_type>());
+  return std::includes(std::move(__first1), std::move(__last1), std::move(__first2), std::move(__last2), __less<>());
 }
----------------
EricWF wrote:
> Is this correct? What if the iterator returns a type convertible to value_type?
AFAICT the standard it either very silent on this, or says one should use `less{}` as the comparator, which does what `__less<>{}` does after this change. So this change is either a conforming change, or arguably a bugfix. Although funnily enough `std::less{}` isn't actually well-formed, and it doesn't specifically use `ranges::less{}` for the classic algorithms.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145285