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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__algorithm/comp.h:32
 
-template <class _T1, class _T2 = _T1>
-struct __less
-{
-    _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX14
-    bool operator()(const _T1& __x, const _T1& __y) const {return __x < __y;}
-
-    _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX14
-    bool operator()(const _T1& __x, const _T2& __y) const {return __x < __y;}
-
-    _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX14
-    bool operator()(const _T2& __x, const _T1& __y) const {return __x < __y;}
-
-    _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX14
-    bool operator()(const _T2& __x, const _T2& __y) const {return __x < __y;}
-};
-
-template <class _T1>
-struct __less<_T1, _T1>
-{
-    _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX14
-    bool operator()(const _T1& __x, const _T1& __y) const {return __x < __y;}
-};
-
-template <class _T1>
-struct __less<const _T1, _T1>
-{
-    _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX14
-    bool operator()(const _T1& __x, const _T1& __y) const {return __x < __y;}
-};
-
-template <class _T1>
-struct __less<_T1, const _T1>
-{
-    _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX14
-    bool operator()(const _T1& __x, const _T1& __y) const {return __x < __y;}
+template <class _T1 = void, class _T2 = _T1>
+struct __less {};
----------------
ldionne wrote:
> So there's actually a behavior change in this patch (the removal of implicit conversions noticed by @EricWF). However, I think it is desirable -- right now I don't think we are conforming in C++20. For example, take `lower_bound`: http://eel.is/c++draft/lower.bound#1. It specifically says `Let comp be less{} and proj be identity{} for overloads with no parameters by those names.`, which implies that we should be using the transparent comparator, but we are not (before this patch).
> 
> I think this mandates a change everywhere along with tests. We should also confirm what's the expected behavior in prior Standards and make sure we conform in those as well.
I'm not actually convinced this behaviour change is reliably observable. AFAICT it would only be possible to observe with an `input_iterator` that returns a proxy reference which is comparable to the referenced type. Even then, an implementation is most likely allowed to call both `operator<(value_type&, proxy)` and `operator<(value_type&, value_type&)`. I'd argue that this case is unspecified behaviour.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145285