diff options
author | Patrick Palka <ppalka@redhat.com> | 2020-06-10 17:37:53 -0400 |
---|---|---|
committer | Patrick Palka <ppalka@redhat.com> | 2020-06-10 17:37:53 -0400 |
commit | a73051a0ea9ce8281e748a74dd924a6eb8fb3723 (patch) | |
tree | 81e44249b7c86a233ef29a170f0084c602db54ec /libstdc++-v3/include | |
parent | 06ef9c119c56568e5f77a5189aa382cb97c95a9e (diff) | |
download | gcc-a73051a0ea9ce8281e748a74dd924a6eb8fb3723.zip gcc-a73051a0ea9ce8281e748a74dd924a6eb8fb3723.tar.gz gcc-a73051a0ea9ce8281e748a74dd924a6eb8fb3723.tar.bz2 |
libstdc++: Fix some ranges algos optimizations [PR95578]
ranges::copy and a number of other ranges algorithms have unwrapping
optimizations for iterators of type __normal_iterator, move_iterator and
reverse_iterator. But in the checks that guard these optimizations we
currently only test that the iterator of the iterator/sentinel pair has
the appropriate type before proceeding with the corresponding
optimization, and do not also test the sentinel type.
This breaks the testcase in this PR because this testcase constructs via
range adaptors a range whose begin() is a __normal_iterator and whose
end() is a custom sentinel type, and then performs ranges::copy on it.
From there we bogusly perform the __normal_iterator unwrapping
optimization on this iterator/sentinel pair, which immediately leads to
a constraint failure since the custom sentinel type does not model
sentinel_for<int*>.
This patch fixes this issue by refining each of the problematic checks
to also test that the iterator and sentinel types are the same before
applying the corresponding unwrapping optimization. Along the way, some
code simplifications are made.
libstdc++-v3/ChangeLog:
PR libstdc++/95578
* include/bits/ranges_algo.h (__lexicographical_compare_fn):
Also check that the iterator and sentinel have the same type before
applying the unwrapping optimization for __normal_iterator.
Split the check into two, one for the first iterator/sentinel
pair and another for second iterator/sentinel pair. Remove uses
of __niter_base, and remove uses of std::move on a
__normal_iterator.
* include/bits/ranges_algobase.h (__equal_fn): Likewise.
(__copy_or_move): Likewise. Perform similar adjustments for
the reverse_iterator and move_iterator optimizations. Inline
the checks into the if-constexprs, and use using-declarations to
make them less visually noisy. Remove uses of __niter_wrap.
(__copy_or_move_backward): Likewise.
* testsuite/25_algorithms/copy/95578.cc: New test.
* testsuite/25_algorithms/copy_backward/95578.cc: New test.
* testsuite/25_algorithms/equal/95578.cc: New test.
* testsuite/25_algorithms/lexicographical_compare/95578.cc: New test.
* testsuite/25_algorithms/move/95578.cc: New test.
* testsuite/25_algorithms/move_backward/95578.cc: New test.
Diffstat (limited to 'libstdc++-v3/include')
-rw-r--r-- | libstdc++-v3/include/bits/ranges_algo.h | 14 | ||||
-rw-r--r-- | libstdc++-v3/include/bits/ranges_algobase.h | 88 |
2 files changed, 57 insertions, 45 deletions
diff --git a/libstdc++-v3/include/bits/ranges_algo.h b/libstdc++-v3/include/bits/ranges_algo.h index c038a50..94ca7b6 100644 --- a/libstdc++-v3/include/bits/ranges_algo.h +++ b/libstdc++-v3/include/bits/ranges_algo.h @@ -3452,11 +3452,15 @@ namespace ranges _Proj1 __proj1 = {}, _Proj2 __proj2 = {}) const { if constexpr (__detail::__is_normal_iterator<_Iter1> - || __detail::__is_normal_iterator<_Iter2>) - return (*this)(std::__niter_base(std::move(__first1)), - std::__niter_base(std::move(__last1)), - std::__niter_base(std::move(__first2)), - std::__niter_base(std::move(__last2)), + && same_as<_Iter1, _Sent1>) + return (*this)(__first1.base(), __last1.base(), + std::move(__first2), std::move(__last2), + std::move(__comp), + std::move(__proj1), std::move(__proj2)); + else if constexpr (__detail::__is_normal_iterator<_Iter2> + && same_as<_Iter2, _Sent2>) + return (*this)(std::move(__first1), std::move(__last1), + __first2.base(), __last2.base(), std::move(__comp), std::move(__proj1), std::move(__proj2)); else diff --git a/libstdc++-v3/include/bits/ranges_algobase.h b/libstdc++-v3/include/bits/ranges_algobase.h index 49ca5ed4..3bdc7cc 100644 --- a/libstdc++-v3/include/bits/ranges_algobase.h +++ b/libstdc++-v3/include/bits/ranges_algobase.h @@ -85,11 +85,15 @@ namespace ranges // TODO: implement more specializations to at least have parity with // std::equal. if constexpr (__detail::__is_normal_iterator<_Iter1> - || __detail::__is_normal_iterator<_Iter2>) - return (*this)(std::__niter_base(std::move(__first1)), - std::__niter_base(std::move(__last1)), - std::__niter_base(std::move(__first2)), - std::__niter_base(std::move(__last2)), + && same_as<_Iter1, _Sent1>) + return (*this)(__first1.base(), __last1.base(), + std::move(__first2), std::move(__last2), + std::move(__pred), + std::move(__proj1), std::move(__proj2)); + else if constexpr (__detail::__is_normal_iterator<_Iter2> + && same_as<_Iter2, _Sent2>) + return (*this)(std::move(__first1), std::move(__last1), + __first2.base(), __last2.base(), std::move(__pred), std::move(__proj1), std::move(__proj2)); else if constexpr (sized_sentinel_for<_Sent1, _Iter1> @@ -211,14 +215,10 @@ namespace ranges { // TODO: implement more specializations to be at least on par with // std::copy/std::move. - constexpr bool __normal_iterator_p - = (__detail::__is_normal_iterator<_Iter> - || __detail::__is_normal_iterator<_Out>); - constexpr bool __reverse_p - = (__detail::__is_reverse_iterator<_Iter> - && __detail::__is_reverse_iterator<_Out>); - constexpr bool __move_iterator_p = __detail::__is_move_iterator<_Iter>; - if constexpr (__move_iterator_p) + using __detail::__is_move_iterator; + using __detail::__is_reverse_iterator; + using __detail::__is_normal_iterator; + if constexpr (__is_move_iterator<_Iter> && same_as<_Iter, _Sent>) { auto [__in, __out] = ranges::__copy_or_move<true>(std::move(__first).base(), @@ -226,23 +226,28 @@ namespace ranges std::move(__result)); return {move_iterator{std::move(__in)}, std::move(__out)}; } - else if constexpr (__reverse_p) + else if constexpr (__is_reverse_iterator<_Iter> && same_as<_Iter, _Sent> + && __is_reverse_iterator<_Out>) { auto [__in,__out] - = ranges::__copy_or_move_backward<_IsMove>(__last.base(), - __first.base(), - __result.base()); + = ranges::__copy_or_move_backward<_IsMove>(std::move(__last).base(), + std::move(__first).base(), + std::move(__result).base()); return {reverse_iterator{std::move(__in)}, reverse_iterator{std::move(__out)}}; } - else if constexpr (__normal_iterator_p) + else if constexpr (__is_normal_iterator<_Iter> && same_as<_Iter, _Sent>) + { + auto [__in,__out] + = ranges::__copy_or_move<_IsMove>(__first.base(), __last.base(), + __result); + return {decltype(__first){__in}, std::move(__out)}; + } + else if constexpr (__is_normal_iterator<_Out>) { auto [__in,__out] - = ranges::__copy_or_move<_IsMove>(std::__niter_base(__first), - std::__niter_base(__last), - std::__niter_base(__result)); - return {std::__niter_wrap(__first, std::move(__in)), - std::__niter_wrap(__result, std::move(__out))}; + = ranges::__copy_or_move<_IsMove>(__first, __last, __result.base()); + return {std::move(__in), decltype(__result){__out}}; } else if constexpr (sized_sentinel_for<_Sent, _Iter>) { @@ -353,30 +358,33 @@ namespace ranges { // TODO: implement more specializations to be at least on par with // std::copy_backward/std::move_backward. - constexpr bool __normal_iterator_p - = (__detail::__is_normal_iterator<_Iter> - || __detail::__is_normal_iterator<_Out>); - constexpr bool __reverse_p - = (__detail::__is_reverse_iterator<_Iter> - && __detail::__is_reverse_iterator<_Out>); - if constexpr (__reverse_p) + using __detail::__is_reverse_iterator; + using __detail::__is_normal_iterator; + if constexpr (__is_reverse_iterator<_Iter> && same_as<_Iter, _Sent> + && __is_reverse_iterator<_Out>) { auto [__in,__out] - = ranges::__copy_or_move<_IsMove>(__last.base(), - __first.base(), - __result.base()); + = ranges::__copy_or_move<_IsMove>(std::move(__last).base(), + std::move(__first).base(), + std::move(__result).base()); return {reverse_iterator{std::move(__in)}, reverse_iterator{std::move(__out)}}; } - else if constexpr (__normal_iterator_p) + else if constexpr (__is_normal_iterator<_Iter> && same_as<_Iter, _Sent>) { auto [__in,__out] - = ranges::__copy_or_move_backward<_IsMove> - (std::__niter_base(__first), - std::__niter_base(__last), - std::__niter_base(__result)); - return {std::__niter_wrap(__first, std::move(__in)), - std::__niter_wrap(__result, std::move(__out))}; + = ranges::__copy_or_move_backward<_IsMove>(__first.base(), + __last.base(), + std::move(__result)); + return {decltype(__first){__in}, std::move(__out)}; + } + else if constexpr (__is_normal_iterator<_Out>) + { + auto [__in,__out] + = ranges::__copy_or_move_backward<_IsMove>(std::move(__first), + std::move(__last), + __result.base()); + return {std::move(__in), decltype(__result){__out}}; } else if constexpr (sized_sentinel_for<_Sent, _Iter>) { |