Skip to content

Commit 8d151f8

Browse files
[libc++] Check correctly ref-qualified __is_callable in algorithms (llvm#73451)
We were only checking that the comparator was rvalue callable, when in reality the algorithms always call comparators as lvalues. This patch also refactors the tests for callable requirements and expands it to a few missing algorithms. Fixes llvm#69554
1 parent 41439d5 commit 8d151f8

23 files changed

+227
-84
lines changed

libcxx/include/__algorithm/equal_range.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ __equal_range(_Iter __first, _Sent __last, const _Tp& __value, _Compare&& __comp
6262
template <class _ForwardIterator, class _Tp, class _Compare>
6363
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 pair<_ForwardIterator, _ForwardIterator>
6464
equal_range(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value, _Compare __comp) {
65-
static_assert(__is_callable<_Compare, decltype(*__first), const _Tp&>::value, "The comparator has to be callable");
65+
static_assert(__is_callable<_Compare&, decltype(*__first), const _Tp&>::value, "The comparator has to be callable");
6666
static_assert(is_copy_constructible<_ForwardIterator>::value, "Iterator has to be copy constructible");
6767
return std::__equal_range<_ClassicAlgPolicy>(
6868
std::move(__first),

libcxx/include/__algorithm/includes.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ includes(_InputIterator1 __first1,
5454
_InputIterator2 __last2,
5555
_Compare __comp) {
5656
static_assert(
57-
__is_callable<_Compare, decltype(*__first1), decltype(*__first2)>::value, "Comparator has to be callable");
57+
__is_callable<_Compare&, decltype(*__first1), decltype(*__first2)>::value, "The comparator has to be callable");
5858

5959
return std::__includes(
6060
std::move(__first1),

libcxx/include/__algorithm/is_permutation.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -249,8 +249,8 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 bool __is_permutation(
249249
template <class _ForwardIterator1, class _ForwardIterator2, class _BinaryPredicate>
250250
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 bool is_permutation(
251251
_ForwardIterator1 __first1, _ForwardIterator1 __last1, _ForwardIterator2 __first2, _BinaryPredicate __pred) {
252-
static_assert(__is_callable<_BinaryPredicate, decltype(*__first1), decltype(*__first2)>::value,
253-
"The predicate has to be callable");
252+
static_assert(__is_callable<_BinaryPredicate&, decltype(*__first1), decltype(*__first2)>::value,
253+
"The comparator has to be callable");
254254

255255
return std::__is_permutation<_ClassicAlgPolicy>(std::move(__first1), std::move(__last1), std::move(__first2), __pred);
256256
}
@@ -286,8 +286,8 @@ _LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 boo
286286
_ForwardIterator2 __first2,
287287
_ForwardIterator2 __last2,
288288
_BinaryPredicate __pred) {
289-
static_assert(__is_callable<_BinaryPredicate, decltype(*__first1), decltype(*__first2)>::value,
290-
"The predicate has to be callable");
289+
static_assert(__is_callable<_BinaryPredicate&, decltype(*__first1), decltype(*__first2)>::value,
290+
"The comparator has to be callable");
291291

292292
return std::__is_permutation<_ClassicAlgPolicy>(
293293
std::move(__first1),

libcxx/include/__algorithm/lower_bound.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ __lower_bound(_ForwardIterator __first, _Sent __last, const _Type& __value, _Com
9393
template <class _ForwardIterator, class _Tp, class _Compare>
9494
_LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _ForwardIterator
9595
lower_bound(_ForwardIterator __first, _ForwardIterator __last, const _Tp& __value, _Compare __comp) {
96-
static_assert(__is_callable<_Compare, decltype(*__first), const _Tp&>::value, "The comparator has to be callable");
96+
static_assert(__is_callable<_Compare&, decltype(*__first), const _Tp&>::value, "The comparator has to be callable");
9797
auto __proj = std::__identity();
9898
return std::__lower_bound<_ClassicAlgPolicy>(__first, __last, __value, __comp, __proj);
9999
}

libcxx/include/__algorithm/max_element.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <__algorithm/comp_ref_type.h>
1414
#include <__config>
1515
#include <__iterator/iterator_traits.h>
16+
#include <__type_traits/is_callable.h>
1617

1718
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
1819
# pragma GCC system_header
@@ -37,6 +38,8 @@ __max_element(_ForwardIterator __first, _ForwardIterator __last, _Compare __comp
3738
template <class _ForwardIterator, class _Compare>
3839
_LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 _ForwardIterator
3940
max_element(_ForwardIterator __first, _ForwardIterator __last, _Compare __comp) {
41+
static_assert(
42+
__is_callable<_Compare&, decltype(*__first), decltype(*__first)>::value, "The comparator has to be callable");
4043
return std::__max_element<__comp_ref_type<_Compare> >(__first, __last, __comp);
4144
}
4245

libcxx/include/__algorithm/min_element.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ min_element(_ForwardIterator __first, _ForwardIterator __last, _Compare __comp)
5353
static_assert(
5454
__has_forward_iterator_category<_ForwardIterator>::value, "std::min_element requires a ForwardIterator");
5555
static_assert(
56-
__is_callable<_Compare, decltype(*__first), decltype(*__first)>::value, "The comparator has to be callable");
56+
__is_callable<_Compare&, decltype(*__first), decltype(*__first)>::value, "The comparator has to be callable");
5757

5858
return std::__min_element<__comp_ref_type<_Compare> >(std::move(__first), std::move(__last), __comp);
5959
}

libcxx/include/__algorithm/minmax.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ minmax(_LIBCPP_LIFETIMEBOUND const _Tp& __a, _LIBCPP_LIFETIMEBOUND const _Tp& __
4040
template <class _Tp, class _Compare>
4141
_LIBCPP_NODISCARD inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 pair<_Tp, _Tp>
4242
minmax(initializer_list<_Tp> __t, _Compare __comp) {
43-
static_assert(__is_callable<_Compare, _Tp, _Tp>::value, "The comparator has to be callable");
43+
static_assert(__is_callable<_Compare&, _Tp, _Tp>::value, "The comparator has to be callable");
4444
__identity __proj;
4545
auto __ret = std::__minmax_element_impl(__t.begin(), __t.end(), __comp, __proj);
4646
return pair<_Tp, _Tp>(*__ret.first, *__ret.second);

libcxx/include/__algorithm/minmax_element.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ minmax_element(_ForwardIterator __first, _ForwardIterator __last, _Compare __com
8484
static_assert(
8585
__has_forward_iterator_category<_ForwardIterator>::value, "std::minmax_element requires a ForwardIterator");
8686
static_assert(
87-
__is_callable<_Compare, decltype(*__first), decltype(*__first)>::value, "The comparator has to be callable");
87+
__is_callable<_Compare&, decltype(*__first), decltype(*__first)>::value, "The comparator has to be callable");
8888
auto __proj = __identity();
8989
return std::__minmax_element_impl(__first, __last, __comp, __proj);
9090
}

libcxx/include/__algorithm/partial_sort_copy.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _RandomAccessIterator
7676
_RandomAccessIterator __result_first,
7777
_RandomAccessIterator __result_last,
7878
_Compare __comp) {
79-
static_assert(
80-
__is_callable<_Compare, decltype(*__first), decltype(*__result_first)>::value, "Comparator has to be callable");
79+
static_assert(__is_callable<_Compare&, decltype(*__first), decltype(*__result_first)>::value,
80+
"The comparator has to be callable");
8181

8282
auto __result = std::__partial_sort_copy<_ClassicAlgPolicy>(
8383
__first,

libcxx/include/__algorithm/search.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,8 @@ search(_ForwardIterator1 __first1,
166166
_ForwardIterator2 __first2,
167167
_ForwardIterator2 __last2,
168168
_BinaryPredicate __pred) {
169-
static_assert(__is_callable<_BinaryPredicate, decltype(*__first1), decltype(*__first2)>::value,
170-
"BinaryPredicate has to be callable");
169+
static_assert(__is_callable<_BinaryPredicate&, decltype(*__first1), decltype(*__first2)>::value,
170+
"The comparator has to be callable");
171171
auto __proj = __identity();
172172
return std::__search_impl(__first1, __last1, __first2, __last2, __pred, __proj, __proj).first;
173173
}

0 commit comments

Comments
 (0)