Skip to content

Commit b8f74cd

Browse files
ldionneldionne
committed
[libc++][pstl] Improve exception handling
There were various places where we incorrectly handled exceptions in the PSTL. Typical issues were missing `noexcept` and taking iterators by value instead of by reference. This patch fixes those inconsistent and incorrect instances, and adds proper tests for all of those. Note that the previous tests were often incorrectly turned into no-ops by the compiler due to copy ellision, which doesn't happen with these new tests.
1 parent 9ddedf0 commit b8f74cd

29 files changed

+400
-914
lines changed

‎libcxx/include/__algorithm/pstl_copy.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,12 @@ template <class _ExecutionPolicy,
8888
_LIBCPP_PSTL_CUSTOMIZATION_POINT(__pstl_copy_n, _RawPolicy),
8989
[&__policy](
9090
_ForwardIterator __g_first, _Size __g_n, _ForwardOutIterator __g_result) -> optional<_ForwardIterator> {
91-
if constexpr (__has_random_access_iterator_category_or_concept<_ForwardIterator>::value)
91+
if constexpr (__has_random_access_iterator_category_or_concept<_ForwardIterator>::value) {
9292
return std::__copy(__policy, std::move(__g_first), std::move(__g_first + __g_n), std::move(__g_result));
93-
else
93+
} else {
94+
(void)__policy;
9495
return std::copy_n(__g_first, __g_n, __g_result);
96+
}
9597
},
9698
std::move(__first),
9799
std::move(__n),

‎libcxx/include/__algorithm/pstl_count.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,8 @@ template <class _ExecutionPolicy,
8484
class _Tp,
8585
class _RawPolicy = __remove_cvref_t<_ExecutionPolicy>,
8686
enable_if_t<is_execution_policy_v<_RawPolicy>, int> = 0>
87-
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI optional<__iter_diff_t<_ForwardIterator>>
88-
__count(_ExecutionPolicy&& __policy, _ForwardIterator __first, _ForwardIterator __last, const _Tp& __value) {
87+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI optional<__iter_diff_t<_ForwardIterator>> __count(
88+
_ExecutionPolicy&& __policy, _ForwardIterator&& __first, _ForwardIterator&& __last, const _Tp& __value) noexcept {
8989
return std::__pstl_frontend_dispatch(
9090
_LIBCPP_PSTL_CUSTOMIZATION_POINT(__pstl_count, _RawPolicy),
9191
[&](_ForwardIterator __g_first, _ForwardIterator __g_last, const _Tp& __g_value)
@@ -94,8 +94,8 @@ __count(_ExecutionPolicy&& __policy, _ForwardIterator __first, _ForwardIterator
9494
return __v == __g_value;
9595
});
9696
},
97-
std::move(__first),
98-
std::move(__last),
97+
std::forward<_ForwardIterator>(__first),
98+
std::forward<_ForwardIterator>(__last),
9999
__value);
100100
}
101101

‎libcxx/include/__algorithm/pstl_equal.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,10 @@ template <class _ExecutionPolicy,
8686
enable_if_t<is_execution_policy_v<__remove_cvref_t<_ExecutionPolicy>>, int> = 0>
8787
_LIBCPP_HIDE_FROM_ABI bool
8888
equal(_ExecutionPolicy&& __policy, _ForwardIterator1 __first1, _ForwardIterator1 __last1, _ForwardIterator2 __first2) {
89-
return std::equal(__policy, std::move(__first1), std::move(__last1), std::move(__first2), std::equal_to{});
89+
auto __res = std::__equal(__policy, std::move(__first1), std::move(__last1), std::move(__first2), std::equal_to{});
90+
if (!__res)
91+
std::__throw_bad_alloc();
92+
return *__res;
9093
}
9194

