[libc++] Check correctly ref-qualified __is_callable in algorithms #101553
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Louis Dionne (ldionne) ChangesWe were only checking that the comparator was rvalue callable, This is take 2 of #73451, which was reverted because it broke some Fixes #69554 Patch is 28.09 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/101553.diff 24 Files Affected:
diff --git a/libcxx/include/__algorithm/equal_range.h b/libcxx/include/__algorithm/equal_range.h
index 09bbf8f006021..676e4366f3db7 100644
--- a/libcxx/include/__algorithm/equal_range.h
+++ b/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),
diff --git a/libcxx/include/__algorithm/includes.h b/libcxx/include/__algorithm/includes.h
index 62af03c374260..0ad09a94baf13 100644
--- a/libcxx/include/__algorithm/includes.h
+++ b/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, "Comparator has to be callable");
+ __is_callable<_Compare&, decltype(*__first1), decltype(*__first2)>::value, "The comparator has to be callable");
return std::__includes(
std::move(__first1),
diff --git a/libcxx/include/__algorithm/is_permutation.h b/libcxx/include/__algorithm/is_permutation.h
index 2ddfb32a212bb..9dcfcf1810e7c 100644
--- a/libcxx/include/__algorithm/is_permutation.h
+++ b/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 predicate has to be callable");
+ static_assert(__is_callable<_BinaryPredicate&, decltype(*__first1), decltype(*__first2)>::value,
+ "The comparator 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 predicate has to be callable");
+ static_assert(__is_callable<_BinaryPredicate&, decltype(*__first1), decltype(*__first2)>::value,
+ "The comparator has to be callable");
return std::__is_permutation<_ClassicAlgPolicy>(
std::move(__first1),
diff --git a/libcxx/include/__algorithm/lower_bound.h b/libcxx/include/__algorithm/lower_bound.h
index c417d84835497..d18ab83ae7078 100644
--- a/libcxx/include/__algorithm/lower_bound.h
+++ b/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);
}
diff --git a/libcxx/include/__algorithm/max_element.h b/libcxx/include/__algorithm/max_element.h
index c036726cbccd8..3e58c40052c93 100644
--- a/libcxx/include/__algorithm/max_element.h
+++ b/libcxx/include/__algorithm/max_element.h
@@ -13,6 +13,7 @@
#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
@@ -37,6 +38,8 @@ __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);
}
diff --git a/libcxx/include/__algorithm/min_element.h b/libcxx/include/__algorithm/min_element.h
index 65f3594d630ce..9a360f94ce109 100644
--- a/libcxx/include/__algorithm/min_element.h
+++ b/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);
}
diff --git a/libcxx/include/__algorithm/minmax.h b/libcxx/include/__algorithm/minmax.h
index 9feda2b4c0da9..bb7a379be125b 100644
--- a/libcxx/include/__algorithm/minmax.h
+++ b/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);
diff --git a/libcxx/include/__algorithm/minmax_element.h b/libcxx/include/__algorithm/minmax_element.h
index 43cb23347c346..23929c96d987f 100644
--- a/libcxx/include/__algorithm/minmax_element.h
+++ b/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);
}
diff --git a/libcxx/include/__algorithm/partial_sort_copy.h b/libcxx/include/__algorithm/partial_sort_copy.h
index ef7c9d34d9498..6d44b4f8f928a 100644
--- a/libcxx/include/__algorithm/partial_sort_copy.h
+++ b/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, "Comparator has to be callable");
+ static_assert(__is_callable<_Compare&, decltype(*__first), decltype(*__result_first)>::value,
+ "The comparator has to be callable");
auto __result = std::__partial_sort_copy<_ClassicAlgPolicy>(
__first,
diff --git a/libcxx/include/__algorithm/search.h b/libcxx/include/__algorithm/search.h
index b82ca78095354..7316e5e4e1d98 100644
--- a/libcxx/include/__algorithm/search.h
+++ b/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,
- "BinaryPredicate has to be callable");
+ static_assert(__is_callable<_BinaryPredicate&, decltype(*__first1), decltype(*__first2)>::value,
+ "The comparator has to be callable");
auto __proj = __identity();
return std::__search_impl(__first1, __last1, __first2, __last2, __pred, __proj, __proj).first;
}
diff --git a/libcxx/include/__algorithm/search_n.h b/libcxx/include/__algorithm/search_n.h
index 771647d3168a4..f9806385656bf 100644
--- a/libcxx/include/__algorithm/search_n.h
+++ b/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, "BinaryPredicate has to be callable");
+ __is_callable<_BinaryPredicate&, decltype(*__first), const _Tp&>::value, "The comparator has to be callable");
auto __proj = __identity();
return std::__search_n_impl(__first, __last, std::__convert_to_integral(__count), __value, __pred, __proj).first;
}
diff --git a/libcxx/include/__algorithm/upper_bound.h b/libcxx/include/__algorithm/upper_bound.h
index c39dec2e89698..102447eddaf79 100644
--- a/libcxx/include/__algorithm/upper_bound.h
+++ b/libcxx/include/__algorithm/upper_bound.h
@@ -18,6 +18,7 @@
#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>
@@ -50,6 +51,7 @@ __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&, const _Tp&, decltype(*__first)>::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());
diff --git a/libcxx/src/regex.cpp b/libcxx/src/regex.cpp
index 9dc0c698541c8..6d9f06e213466 100644
--- a/libcxx/src/regex.cpp
+++ b/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) { return strcmp(x.elem_, y) < 0; }
- bool operator()(const classnames& x, const char* y) { return strcmp(x.elem_, y) < 0; }
+ 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; }
};
} // namespace
diff --git a/libcxx/test/libcxx/algorithms/callable-requirements-rvalue.compile.pass.cpp b/libcxx/test/libcxx/algorithms/callable-requirements-rvalue.compile.pass.cpp
new file mode 100644
index 0000000000000..4a5535e71ab96
--- /dev/null
+++ b/libcxx/test/libcxx/algorithms/callable-requirements-rvalue.compile.pass.cpp
@@ -0,0 +1,46 @@
+//===----------------------------------------------------------------------===//
+//
+// 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>
+
+// Make sure that we don't error out when passing a comparator that is lvalue callable
+// but not rvalue callable to algorithms. While it is technically ill-formed for users
+// to provide us such predicates, this test is useful for libc++ to ensure that we check
+// predicate requirements correctly (i.e. that we check them on lvalues and not on
+// rvalues). See https://github.com/llvm/llvm-project/issues/69554 for additional
+// context.
+
+#include <algorithm>
+
+#include "test_macros.h"
+
+struct NotRvalueCallable {
+ bool operator()(int a, int b) const& { return a < b; }
+ bool operator()(int, int) && = delete;
+};
+
+void f() {
+ int a[] = {1, 2, 3, 4};
+ (void)std::lower_bound(a, a + 4, 0, NotRvalueCallable{});
+ (void)std::upper_bound(a, a + 4, 0, NotRvalueCallable{});
+ (void)std::minmax({1, 2, 3}, NotRvalueCallable{});
+ (void)std::minmax_element(a, a + 4, NotRvalueCallable{});
+ (void)std::min_element(a, a + 4, NotRvalueCallable{});
+ (void)std::max_element(a, a + 4, NotRvalueCallable{});
+ (void)std::is_permutation(a, a + 4, a, NotRvalueCallable{});
+#if TEST_STD_VER >= 14
+ (void)std::is_permutation(a, a + 4, a, a + 4, NotRvalueCallable{});
+#endif
+ (void)std::includes(a, a + 4, a, a + 4, NotRvalueCallable{});
+ (void)std::equal_range(a, a + 4, 0, NotRvalueCallable{});
+ (void)std::partial_sort_copy(a, a + 4, a, a + 4, NotRvalueCallable{});
+ (void)std::search(a, a + 4, a, a + 4, NotRvalueCallable{});
+ (void)std::search_n(a, a + 4, 4, 0, NotRvalueCallable{});
+}
diff --git a/libcxx/test/libcxx/algorithms/callable-requirements.verify.cpp b/libcxx/test/libcxx/algorithms/callable-requirements.verify.cpp
new file mode 100644
index 0000000000000..7555d9815c60b
--- /dev/null
+++ b/libcxx/test/libcxx/algorithms/callable-requirements.verify.cpp
@@ -0,0 +1,118 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+
+// Ignore spurious errors after the initial static_assert failure.
+// ADDITIONAL_COMPILE_FLAGS: -Xclang -verify-ignore-unexpected=error
+
+// <algorithm>
+
+// Check that calling a classic STL algorithm with various non-callable comparators is diagnosed.
+
+#include <algorithm>
+
+#include "test_macros.h"
+
+struct NotCallable {
+ bool compare(int a, int b) const { return a < b; }
+};
+
+struct NotMutableCallable {
+ bool operator()(int a, int b) = delete;
+ bool operator()(int a, int b) const { return a < b; }
+};
+
+void f() {
+ int a[] = {1, 2, 3, 4};
+ {
+ (void)std::lower_bound(a, a + 4, 0, &NotCallable::compare);
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::upper_bound(a, a + 4, 0, &NotCallable::compare);
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::minmax({1, 2, 3}, &NotCallable::compare);
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::minmax_element(a, a + 4, &NotCallable::compare);
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::min_element(a, a + 4, &NotCallable::compare);
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::max_element(a, a + 4, &NotCallable::compare);
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::is_permutation(a, a + 4, a, &NotCallable::compare);
+ // expected-error@*:* {{The comparator has to be callable}}
+
+#if TEST_STD_VER >= 14
+ (void)std::is_permutation(a, a + 4, a, a + 4, &NotCallable::compare);
+ // expected-error@*:* {{The comparator has to be callable}}
+#endif
+
+ (void)std::includes(a, a + 4, a, a + 4, &NotCallable::compare);
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::equal_range(a, a + 4, 0, &NotCallable::compare);
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::partial_sort_copy(a, a + 4, a, a + 4, &NotCallable::compare);
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::search(a, a + 4, a, a + 4, &NotCallable::compare);
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::search_n(a, a + 4, 4, 0, &NotCallable::compare);
+ // expected-error@*:* {{The comparator has to be callable}}
+ }
+
+ {
+ (void)std::lower_bound(a, a + 4, 0, NotMutableCallable{});
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::upper_bound(a, a + 4, 0, NotMutableCallable{});
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::minmax({1, 2, 3}, NotMutableCallable{});
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::minmax_element(a, a + 4, NotMutableCallable{});
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::min_element(a, a + 4, NotMutableCallable{});
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::max_element(a, a + 4, NotMutableCallable{});
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::is_permutation(a, a + 4, a, NotMutableCallable{});
+ // expected-error@*:* {{The comparator has to be callable}}
+
+#if TEST_STD_VER >= 14
+ (void)std::is_permutation(a, a + 4, a, a + 4, NotMutableCallable{});
+ // expected-error@*:* {{The comparator has to be callable}}
+#endif
+
+ (void)std::includes(a, a + 4, a, a + 4, NotMutableCallable{});
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::equal_range(a, a + 4, 0, NotMutableCallable{});
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::partial_sort_copy(a, a + 4, a, a + 4, NotMutableCallable{});
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::search(a, a + 4, a, a + 4, NotMutableCallable{});
+ // expected-error@*:* {{The comparator has to be callable}}
+
+ (void)std::search_n(a, a + 4, 4, 0, NotMutableCallable{});
+ // expected-error@*:* {{The comparator has to be callable}}
+ }
+}
diff --git a/libcxx/test/libcxx/algorithms/callable.verify.cpp b/libcxx/test/libcxx/algorithms/callable.verify.cpp
deleted file mode 100644
index da87ecd0971d2..0000000000000
--- a/libcxx/test/libcxx/algorithms/callable.verify.cpp
+++ /dev/null
@@ -1,30 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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); //...
[truncated]
|
You can test this locally with the following command:git-clang-format --diff 937cbe270e5ee4e2e4d6f5568768a55d5e383076 689e6317169beb0da1b9f618eb7867d70f21367f --extensions cpp,h -- libcxx/test/libcxx/algorithms/callable-requirements-rvalue.compile.pass.cpp libcxx/test/libcxx/algorithms/callable-requirements.verify.cpp libcxx/include/__algorithm/equal_range.h libcxx/include/__algorithm/includes.h libcxx/include/__algorithm/is_permutation.h libcxx/include/__algorithm/lower_bound.h libcxx/include/__algorithm/max_element.h libcxx/include/__algorithm/min_element.h libcxx/include/__algorithm/minmax.h libcxx/include/__algorithm/minmax_element.h libcxx/include/__algorithm/partial_sort_copy.h libcxx/include/__algorithm/search.h libcxx/include/__algorithm/search_n.h libcxx/include/__algorithm/upper_bound.h libcxx/src/regex.cpp libcxx/test/libcxx/algorithms/ranges_robust_against_copying_projections.pass.cpp libcxx/test/libcxx/algorithms/robust_against_cpp20_hostile_iterators.compile.pass.cpp libcxx/test/std/algorithms/alg.nonmodifying/alg.is_permutation/is_permutation_pred.pass.cpp libcxx/test/std/algorithms/alg.nonmodifying/alg.search/search_n_pred.pass.cpp libcxx/test/std/algorithms/alg.nonmodifying/alg.search/search_pred.pass.cpp libcxx/test/std/algorithms/alg.sorting/alg.binary.search/upper.bound/upper_bound_comp.pass.cpp libcxx/test/std/algorithms/alg.sorting/alg.merge/inplace_merge_comp.pass.cpp libcxx/test/std/algorithms/alg.sorting/alg.sort/stable.sort/stable_sort_comp.pass.cppView the diff from clang-format here.diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.search/search_n_pred.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.search/search_n_pred.pass.cpp
index 2fa3e9ffac..ccc948ae1f 100644
--- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.search/search_n_pred.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.search/search_n_pred.pass.cpp
@@ -32,14 +32,14 @@ TEST_CONSTEXPR bool test_constexpr() {
}
#endif
-struct count_equal {
- static unsigned count;
- template <class T>
- bool operator()(const T& x, const T& y) const {
- ++count;
- return x == y;
- }
-};
+ struct count_equal {
+ static unsigned count;
+ template <class T>
+ bool operator()(const T& x, const T& y) const {
+ ++count;
+ return x == y;
+ }
+ };
unsigned count_equal::count = 0;
diff --git a/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/upper.bound/upper_bound_comp.pass.cpp b/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/upper.bound/upper_bound_comp.pass.cpp
index 8e62064078..c29e4a48ed 100644
--- a/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/upper.bound/upper_bound_comp.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.sorting/alg.binary.search/upper.bound/upper_bound_comp.pass.cpp
@@ -46,13 +46,11 @@ test(Iter first, Iter last, const T& value)
}
struct Val {
- int val;
+ int val;
};
struct Comparator {
- bool operator()(Val const& t, int x) const {
- return t.val < x;
- }
+ bool operator()(Val const& t, int x) const { return t.val < x; }
};
template <class Iter>
@@ -75,10 +73,10 @@ test()
// Make sure upper_bound works with a value that differs from the sequence's element type
{
- int array[] = {1, 2, 3};
- Val val = {2};
- Iter result = std::upper_bound(Iter(array), Iter(array + 3), val, Comparator());
- assert(result == Iter(array + 2));
+ int array[] = {1, 2, 3};
+ Val val = {2};
+ Iter result = std::upper_bound(Iter(array), Iter(array + 3), val, Comparator());
+ assert(result == Iter(array + 2));
}
}
|
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. This is take 2 of llvm#73451, which was reverted because it broke some CI bots. Fixes llvm#69554
ldionne
force-pushed
the
review/is_callable
branch
from
8f6ed8e to
689e631
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This is take 2 of #73451, which was reverted because it broke some
CI bots. The issue was that we checked __is_callable with arguments
in the wrong order inside std::upper_bound. This has now been fixed
and a test was added.
Fixes #69554