[libc++] Revert "Check correctly ref-qualified __is_callable in algorithms (#73451)"

This reverts commit 8d151f8, which
broke some build bots. I think that is caused by an invalid argument
order when checking __is_comparable in upper_bound.

Author: ldionne

libcxx/include/__algorithm/equal_range.h

@@ -62,7 +62,7 @@ __equal_range(_Iter __first, _Sent __last, const _Tp& __value, _Compare&& __comp
 template <class _ForwardIterator, class _Tp, class _Compare>
 _LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 pair<_ForwardIterator, _ForwardIterator>
 equal_range(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value, _Compare __comp) {
-  static_assert(__is_callable<_Compare&, decltype(*__first), const _Tp&>::value, "The comparator has to be callable");
+  static_assert(__is_callable<_Compare, decltype(*__first), const _Tp&>::value, "The comparator has to be callable");
   static_assert(is_copy_constructible<_ForwardIterator>::value, "Iterator has to be copy constructible");
   return std::__equal_range<_ClassicAlgPolicy>(
       std::move(__first),

libcxx/include/__algorithm/includes.h

@@ -54,7 +54,7 @@ includes(_InputIterator1 __first1,
          _InputIterator2 __last2,
          _Compare __comp) {
   static_assert(
-      __is_callable<_Compare&, decltype(*__first1), decltype(*__first2)>::value, "The comparator has to be callable");
+      __is_callable<_Compare, decltype(*__first1), decltype(*__first2)>::value, "Comparator has to be callable");
 
   return std::__includes(
       std::move(__first1),

libcxx/include/__algorithm/is_permutation.h

@@ -249,8 +249,8 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 bool __is_permutation(
 template <class _ForwardIterator1, class _ForwardIterator2, class _BinaryPredicate>
 _LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 bool is_permutation(
     _ForwardIterator1 __first1, _ForwardIterator1 __last1, _ForwardIterator2 __first2, _BinaryPredicate __pred) {
-  static_assert(__is_callable<_BinaryPredicate&, decltype(*__first1), decltype(*__first2)>::value,
-                "The comparator has to be callable");
+  static_assert(__is_callable<_BinaryPredicate, decltype(*__first1), decltype(*__first2)>::value,
+                "The predicate has to be callable");
 
   return std::__is_permutation<_ClassicAlgPolicy>(std::move(__first1), std::move(__last1), std::move(__first2), __pred);
 }
@@ -286,8 +286,8 @@ _LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 boo
     _ForwardIterator2 __first2,
     _ForwardIterator2 __last2,
     _BinaryPredicate __pred) {
-  static_assert(__is_callable<_BinaryPredicate&, decltype(*__first1), decltype(*__first2)>::value,
-                "The comparator has to be callable");
+  static_assert(__is_callable<_BinaryPredicate, decltype(*__first1), decltype(*__first2)>::value,
+                "The predicate has to be callable");
 
   return std::__is_permutation<_ClassicAlgPolicy>(
       std::move(__first1),

libcxx/include/__algorithm/lower_bound.h

@@ -93,7 +93,7 @@ __lower_bound(_ForwardIterator __first, _Sent __last, const _Type& __value, _Com
 template <class _ForwardIterator, class _Tp, class _Compare>
 _LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _ForwardIterator
 lower_bound(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value, _Compare __comp) {
-  static_assert(__is_callable<_Compare&, decltype(*__first), const _Tp&>::value, "The comparator has to be callable");
+  static_assert(__is_callable<_Compare, decltype(*__first), const _Tp&>::value, "The comparator has to be callable");
   auto __proj = std::__identity();
   return std::__lower_bound<_ClassicAlgPolicy>(__first, __last, __value, __comp, __proj);
 }

libcxx/include/__algorithm/max_element.h

@@ -13,7 +13,6 @@
 #include <__algorithm/comp_ref_type.h>
 #include <__config>
 #include <__iterator/iterator_traits.h>
-#include <__type_traits/is_callable.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -38,8 +37,6 @@ __max_element(_ForwardIterator __first, _ForwardIterator __last, _Compare __comp
 template <class _ForwardIterator, class _Compare>
 _LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 _ForwardIterator
 max_element(_ForwardIterator __first, _ForwardIterator __last, _Compare __comp) {
-  static_assert(
-      __is_callable<_Compare&, decltype(*__first), decltype(*__first)>::value, "The comparator has to be callable");
   return std::__max_element<__comp_ref_type<_Compare> >(__first, __last, __comp);
 }
 

libcxx/include/__algorithm/min_element.h

@@ -53,7 +53,7 @@ min_element(_ForwardIterator __first, _ForwardIterator __last, _Compare __comp)
   static_assert(
       __has_forward_iterator_category<_ForwardIterator>::value, "std::min_element requires a ForwardIterator");
   static_assert(
-      __is_callable<_Compare&, decltype(*__first), decltype(*__first)>::value, "The comparator has to be callable");
+      __is_callable<_Compare, decltype(*__first), decltype(*__first)>::value, "The comparator has to be callable");
 
   return std::__min_element<__comp_ref_type<_Compare> >(std::move(__first), std::move(__last), __comp);
 }

libcxx/include/__algorithm/minmax.h

@@ -40,7 +40,7 @@ minmax(_LIBCPP_LIFETIMEBOUND const _Tp& __a, _LIBCPP_LIFETIMEBOUND const _Tp& __
 template <class _Tp, class _Compare>
 _LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 pair<_Tp, _Tp>
 minmax(initializer_list<_Tp> __t, _Compare __comp) {
-  static_assert(__is_callable<_Compare&, _Tp, _Tp>::value, "The comparator has to be callable");
+  static_assert(__is_callable<_Compare, _Tp, _Tp>::value, "The comparator has to be callable");
   __identity __proj;
   auto __ret = std::__minmax_element_impl(__t.begin(), __t.end(), __comp, __proj);
   return pair<_Tp, _Tp>(*__ret.first, *__ret.second);

libcxx/include/__algorithm/minmax_element.h

@@ -84,7 +84,7 @@ minmax_element(_ForwardIterator __first, _ForwardIterator __last, _Compare __com
   static_assert(
       __has_forward_iterator_category<_ForwardIterator>::value, "std::minmax_element requires a ForwardIterator");
   static_assert(
-      __is_callable<_Compare&, decltype(*__first), decltype(*__first)>::value, "The comparator has to be callable");
+      __is_callable<_Compare, decltype(*__first), decltype(*__first)>::value, "The comparator has to be callable");
   auto __proj = __identity();
   return std::__minmax_element_impl(__first, __last, __comp, __proj);
 }

libcxx/include/__algorithm/partial_sort_copy.h

@@ -76,8 +76,8 @@ inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _RandomAccessIterator
     _RandomAccessIterator __result_first,
     _RandomAccessIterator __result_last,
     _Compare __comp) {
-  static_assert(__is_callable<_Compare&, decltype(*__first), decltype(*__result_first)>::value,
-                "The comparator has to be callable");
+  static_assert(
+      __is_callable<_Compare, decltype(*__first), decltype(*__result_first)>::value, "Comparator has to be callable");
 
   auto __result = std::__partial_sort_copy<_ClassicAlgPolicy>(
       __first,

libcxx/include/__algorithm/search.h

@@ -166,8 +166,8 @@ search(_ForwardIterator1 __first1,
        _ForwardIterator2 __first2,
        _ForwardIterator2 __last2,
        _BinaryPredicate __pred) {
-  static_assert(__is_callable<_BinaryPredicate&, decltype(*__first1), decltype(*__first2)>::value,
-                "The comparator has to be callable");
+  static_assert(__is_callable<_BinaryPredicate, decltype(*__first1), decltype(*__first2)>::value,
+                "BinaryPredicate has to be callable");
   auto __proj = __identity();
   return std::__search_impl(__first1, __last1, __first2, __last2, __pred, __proj, __proj).first;
 }

libcxx/include/__algorithm/search_n.h

@@ -139,7 +139,7 @@ template <class _ForwardIterator, class _Size, class _Tp, class _BinaryPredicate
 _LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _ForwardIterator search_n(
     _ForwardIterator __first, _ForwardIterator __last, _Size __count, const _Tp& __value, _BinaryPredicate __pred) {
   static_assert(
-      __is_callable<_BinaryPredicate&, decltype(*__first), const _Tp&>::value, "The comparator has to be callable");
+      __is_callable<_BinaryPredicate, decltype(*__first), const _Tp&>::value, "BinaryPredicate has to be callable");
   auto __proj = __identity();
   return std::__search_n_impl(__first, __last, std::__convert_to_integral(__count), __value, __pred, __proj).first;
 }

libcxx/include/__algorithm/upper_bound.h

@@ -18,7 +18,6 @@
 #include <__iterator/advance.h>
 #include <__iterator/distance.h>
 #include <__iterator/iterator_traits.h>
-#include <__type_traits/is_callable.h>
 #include <__type_traits/is_constructible.h>
 #include <__utility/move.h>
 
@@ -51,7 +50,6 @@ __upper_bound(_Iter __first, _Sent __last, const _Tp& __value, _Compare&& __comp
 template <class _ForwardIterator, class _Tp, class _Compare>
 _LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _ForwardIterator
 upper_bound(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value, _Compare __comp) {
-  static_assert(__is_callable<_Compare&, decltype(*__first), const _Tp&>::value, "The comparator has to be callable");
   static_assert(is_copy_constructible<_ForwardIterator>::value, "Iterator has to be copy constructible");
   return std::__upper_bound<_ClassicAlgPolicy>(
       std::move(__first), std::move(__last), __value, std::move(__comp), std::__identity());

libcxx/src/regex.cpp

@@ -323,8 +323,8 @@ const classnames ClassNames[] = {
     {"xdigit", ctype_base::xdigit}};
 
 struct use_strcmp {
-  bool operator()(const collationnames& x, const char* y) const { return strcmp(x.elem_, y) < 0; }
-  bool operator()(const classnames& x, const char* y) const { return strcmp(x.elem_, y) < 0; }
+  bool operator()(const collationnames& x, const char* y) { return strcmp(x.elem_, y) < 0; }
+  bool operator()(const classnames& x, const char* y) { return strcmp(x.elem_, y) < 0; }
 };
 
 } // namespace

libcxx/test/libcxx/algorithms/callable-requirements-rvalue.compile.pass.cpp

@@ -0,0 +1,30 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03
+
+// <algorithm>
+
+// check that the classical algorithms with non-callable comparators fail
+
+#include <algorithm>
+
+void f() {
+  struct S {
+    int i;
+
+    S(int i_) : i(i_) {}
+
+    bool compare(const S&) const;
+  };
+
+  S a[] = {1, 2, 3, 4};
+  (void) std::lower_bound(a, a + 4, 0, &S::compare); // expected-error@*:* {{The comparator has to be callable}}
+  (void) std::minmax({S{1}}, &S::compare); // expected-error@*:* {{The comparator has to be callable}}
+  (void) std::minmax_element(a, a + 4, &S::compare); // expected-error@*:* {{The comparator has to be callable}}
+}

libcxx/test/libcxx/algorithms/callable-requirements.verify.cpp

@@ -26,35 +26,35 @@ struct Proj {
     constexpr explicit Proj(int *copies) : copies_(copies) {}
     constexpr Proj(const Proj& rhs) : copies_(rhs.copies_) { *copies_ += 1; }
     constexpr Proj& operator=(const Proj&) = default;
-    constexpr void* operator()(T) const { return nullptr; }
+    constexpr void *operator()(T) const { return nullptr; }
 };
 
 struct Less {
-  constexpr bool operator()(void*, void*) const { return false; }
+    constexpr bool operator()(void*, void*) const { return false; }
 };
 
 struct Equal {
-  constexpr bool operator()(void*, void*) const { return true; }
+    constexpr bool operator()(void*, void*) const { return true; }
 };
 
 struct UnaryVoid {
-  constexpr void operator()(void*) const {}
+    constexpr void operator()(void*) const {}
 };
 
 struct UnaryTrue {
-  constexpr bool operator()(void*) const { return true; }
+    constexpr bool operator()(void*) const { return true; }
 };
 
 struct NullaryValue {
-  constexpr std::nullptr_t operator()() const { return nullptr; }
+    constexpr std::nullptr_t operator()() const { return nullptr; }
 };
 
 struct UnaryTransform {
-  constexpr T operator()(void*) const { return T(); }
+    constexpr T operator()(void*) const { return T(); }
 };
 
 struct BinaryTransform {
-  constexpr T operator()(void*, void*) const { return T(); }
+    constexpr T operator()(void*, void*) const { return T(); }
 };
 
 constexpr bool all_the_algorithms()