9295
template <class _ExecutionPolicy,
@@ -162,8 +165,11 @@ equal(_ExecutionPolicy&& __policy,
162165
_ForwardIterator1 __last1,
163166
_ForwardIterator2 __first2,
164167
_ForwardIterator2 __last2) {
165-
return std::equal(
168+
auto __res = std::__equal(
166169
__policy, std::move(__first1), std::move(__last1), std::move(__first2), std::move(__last2), std::equal_to{});
170+
if (!__res)
171+
std::__throw_bad_alloc();
172+
return *__res;
167173
}
168174

169175
_LIBCPP_END_NAMESPACE_STD

‎libcxx/include/__algorithm/pstl_fill.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ template <class _ExecutionPolicy,
4141
class _Tp,
4242
class _RawPolicy = __remove_cvref_t<_ExecutionPolicy>,
4343
enable_if_t<is_execution_policy_v<_RawPolicy>, int> = 0>
44-
_LIBCPP_HIDE_FROM_ABI optional<__empty>
45-
__fill(_ExecutionPolicy&& __policy, _ForwardIterator __first, _ForwardIterator __last, const _Tp& __value) noexcept {
44+
_LIBCPP_HIDE_FROM_ABI optional<__empty> __fill(
45+
_ExecutionPolicy&& __policy, _ForwardIterator&& __first, _ForwardIterator&& __last, const _Tp& __value) noexcept {
4646
_LIBCPP_REQUIRE_CPP17_FORWARD_ITERATOR(_ForwardIterator);
4747
return std::__pstl_frontend_dispatch(
4848
_LIBCPP_PSTL_CUSTOMIZATION_POINT(__pstl_fill, _RawPolicy),
@@ -51,8 +51,8 @@ __fill(_ExecutionPolicy&& __policy, _ForwardIterator __first, _ForwardIterator _
5151
__element = __g_value;
5252
});
5353
},
54-
std::move(__first),
55-
std::move(__last),
54+
std::forward<_ForwardIterator>(__first),
55+
std::forward<_ForwardIterator>(__last),
5656
__value);
5757
}
5858

‎libcxx/include/__algorithm/pstl_find.h

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ template <class _ExecutionPolicy,
6565
class _Predicate,
6666
class _RawPolicy = __remove_cvref_t<_ExecutionPolicy>,
6767
enable_if_t<is_execution_policy_v<_RawPolicy>, int> = 0>
68-
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI optional<__remove_cvref_t<_ForwardIterator>>
69-
__find_if_not(_ExecutionPolicy&& __policy, _ForwardIterator&& __first, _ForwardIterator&& __last, _Predicate&& __pred) {
68+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI optional<__remove_cvref_t<_ForwardIterator>> __find_if_not(
69+
_ExecutionPolicy&& __policy, _ForwardIterator&& __first, _ForwardIterator&& __last, _Predicate&& __pred) noexcept {
7070
return std::__pstl_frontend_dispatch(
7171
_LIBCPP_PSTL_CUSTOMIZATION_POINT(__pstl_find_if_not, _RawPolicy),
7272
[&](_ForwardIterator&& __g_first, _ForwardIterator&& __g_last, _Predicate&& __g_pred)
@@ -76,9 +76,9 @@ __find_if_not(_ExecutionPolicy&& __policy, _ForwardIterator&& __first, _ForwardI
7676
return !__g_pred(__value);
7777
});
7878
},
79-
std::move(__first),
80-
std::move(__last),
81-
std::move(__pred));
79+
std::forward<_ForwardIterator>(__first),
80+
std::forward<_ForwardIterator>(__last),
81+
std::forward<_Predicate>(__pred));
8282
}
8383

8484
template <class _ExecutionPolicy,
@@ -103,8 +103,8 @@ template <class _ExecutionPolicy,
103103
class _Tp,
104104
class _RawPolicy = __remove_cvref_t<_ExecutionPolicy>,
105105
enable_if_t<is_execution_policy_v<_RawPolicy>, int> = 0>
106-
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI optional<__remove_cvref_t<_ForwardIterator>>
107-
__find(_ExecutionPolicy&& __policy, _ForwardIterator __first, _ForwardIterator __last, const _Tp& __value) noexcept {
106+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI optional<__remove_cvref_t<_ForwardIterator>> __find(
107+
_ExecutionPolicy&& __policy, _ForwardIterator&& __first, _ForwardIterator&& __last, const _Tp& __value) noexcept {
108108
return std::__pstl_frontend_dispatch(
109109
_LIBCPP_PSTL_CUSTOMIZATION_POINT(__pstl_find, _RawPolicy),
110110
[&](_ForwardIterator __g_first, _ForwardIterator __g_last, const _Tp& __g_value) -> optional<_ForwardIterator> {
@@ -113,8 +113,8 @@ __find(_ExecutionPolicy&& __policy, _ForwardIterator __first, _ForwardIterator _
113113
return __element == __g_value;
114114
});
115115
},
116-
std::move(__first),
117-
std::move(__last),
116+
std::forward<_ForwardIterator>(__first),
117+
std::forward<_ForwardIterator>(__last),
118118
__value);
119119
}
120120

‎libcxx/include/__algorithm/pstl_generate.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ template <class _ExecutionPolicy,
4040
class _Generator,
4141
class _RawPolicy = __remove_cvref_t<_ExecutionPolicy>,
4242
enable_if_t<is_execution_policy_v<_RawPolicy>, int> = 0>
43-
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI optional<__empty>
44-
__generate(_ExecutionPolicy&& __policy, _ForwardIterator&& __first, _ForwardIterator&& __last, _Generator&& __gen) {
43+
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI optional<__empty> __generate(
44+
_ExecutionPolicy&& __policy, _ForwardIterator&& __first, _ForwardIterator&& __last, _Generator&& __gen) noexcept {
4545
_LIBCPP_REQUIRE_CPP17_FORWARD_ITERATOR(_ForwardIterator);
4646
return std::__pstl_frontend_dispatch(
4747
_LIBCPP_PSTL_CUSTOMIZATION_POINT(__pstl_generate, _RawPolicy),
@@ -78,7 +78,7 @@ template <class _ExecutionPolicy,
7878
class _RawPolicy = __remove_cvref_t<_ExecutionPolicy>,
7979
enable_if_t<is_execution_policy_v<_RawPolicy>, int> = 0>
8080
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI optional<__empty>
81-
__generate_n(_ExecutionPolicy&& __policy, _ForwardIterator&& __first, _Size&& __n, _Generator&& __gen) {
81+
__generate_n(_ExecutionPolicy&& __policy, _ForwardIterator&& __first, _Size&& __n, _Generator&& __gen) noexcept {
8282
return std::__pstl_frontend_dispatch(
8383
_LIBCPP_PSTL_CUSTOMIZATION_POINT(__pstl_generate_n, _RawPolicy),
8484
[&__policy](_ForwardIterator __g_first, _Size __g_n, _Generator __g_gen) {

‎libcxx/include/__algorithm/pstl_is_partitioned.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ template <class _ExecutionPolicy,
4040
class _RawPolicy = __remove_cvref_t<_ExecutionPolicy>,
4141
enable_if_t<is_execution_policy_v<_RawPolicy>, int> = 0>
4242
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI optional<bool> __is_partitioned(
43-
_ExecutionPolicy&& __policy, _ForwardIterator&& __first, _ForwardIterator&& __last, _Predicate&& __pred) {
43+
_ExecutionPolicy&& __policy, _ForwardIterator&& __first, _ForwardIterator&& __last, _Predicate&& __pred) noexcept {
4444
return std::__pstl_frontend_dispatch(
4545
_LIBCPP_PSTL_CUSTOMIZATION_POINT(__pstl_is_partitioned, _RawPolicy),
4646
[&__policy](_ForwardIterator __g_first, _ForwardIterator __g_last, _Predicate __g_pred) {

‎libcxx/include/__algorithm/pstl_merge.h

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include <__type_traits/enable_if.h>
1616
#include <__type_traits/is_execution_policy.h>
1717
#include <__type_traits/remove_cvref.h>
18+
#include <__utility/forward.h>
1819
#include <__utility/move.h>
1920
#include <optional>
2021

@@ -33,26 +34,26 @@ template <class _ExecutionPolicy,
3334
class _ForwardIterator1,
3435
class _ForwardIterator2,
3536
class _ForwardOutIterator,
36-
class _Comp = std::less<>,
37+
class _Comp,
3738
class _RawPolicy = __remove_cvref_t<_ExecutionPolicy>,
3839
enable_if_t<is_execution_policy_v<_RawPolicy>, int> = 0>
3940
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI optional<_ForwardOutIterator>
4041
__merge(_ExecutionPolicy&&,
41-
_ForwardIterator1 __first1,
42-
_ForwardIterator1 __last1,
43-
_ForwardIterator2 __first2,
44-
_ForwardIterator2 __last2,
45-
_ForwardOutIterator __result,
46-
_Comp __comp = {}) noexcept {
42+
_ForwardIterator1&& __first1,
43+
_ForwardIterator1&& __last1,
44+
_ForwardIterator2&& __first2,
45+
_ForwardIterator2&& __last2,
46+
_ForwardOutIterator&& __result,
47+
_Comp&& __comp) noexcept {
4748
using _Backend = typename __select_backend<_RawPolicy>::type;
4849
return std::__pstl_merge<_RawPolicy>(
4950
_Backend{},
50-
std::move(__first1),
51-
std::move(__last1),
52-
std::move(__first2),
53-
std::move(__last2),
54-
std::move(__result),
55-
std::move(__comp));
51+
std::forward<_ForwardIterator1>(__first1),
52+
std::forward<_ForwardIterator1>(__last1),
53+
std::forward<_ForwardIterator2>(__first2),
54+
std::forward<_ForwardIterator2>(__last2),
55+
std::forward<_ForwardOutIterator>(__result),
56+
std::forward<_Comp>(__comp));
5657
}
5758

5859
template <class _ExecutionPolicy,

‎libcxx/include/__algorithm/pstl_replace.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,8 @@ template <class _ExecutionPolicy,
8989
enable_if_t<is_execution_policy_v<_RawPolicy>, int> = 0>
9090
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI optional<__empty>
9191
__replace(_ExecutionPolicy&& __policy,
92-
_ForwardIterator __first,
93-
_ForwardIterator __last,
92+
_ForwardIterator&& __first,
93+
_ForwardIterator&& __last,
9494
const _Tp& __old_value,
9595
const _Tp& __new_value) noexcept {
9696
return std::__pstl_frontend_dispatch(
@@ -104,8 +104,8 @@ __replace(_ExecutionPolicy&& __policy,
104104
[&](__iter_reference<_ForwardIterator> __element) { return __element == __g_old_value; },
105105
__g_new_value);
106106
},
107-
std::move(__first),
108-
std::move(__last),
107+
std::forward<_ForwardIterator>(__first),
108+
std::forward<_ForwardIterator>(__last),
109109
__old_value,
110110
__new_value);
111111
}
@@ -141,7 +141,7 @@ template <class _ExecutionPolicy,
141141
_ForwardIterator&& __last,
142142
_ForwardOutIterator&& __result,
143143
_Pred&& __pred,
144-
const _Tp& __new_value) {
144+
const _Tp& __new_value) noexcept {
145145
return std::__pstl_frontend_dispatch(
146146
_LIBCPP_PSTL_CUSTOMIZATION_POINT(__pstl_replace_copy_if, _RawPolicy),
147147
[&__policy](_ForwardIterator __g_first,

‎libcxx/include/__algorithm/pstl_sort.h

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,16 @@ template <class _ExecutionPolicy,
4141
class _RawPolicy = __remove_cvref_t<_ExecutionPolicy>,
4242
enable_if_t<is_execution_policy_v<_RawPolicy>, int> = 0>
4343
[[nodiscard]] _LIBCPP_HIDE_FROM_ABI optional<__empty> __sort(
44-
_ExecutionPolicy&& __policy, _RandomAccessIterator __first, _RandomAccessIterator __last, _Comp __comp) noexcept {
44+
_ExecutionPolicy&& __policy, _RandomAccessIterator&& __first, _RandomAccessIterator&& __last, _Comp&& __comp) noexcept {
4545
return std::__pstl_frontend_dispatch(
4646
_LIBCPP_PSTL_CUSTOMIZATION_POINT(__pstl_sort, _RawPolicy),
4747
[&__policy](_RandomAccessIterator __g_first, _RandomAccessIterator __g_last, _Comp __g_comp) {
4848
std::stable_sort(__policy, std::move(__g_first), std::move(__g_last), std::move(__g_comp));
4949
return optional<__empty>{__empty{}};
5050
},
51-
std::move(__first),
52-
std::move(__last),
53-
std::move(__comp));
51+
std::forward<_RandomAccessIterator>(__first),
52+
std::forward<_RandomAccessIterator>(__last),
53+
std::forward<_Comp>(__comp));
5454
}
5555

5656
template <class _ExecutionPolicy,
@@ -70,7 +70,8 @@ template <class _ExecutionPolicy,
7070
enable_if_t<is_execution_policy_v<_RawPolicy>, int> = 0>
7171
_LIBCPP_HIDE_FROM_ABI void
7272
sort(_ExecutionPolicy&& __policy, _RandomAccessIterator __first, _RandomAccessIterator __last) {
73-
std::sort(std::forward<_ExecutionPolicy>(__policy), std::move(__first), std::move(__last), less{});
73+
if (!std::__sort(__policy, std::move(__first), std::move(__last), less{}))
74+
std::__throw_bad_alloc();
7475
}
7576

7677
_LIBCPP_END_NAMESPACE_STD

‎libcxx/test/std/algorithms/alg.modifying.operations/alg.fill/pstl.exception_handling.pass.cpp

Lines changed: 0 additions & 58 deletions
This file was deleted.

‎libcxx/test/std/algorithms/alg.modifying.operations/alg.move/pstl.exception_handling.pass.cpp

Lines changed: 0 additions & 40 deletions
This file was deleted.

0 commit comments

Comments
 (0)