From 11da98f5d5e9092645e9a4175a2d6052134378d7 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Sun, 12 Jul 2020 20:31:45 +0200 Subject: [PATCH 1/6] Implement ranges::rotate --- stl/inc/algorithm | 149 ++++++++++++++++++ tests/std/test.lst | 2 + .../tests/P0896R4_ranges_alg_rotate/env.lst | 4 + .../tests/P0896R4_ranges_alg_rotate/test.cpp | 56 +++++++ .../P0896R4_ranges_alg_rotate_copy/env.lst | 4 + .../P0896R4_ranges_alg_rotate_copy/test.cpp | 65 ++++++++ 6 files changed, 280 insertions(+) create mode 100644 tests/std/tests/P0896R4_ranges_alg_rotate/env.lst create mode 100644 tests/std/tests/P0896R4_ranges_alg_rotate/test.cpp create mode 100644 tests/std/tests/P0896R4_ranges_alg_rotate_copy/env.lst create mode 100644 tests/std/tests/P0896R4_ranges_alg_rotate_copy/test.cpp diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 9f5ea0f97cf..764b4ebf56a 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -4626,6 +4626,155 @@ _FwdIt2 rotate_copy(_ExPo&&, _FwdIt1 _First, _FwdIt1 _Mid, _FwdIt1 _Last, _FwdIt return _STD rotate_copy(_First, _Mid, _Last, _Dest); } +#ifdef __cpp_lib_concepts +namespace ranges { + // VARIABLE ranges::rotate + class _Rotate_fn : private _Not_quite_object { + public: + using _Not_quite_object::_Not_quite_object; + + template _Se> + _NODISCARD constexpr subrange<_It> operator()(_It _First, _It _Mid, _Se _Last) const { + _Adl_verify_range(_First, _Mid); + _Adl_verify_range(_Mid, _Last); + auto _UResult = _Rotate_unchecked( + _Get_unwrapped(_STD move(_First)), _Get_unwrapped(_STD move(_Mid)), _Get_unwrapped(_STD move(_Last))); + + return _Rewrap_subrange>(_First, _STD move(_UResult)); + } + + // clang-format off + template + requires permutable> + _NODISCARD constexpr borrowed_subrange_t<_Rng> operator()(_Rng&& _Range, iterator_t<_Rng> _Mid) const { + _Adl_verify_range(_Ubegin(_Range), _Mid); + auto _UResult = _Rotate_unchecked(_Ubegin(_Range), _Get_unwrapped(_STD move(_Mid)), _Uend(_Range)); + + return _Rewrap_subrange>(_Range, _STD move(_UResult)); + } + // clang-format on + private: + template + _NODISCARD static constexpr subrange<_It> _Rotate_unchecked(_It _First, _It _Mid, _Se _Last) { + // Exchange the ranges [_First, _Mid) and [_Mid, _Last) + // that is, rotates [_First, _Last) left by distance(_First, _Mid) positions + _STL_INTERNAL_STATIC_ASSERT(permutable<_It>); + _STL_INTERNAL_STATIC_ASSERT(sentinel_for<_Se, _It>); + + auto _End = _Get_final_iterator_unwrapped<_It>(_First, _STD move(_Last)); + if (_First == _Mid) { + return {_End, _End}; + } + + if (_Mid == _End) { + return {_STD move(_First), _STD move(_End)}; + } + + if constexpr (random_access_iterator<_It>) { + _RANGES _Reverse_common(_First, _Mid); + _RANGES _Reverse_common(_Mid, _End); + _RANGES _Reverse_common(_First, _End); + _First += (_End - _Mid); + return {_STD move(_First), _STD move(_End)}; + } else if constexpr (bidirectional_iterator<_It>) { + _RANGES _Reverse_common(_First, _Mid); + _RANGES _Reverse_common(_Mid, _End); + while (_First != _Mid && _End != _Mid) { + _RANGES iter_swap(_First, --_End); + ++_First; + } + if (_First == _Mid) { + _RANGES _Reverse_common(_Mid, _End); + return {_STD move(_First), _STD move(_End)}; + } else { + _RANGES _Reverse_common(_First, _Mid); + return {_STD move(_First), _STD move(_End)}; + } + } else { + auto _Next = _Mid; + do { // rotate the first cycle + _RANGES iter_swap(_First, _Next); + ++_First; + ++_Next; + if (_First == _Mid) { + _Mid = _Next; + } + } while (_Next != _Last); + + auto _Begin = _First; + + while (_Mid != _Last) { // rotate subsequent cycles + _Next = _Mid; + do { + _RANGES iter_swap(_First, _Next); + ++_First; + ++_Next; + if (_First == _Mid) { + _Mid = _Next; + } + } while (_Next != _Last); + } + return {_STD move(_Begin), _STD move(_End)}; + } + } + }; + + inline constexpr _Rotate_fn rotate{_Not_quite_object::_Construct_tag{}}; + + // ALIAS TEMPLATE rotate_copy_result + template + using rotate_copy_result = in_out_result<_In, _Out>; + + // VARIABLE ranges::rotate_copy + class _Rotate_copy_fn : private _Not_quite_object { + public: + using _Not_quite_object::_Not_quite_object; + + // clang-format off + template _Se, weakly_incrementable _Out> + requires indirectly_copyable<_It, _Out> + _NODISCARD constexpr rotate_copy_result<_It, _Out> operator()( + _It _First, _It _Mid, _Se _Last, _Out _Result) const { + _Adl_verify_range(_First, _Mid); + _Adl_verify_range(_Mid, _Last); + auto _UResult = _Rotate_copy_unchecked(_Get_unwrapped(_STD move(_First)), _Get_unwrapped(_STD move(_Mid)), + _Get_unwrapped(_STD move(_Last)), _STD move(_Result)); + + _Seek_wrapped(_First, _STD move(_UResult.in)); + return {_STD move(_First), _STD move(_UResult.out)}; + } + + template + requires indirectly_copyable, _Out> + _NODISCARD constexpr rotate_copy_result, _Out> operator()( + _Rng&& _Range, iterator_t<_Rng> _Mid, _Out _Result) const { + _Adl_verify_range(_Ubegin(_Range), _Mid); + auto _UResult = _Rotate_copy_unchecked( + _Ubegin(_Range), _Get_unwrapped(_STD move(_Mid)), _Uend(_Range), _STD move(_Result)); + + return {_Rewrap_iterator(_Range, _STD move(_UResult.in)), _STD move(_UResult.out)}; + } + // clang-format on + private: + template + _NODISCARD static constexpr rotate_copy_result<_It, _Out> _Rotate_copy_unchecked( + _It _First, _It _Mid, _Se _Last, _Out _Result) { + // Copy the content of [_Mid, _Last) and [_First, _Mid) to _Result + _STL_INTERNAL_STATIC_ASSERT(forward_iterator<_It>); + _STL_INTERNAL_STATIC_ASSERT(sentinel_for<_Se, _It>); + _STL_INTERNAL_STATIC_ASSERT(weakly_incrementable<_Out>); + _STL_INTERNAL_STATIC_ASSERT(indirectly_copyable<_It, _Out>); + + auto _UResult1 = _RANGES _Copy_unchecked(_Mid, _STD move(_Last), _STD move(_Result)); + auto _UResult2 = _RANGES _Copy_unchecked(_STD move(_First), _STD move(_Mid), _STD move(_UResult1.out)); + return {_STD move(_UResult1.in), _STD move(_UResult2.out)}; + } + }; + + inline constexpr _Rotate_copy_fn rotate_copy{_Not_quite_object::_Construct_tag{}}; +} // namespace ranges +#endif // __cpp_lib_concepts + // FUNCTION TEMPLATE sample template _SampleIt _Sample_reservoir_unchecked( diff --git a/tests/std/test.lst b/tests/std/test.lst index c30028652af..b0cc976b56b 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -278,6 +278,8 @@ tests\P0896R4_ranges_alg_replace_copy tests\P0896R4_ranges_alg_replace_copy_if tests\P0896R4_ranges_alg_replace_if tests\P0896R4_ranges_alg_reverse +tests\P0896R4_ranges_alg_rotate +tests\P0896R4_ranges_alg_rotate_copy tests\P0896R4_ranges_alg_search tests\P0896R4_ranges_alg_search_n tests\P0896R4_ranges_alg_swap_ranges diff --git a/tests/std/tests/P0896R4_ranges_alg_rotate/env.lst b/tests/std/tests/P0896R4_ranges_alg_rotate/env.lst new file mode 100644 index 00000000000..f3ccc8613c6 --- /dev/null +++ b/tests/std/tests/P0896R4_ranges_alg_rotate/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\concepts_matrix.lst diff --git a/tests/std/tests/P0896R4_ranges_alg_rotate/test.cpp b/tests/std/tests/P0896R4_ranges_alg_rotate/test.cpp new file mode 100644 index 00000000000..8548367087d --- /dev/null +++ b/tests/std/tests/P0896R4_ranges_alg_rotate/test.cpp @@ -0,0 +1,56 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include +#include +#include +#include +#include + +#include +using namespace std; +using P = pair; + +// Validate dangling story +STATIC_ASSERT(same_as{}, nullptr_to)), ranges::dangling>); +STATIC_ASSERT(same_as{}, nullptr_to)), ranges::subrange>); + +struct instantiator { + static constexpr P expected[5] = {{3, 47}, {4, 99}, {0, 99}, {1, 47}, {2, 99}}; + + template + static constexpr void call() { +#if !defined(__clang__) && !defined(__EDG__) // TRANSITION, VSO-938163 +#pragma warning(suppress : 4127) // conditional expression is constant + if (!ranges::contiguous_range || !is_constant_evaluated()) +#endif // TRANSITION, VSO-938163 + { + using ranges::rotate, ranges::subrange, ranges::equal, ranges::iterator_t; + { // Validate iterator + sentinel overload + P input[5] = {{0, 99}, {1, 47}, {2, 99}, {3, 47}, {4, 99}}; + ReadWrite wrapped_input{input}; + + auto result = rotate(wrapped_input.begin(), next(wrapped_input.begin(), 3), wrapped_input.end()); + STATIC_ASSERT(same_as>>); + assert(result.begin() == next(wrapped_input.begin(), 2)); + assert(result.end() == wrapped_input.end()); + assert(equal(expected, input)); + } + { // Validate range overload + P input[5] = {{0, 99}, {1, 47}, {2, 99}, {3, 47}, {4, 99}}; + ReadWrite wrapped_input{input}; + + auto result = rotate(wrapped_input, next(wrapped_input.begin(), 3)); + STATIC_ASSERT(same_as>>); + assert(result.begin() == next(wrapped_input.begin(), 2)); + assert(result.end() == wrapped_input.end()); + assert(equal(expected, input)); + } + } + } +}; + +int main() { + STATIC_ASSERT((test_fwd(), true)); + test_fwd(); +} diff --git a/tests/std/tests/P0896R4_ranges_alg_rotate_copy/env.lst b/tests/std/tests/P0896R4_ranges_alg_rotate_copy/env.lst new file mode 100644 index 00000000000..f3ccc8613c6 --- /dev/null +++ b/tests/std/tests/P0896R4_ranges_alg_rotate_copy/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\concepts_matrix.lst diff --git a/tests/std/tests/P0896R4_ranges_alg_rotate_copy/test.cpp b/tests/std/tests/P0896R4_ranges_alg_rotate_copy/test.cpp new file mode 100644 index 00000000000..1de3bc8ec71 --- /dev/null +++ b/tests/std/tests/P0896R4_ranges_alg_rotate_copy/test.cpp @@ -0,0 +1,65 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include +#include +#include +#include +#include + +#include +using namespace std; + +// Validate that rotate_copy_result aliases in_out_result +STATIC_ASSERT(same_as, ranges::in_out_result>); + +// Validate dangling story +STATIC_ASSERT(same_as{}, nullptr_to, nullptr_to)), + ranges::rotate_copy_result>); +STATIC_ASSERT(same_as{}, nullptr_to, nullptr_to)), + ranges::rotate_copy_result>); + +struct instantiator { + static constexpr int input[5] = {1, 2, 3, 4, 5}; + static constexpr int expected[5] = {4, 5, 1, 2, 3}; + + template > Write> + static constexpr void call() { +#if !defined(__clang__) && !defined(__EDG__) // TRANSITION, VSO-938163 +#pragma warning(suppress : 4127) // conditional expression is constant + if (!ranges::contiguous_range || !is_constant_evaluated()) +#endif // TRANSITION, VSO-938163 + { + using ranges::rotate_copy, ranges::rotate_copy_result, ranges::iterator_t; + { // Validate iterator + sentinel overload + int output[5] = {-1, -1, -1, -1, -1}; + Read wrapped_input{input}; + iterator_t mid = next(wrapped_input.begin(), 3); + + auto result = rotate_copy(wrapped_input.begin(), mid, wrapped_input.end(), Write{output}); + STATIC_ASSERT(same_as, Write>>); + assert(result.in == wrapped_input.end()); + assert(result.out.peek() == end(output)); + assert(ranges::equal(expected, output)); + } + { // Validate range overload + int output[5] = {-1, -1, -1, -1, -1}; + Read wrapped_input{input}; + iterator_t mid = next(wrapped_input.begin(), 3); + + auto result = rotate_copy(wrapped_input, mid, Write{output}); + STATIC_ASSERT(same_as, Write>>); + assert(result.in == wrapped_input.end()); + assert(result.out.peek() == end(output)); + assert(ranges::equal(expected, output)); + } + } + } +}; + +int main() { +#ifndef _PREFAST_ // TRANSITION, GH-1030 + STATIC_ASSERT((test_fwd_write(), true)); +#endif // TRANSITION, GH-1030 + test_fwd_write(); +} From fdd5c005989c627b3489d2c73ec6889c0cc6c9f9 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 24 Jul 2020 08:57:01 +0200 Subject: [PATCH 2/6] Apply Caseys review comments --- stl/inc/algorithm | 50 ++++++++++++------- .../P0896R4_ranges_alg_rotate_copy/test.cpp | 1 - 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 764b4ebf56a..5d4ec740321 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -4634,7 +4634,7 @@ namespace ranges { using _Not_quite_object::_Not_quite_object; template _Se> - _NODISCARD constexpr subrange<_It> operator()(_It _First, _It _Mid, _Se _Last) const { + constexpr subrange<_It> operator()(_It _First, _It _Mid, _Se _Last) const { _Adl_verify_range(_First, _Mid); _Adl_verify_range(_Mid, _Last); auto _UResult = _Rotate_unchecked( @@ -4646,11 +4646,12 @@ namespace ranges { // clang-format off template requires permutable> - _NODISCARD constexpr borrowed_subrange_t<_Rng> operator()(_Rng&& _Range, iterator_t<_Rng> _Mid) const { - _Adl_verify_range(_Ubegin(_Range), _Mid); + constexpr borrowed_subrange_t<_Rng> operator()(_Rng&& _Range, iterator_t<_Rng> _Mid) const { + _Adl_verify_range(_RANGES begin(_Range), _Mid); + _Adl_verify_range(_Mid, _RANGES end(_Range)); auto _UResult = _Rotate_unchecked(_Ubegin(_Range), _Get_unwrapped(_STD move(_Mid)), _Uend(_Range)); - return _Rewrap_subrange>(_Range, _STD move(_UResult)); + return _Rewrap_subrange>(_Mid, _STD move(_UResult)); } // clang-format on private: @@ -4674,21 +4675,19 @@ namespace ranges { _RANGES _Reverse_common(_First, _Mid); _RANGES _Reverse_common(_Mid, _End); _RANGES _Reverse_common(_First, _End); - _First += (_End - _Mid); + _First += _End - _Mid; + return {_STD move(_First), _STD move(_End)}; } else if constexpr (bidirectional_iterator<_It>) { _RANGES _Reverse_common(_First, _Mid); _RANGES _Reverse_common(_Mid, _End); - while (_First != _Mid && _End != _Mid) { - _RANGES iter_swap(_First, --_End); - ++_First; - } - if (_First == _Mid) { - _RANGES _Reverse_common(_Mid, _End); - return {_STD move(_First), _STD move(_End)}; + auto _Result = _RANGES _Reverse_until_sentinel_unchecked(_First, _Mid, _Last); + _RANGES _Reverse_unchecked(_Result.in1, _Result.in2); + + if (_Result.in1 == _Mid) { + return {_STD move(_Result.in2), _STD move(_Last)}; } else { - _RANGES _Reverse_common(_First, _Mid); - return {_STD move(_First), _STD move(_End)}; + return {_STD move(_Result.in1), _STD move(_Last)}; } } else { auto _Next = _Mid; @@ -4717,6 +4716,20 @@ namespace ranges { return {_STD move(_Begin), _STD move(_End)}; } } + + template + _NODISCARD static constexpr in_in_result<_It, _It> _Reverse_until_sentinel_unchecked( + _It _First, const _It _Sentinel, _It _Last) { + // reverse until either _First or _Last hits _Sentinel + _STL_INTERNAL_STATIC_ASSERT(permutable<_It>); + + while (_First != _Sentinel && _Last != _Sentinel) { + _RANGES iter_swap(_First, --_Last); + ++_First; + } + + return {_STD move(_First), _STD move(_Last)}; + } }; inline constexpr _Rotate_fn rotate{_Not_quite_object::_Construct_tag{}}; @@ -4733,7 +4746,7 @@ namespace ranges { // clang-format off template _Se, weakly_incrementable _Out> requires indirectly_copyable<_It, _Out> - _NODISCARD constexpr rotate_copy_result<_It, _Out> operator()( + constexpr rotate_copy_result<_It, _Out> operator()( _It _First, _It _Mid, _Se _Last, _Out _Result) const { _Adl_verify_range(_First, _Mid); _Adl_verify_range(_Mid, _Last); @@ -4746,13 +4759,14 @@ namespace ranges { template requires indirectly_copyable, _Out> - _NODISCARD constexpr rotate_copy_result, _Out> operator()( + constexpr rotate_copy_result, _Out> operator()( _Rng&& _Range, iterator_t<_Rng> _Mid, _Out _Result) const { - _Adl_verify_range(_Ubegin(_Range), _Mid); + _Adl_verify_range(_RANGES begin(_Range), _Mid); + _Adl_verify_range(_Mid, _RANGES end(_Range)); auto _UResult = _Rotate_copy_unchecked( _Ubegin(_Range), _Get_unwrapped(_STD move(_Mid)), _Uend(_Range), _STD move(_Result)); - return {_Rewrap_iterator(_Range, _STD move(_UResult.in)), _STD move(_UResult.out)}; + return {_Rewrap_iterator(_Mid, _STD move(_UResult.in)), _STD move(_UResult.out)}; } // clang-format on private: diff --git a/tests/std/tests/P0896R4_ranges_alg_rotate_copy/test.cpp b/tests/std/tests/P0896R4_ranges_alg_rotate_copy/test.cpp index 1de3bc8ec71..c01a499cd9d 100644 --- a/tests/std/tests/P0896R4_ranges_alg_rotate_copy/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_rotate_copy/test.cpp @@ -5,7 +5,6 @@ #include #include #include -#include #include using namespace std; From f54e0765df152d39e6b38c8f54431d150d6da759 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 24 Jul 2020 09:02:29 +0200 Subject: [PATCH 3/6] Move ranges::rotate in front of rotate_copy --- stl/inc/algorithm | 52 +++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 5d4ec740321..f01631d610b 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -4602,30 +4602,6 @@ _FwdIt reverse_copy(_ExPo&&, _BidIt _First, _BidIt _Last, _FwdIt _Dest) noexcept } #endif // _HAS_CXX17 -// FUNCTION TEMPLATE rotate_copy -template -_CONSTEXPR20 _OutIt rotate_copy(_FwdIt _First, _FwdIt _Mid, _FwdIt _Last, _OutIt _Dest) { - // copy rotating [_First, _Last) - _Adl_verify_range(_First, _Mid); - _Adl_verify_range(_Mid, _Last); - const auto _UFirst = _Get_unwrapped(_First); - const auto _UMid = _Get_unwrapped(_Mid); - const auto _ULast = _Get_unwrapped(_Last); - auto _UDest = _Get_unwrapped_n(_Dest, _Idl_distance<_FwdIt>(_UFirst, _ULast)); - _UDest = _Copy_unchecked(_UMid, _ULast, _UDest); - _Seek_wrapped(_Dest, _Copy_unchecked(_UFirst, _UMid, _UDest)); - return _Dest; -} - -#if _HAS_CXX17 -template = 0> -_FwdIt2 rotate_copy(_ExPo&&, _FwdIt1 _First, _FwdIt1 _Mid, _FwdIt1 _Last, _FwdIt2 _Dest) noexcept /* terminates */ { - // copy rotating [_First, _Last) - // not parallelized as benchmarks show it isn't worth it - _REQUIRE_PARALLEL_ITERATOR(_FwdIt2); - return _STD rotate_copy(_First, _Mid, _Last, _Dest); -} - #ifdef __cpp_lib_concepts namespace ranges { // VARIABLE ranges::rotate @@ -4733,7 +4709,35 @@ namespace ranges { }; inline constexpr _Rotate_fn rotate{_Not_quite_object::_Construct_tag{}}; +} // namespace ranges +#endif // __cpp_lib_concepts +// FUNCTION TEMPLATE rotate_copy +template +_CONSTEXPR20 _OutIt rotate_copy(_FwdIt _First, _FwdIt _Mid, _FwdIt _Last, _OutIt _Dest) { + // copy rotating [_First, _Last) + _Adl_verify_range(_First, _Mid); + _Adl_verify_range(_Mid, _Last); + const auto _UFirst = _Get_unwrapped(_First); + const auto _UMid = _Get_unwrapped(_Mid); + const auto _ULast = _Get_unwrapped(_Last); + auto _UDest = _Get_unwrapped_n(_Dest, _Idl_distance<_FwdIt>(_UFirst, _ULast)); + _UDest = _Copy_unchecked(_UMid, _ULast, _UDest); + _Seek_wrapped(_Dest, _Copy_unchecked(_UFirst, _UMid, _UDest)); + return _Dest; +} + +#if _HAS_CXX17 +template = 0> +_FwdIt2 rotate_copy(_ExPo&&, _FwdIt1 _First, _FwdIt1 _Mid, _FwdIt1 _Last, _FwdIt2 _Dest) noexcept /* terminates */ { + // copy rotating [_First, _Last) + // not parallelized as benchmarks show it isn't worth it + _REQUIRE_PARALLEL_ITERATOR(_FwdIt2); + return _STD rotate_copy(_First, _Mid, _Last, _Dest); +} + +#ifdef __cpp_lib_concepts +namespace ranges { // ALIAS TEMPLATE rotate_copy_result template using rotate_copy_result = in_out_result<_In, _Out>; From 44d9cfa7f591ee53d6a881597dc0a16733727124 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Tue, 28 Jul 2020 08:07:06 +0200 Subject: [PATCH 4/6] Fix stupidness --- stl/inc/algorithm | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 964e871df4f..03ebce85e7e 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -4975,13 +4975,13 @@ namespace ranges { } else if constexpr (bidirectional_iterator<_It>) { _RANGES _Reverse_common(_First, _Mid); _RANGES _Reverse_common(_Mid, _End); - auto _Result = _RANGES _Reverse_until_sentinel_unchecked(_First, _Mid, _Last); - _RANGES _Reverse_unchecked(_Result.in1, _Result.in2); + auto _Result = _Reverse_until_sentinel_unchecked(_First, _Mid, _End); + _RANGES _Reverse_common(_Result.in1, _Result.in2); if (_Result.in1 == _Mid) { - return {_STD move(_Result.in2), _STD move(_Last)}; + return {_STD move(_Result.in2), _STD move(_End)}; } else { - return {_STD move(_Result.in1), _STD move(_Last)}; + return {_STD move(_Result.in1), _STD move(_End)}; } } else { auto _Next = _Mid; From 1aedf8f11b59195e34e85179343efb7a41a1c37e Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Tue, 28 Jul 2020 09:52:27 -0700 Subject: [PATCH 5/6] Polish: * Unqualify calls to `_Reverse_common`, which cannot be confused with a `_Meow_unchecked` function in `_STD`. * In `_Rotate_unchecked`: * Name the iterator value that's equal to the sentinel `_Final` instead of `_End` which has become conventional. * Avoid making an extra traversal ahead-of-time to find the final iterator of non-bidi ranges. * When we do need to find the final iterator value, start any traversal at `_Mid` instead of `_First`. * Let's not overload "sentinel" - which has enough meanings in Ranges - for `_Reverse_until_sentinel_unchecked`. * Don't test `constexpr rotate` at all with MSVC: it's extremely sensitive to VSO-938163. * Remove unnecessary bug workarounds from the `rotate_copy` test. --- stl/inc/algorithm | 69 ++++++++++--------- .../tests/P0896R4_ranges_alg_rotate/test.cpp | 48 ++++++------- .../P0896R4_ranges_alg_rotate_copy/test.cpp | 52 ++++++-------- 3 files changed, 81 insertions(+), 88 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 03ebce85e7e..0041d05980d 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -4845,7 +4845,7 @@ namespace ranges { auto _UFirst = _Get_unwrapped(_STD move(_First)); auto _ULast = _Get_final_iterator_unwrapped<_It>(_UFirst, _STD move(_Last)); _Seek_wrapped(_First, _ULast); - _RANGES _Reverse_common(_STD move(_UFirst), _STD move(_ULast)); + _Reverse_common(_STD move(_UFirst), _STD move(_ULast)); return _First; } @@ -4853,7 +4853,7 @@ namespace ranges { requires permutable> constexpr borrowed_iterator_t<_Rng> operator()(_Rng&& _Range) const { auto _ULast = _Get_final_iterator_unwrapped(_Range); - _RANGES _Reverse_common(_Ubegin(_Range), _ULast); + _Reverse_common(_Ubegin(_Range), _ULast); return _Rewrap_iterator(_Range, _STD move(_ULast)); } // clang-format on @@ -4941,13 +4941,14 @@ namespace ranges { template requires permutable> constexpr borrowed_subrange_t<_Rng> operator()(_Rng&& _Range, iterator_t<_Rng> _Mid) const { + // clang-format on _Adl_verify_range(_RANGES begin(_Range), _Mid); _Adl_verify_range(_Mid, _RANGES end(_Range)); auto _UResult = _Rotate_unchecked(_Ubegin(_Range), _Get_unwrapped(_STD move(_Mid)), _Uend(_Range)); return _Rewrap_subrange>(_Mid, _STD move(_UResult)); } - // clang-format on + private: template _NODISCARD static constexpr subrange<_It> _Rotate_unchecked(_It _First, _It _Mid, _Se _Last) { @@ -4956,32 +4957,34 @@ namespace ranges { _STL_INTERNAL_STATIC_ASSERT(permutable<_It>); _STL_INTERNAL_STATIC_ASSERT(sentinel_for<_Se, _It>); - auto _End = _Get_final_iterator_unwrapped<_It>(_First, _STD move(_Last)); if (_First == _Mid) { - return {_End, _End}; + auto _Final = _Get_final_iterator_unwrapped<_It>(_Mid, _STD move(_Last)); + return {_Final, _Final}; } - if (_Mid == _End) { - return {_STD move(_First), _STD move(_End)}; + if (_Mid == _Last) { + return {_STD move(_First), _STD move(_Mid)}; } - if constexpr (random_access_iterator<_It>) { - _RANGES _Reverse_common(_First, _Mid); - _RANGES _Reverse_common(_Mid, _End); - _RANGES _Reverse_common(_First, _End); - _First += _End - _Mid; + if constexpr (bidirectional_iterator<_It>) { + _Reverse_common(_First, _Mid); + auto _Final = _Get_final_iterator_unwrapped<_It>(_Mid, _STD move(_Last)); + _Reverse_common(_STD move(_First), _Final); - return {_STD move(_First), _STD move(_End)}; - } else if constexpr (bidirectional_iterator<_It>) { - _RANGES _Reverse_common(_First, _Mid); - _RANGES _Reverse_common(_Mid, _End); - auto _Result = _Reverse_until_sentinel_unchecked(_First, _Mid, _End); - _RANGES _Reverse_common(_Result.in1, _Result.in2); + if constexpr (random_access_iterator<_It>) { + _Reverse_common(_First, _Final); + _First += _Final - _Mid; - if (_Result.in1 == _Mid) { - return {_STD move(_Result.in2), _STD move(_End)}; + return {_STD move(_First), _STD move(_Final)}; } else { - return {_STD move(_Result.in1), _STD move(_End)}; + auto [_Mid_first, _Mid_last] = _Reverse_until_mid_unchecked(_STD move(_First), _Mid, _Final); + _Reverse_common(_Mid_first, _Mid_last); + + if (_Mid_first == _Mid) { + return {_STD move(_Mid_last), _STD move(_Final)}; + } else { + return {_STD move(_Mid_first), _STD move(_Final)}; + } } } else { auto _Next = _Mid; @@ -5007,20 +5010,20 @@ namespace ranges { } } while (_Next != _Last); } - return {_STD move(_Begin), _STD move(_End)}; + return {_STD move(_Begin), _STD move(_Mid)}; } } template - _NODISCARD static constexpr in_in_result<_It, _It> _Reverse_until_sentinel_unchecked( - _It _First, const _It _Sentinel, _It _Last) { - // reverse until either _First or _Last hits _Sentinel + _NODISCARD static constexpr subrange<_It> _Reverse_until_mid_unchecked(_It _First, const _It _Mid, _It _Last) { + // reverse until either _First or _Last hits _Mid _STL_INTERNAL_STATIC_ASSERT(permutable<_It>); + _STL_INTERNAL_CHECK(_First != _Mid); + _STL_INTERNAL_CHECK(_Mid != _Last); - while (_First != _Sentinel && _Last != _Sentinel) { + do { _RANGES iter_swap(_First, --_Last); - ++_First; - } + } while (++_First != _Mid && _Last != _Mid); return {_STD move(_First), _STD move(_Last)}; } @@ -5068,8 +5071,8 @@ namespace ranges { // clang-format off template _Se, weakly_incrementable _Out> requires indirectly_copyable<_It, _Out> - constexpr rotate_copy_result<_It, _Out> operator()( - _It _First, _It _Mid, _Se _Last, _Out _Result) const { + constexpr rotate_copy_result<_It, _Out> operator()(_It _First, _It _Mid, _Se _Last, _Out _Result) const { + // clang-format on _Adl_verify_range(_First, _Mid); _Adl_verify_range(_Mid, _Last); auto _UResult = _Rotate_copy_unchecked(_Get_unwrapped(_STD move(_First)), _Get_unwrapped(_STD move(_Mid)), @@ -5079,18 +5082,20 @@ namespace ranges { return {_STD move(_First), _STD move(_UResult.out)}; } + // clang-format off template requires indirectly_copyable, _Out> constexpr rotate_copy_result, _Out> operator()( _Rng&& _Range, iterator_t<_Rng> _Mid, _Out _Result) const { + // clang-format on _Adl_verify_range(_RANGES begin(_Range), _Mid); _Adl_verify_range(_Mid, _RANGES end(_Range)); auto _UResult = _Rotate_copy_unchecked( _Ubegin(_Range), _Get_unwrapped(_STD move(_Mid)), _Uend(_Range), _STD move(_Result)); - return {_Rewrap_iterator(_Mid, _STD move(_UResult.in)), _STD move(_UResult.out)}; + return {_Rewrap_iterator(_Range, _STD move(_UResult.in)), _STD move(_UResult.out)}; } - // clang-format on + private: template _NODISCARD static constexpr rotate_copy_result<_It, _Out> _Rotate_copy_unchecked( diff --git a/tests/std/tests/P0896R4_ranges_alg_rotate/test.cpp b/tests/std/tests/P0896R4_ranges_alg_rotate/test.cpp index 8548367087d..511a83c0b98 100644 --- a/tests/std/tests/P0896R4_ranges_alg_rotate/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_rotate/test.cpp @@ -20,37 +20,33 @@ struct instantiator { template static constexpr void call() { -#if !defined(__clang__) && !defined(__EDG__) // TRANSITION, VSO-938163 -#pragma warning(suppress : 4127) // conditional expression is constant - if (!ranges::contiguous_range || !is_constant_evaluated()) -#endif // TRANSITION, VSO-938163 - { - using ranges::rotate, ranges::subrange, ranges::equal, ranges::iterator_t; - { // Validate iterator + sentinel overload - P input[5] = {{0, 99}, {1, 47}, {2, 99}, {3, 47}, {4, 99}}; - ReadWrite wrapped_input{input}; - - auto result = rotate(wrapped_input.begin(), next(wrapped_input.begin(), 3), wrapped_input.end()); - STATIC_ASSERT(same_as>>); - assert(result.begin() == next(wrapped_input.begin(), 2)); - assert(result.end() == wrapped_input.end()); - assert(equal(expected, input)); - } - { // Validate range overload - P input[5] = {{0, 99}, {1, 47}, {2, 99}, {3, 47}, {4, 99}}; - ReadWrite wrapped_input{input}; - - auto result = rotate(wrapped_input, next(wrapped_input.begin(), 3)); - STATIC_ASSERT(same_as>>); - assert(result.begin() == next(wrapped_input.begin(), 2)); - assert(result.end() == wrapped_input.end()); - assert(equal(expected, input)); - } + using ranges::rotate, ranges::subrange, ranges::equal, ranges::iterator_t; + { // Validate iterator overload + P input[5] = {{0, 99}, {1, 47}, {2, 99}, {3, 47}, {4, 99}}; + ReadWrite wrapped_input{input}; + + auto result = rotate(wrapped_input.begin(), next(wrapped_input.begin(), 3), wrapped_input.end()); + STATIC_ASSERT(same_as>>); + assert(result.begin() == next(wrapped_input.begin(), 2)); + assert(result.end() == wrapped_input.end()); + assert(equal(expected, input)); + } + { // Validate range overload + P input[5] = {{0, 99}, {1, 47}, {2, 99}, {3, 47}, {4, 99}}; + ReadWrite wrapped_input{input}; + + auto result = rotate(wrapped_input, next(wrapped_input.begin(), 3)); + STATIC_ASSERT(same_as>>); + assert(result.begin() == next(wrapped_input.begin(), 2)); + assert(result.end() == wrapped_input.end()); + assert(equal(expected, input)); } } }; int main() { +#if defined(__clang__) || defined(__EDG__) // TRANSITION, VSO-938163 STATIC_ASSERT((test_fwd(), true)); +#endif // TRANSITION, VSO-938163 test_fwd(); } diff --git a/tests/std/tests/P0896R4_ranges_alg_rotate_copy/test.cpp b/tests/std/tests/P0896R4_ranges_alg_rotate_copy/test.cpp index c01a499cd9d..a28336063ae 100644 --- a/tests/std/tests/P0896R4_ranges_alg_rotate_copy/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_rotate_copy/test.cpp @@ -24,41 +24,33 @@ struct instantiator { template > Write> static constexpr void call() { -#if !defined(__clang__) && !defined(__EDG__) // TRANSITION, VSO-938163 -#pragma warning(suppress : 4127) // conditional expression is constant - if (!ranges::contiguous_range || !is_constant_evaluated()) -#endif // TRANSITION, VSO-938163 - { - using ranges::rotate_copy, ranges::rotate_copy_result, ranges::iterator_t; - { // Validate iterator + sentinel overload - int output[5] = {-1, -1, -1, -1, -1}; - Read wrapped_input{input}; - iterator_t mid = next(wrapped_input.begin(), 3); - - auto result = rotate_copy(wrapped_input.begin(), mid, wrapped_input.end(), Write{output}); - STATIC_ASSERT(same_as, Write>>); - assert(result.in == wrapped_input.end()); - assert(result.out.peek() == end(output)); - assert(ranges::equal(expected, output)); - } - { // Validate range overload - int output[5] = {-1, -1, -1, -1, -1}; - Read wrapped_input{input}; - iterator_t mid = next(wrapped_input.begin(), 3); - - auto result = rotate_copy(wrapped_input, mid, Write{output}); - STATIC_ASSERT(same_as, Write>>); - assert(result.in == wrapped_input.end()); - assert(result.out.peek() == end(output)); - assert(ranges::equal(expected, output)); - } + using ranges::rotate_copy, ranges::rotate_copy_result, ranges::equal, ranges::iterator_t; + { // Validate iterator overload + int output[5] = {-1, -1, -1, -1, -1}; + Read wrapped_input{input}; + iterator_t mid = next(wrapped_input.begin(), 3); + + auto result = rotate_copy(wrapped_input.begin(), mid, wrapped_input.end(), Write{output}); + STATIC_ASSERT(same_as, Write>>); + assert(result.in == wrapped_input.end()); + assert(result.out.peek() == end(output)); + assert(equal(expected, output)); + } + { // Validate range overload + int output[5] = {-1, -1, -1, -1, -1}; + Read wrapped_input{input}; + iterator_t mid = next(wrapped_input.begin(), 3); + + auto result = rotate_copy(wrapped_input, mid, Write{output}); + STATIC_ASSERT(same_as, Write>>); + assert(result.in == wrapped_input.end()); + assert(result.out.peek() == end(output)); + assert(equal(expected, output)); } } }; int main() { -#ifndef _PREFAST_ // TRANSITION, GH-1030 STATIC_ASSERT((test_fwd_write(), true)); -#endif // TRANSITION, GH-1030 test_fwd_write(); } From cd0865db241e609878486eaad013e7d0c24ec29d Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Tue, 28 Jul 2020 10:01:09 -0700 Subject: [PATCH 6/6] Fix typo --- stl/inc/algorithm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 0041d05980d..614594ed68e 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -4969,7 +4969,7 @@ namespace ranges { if constexpr (bidirectional_iterator<_It>) { _Reverse_common(_First, _Mid); auto _Final = _Get_final_iterator_unwrapped<_It>(_Mid, _STD move(_Last)); - _Reverse_common(_STD move(_First), _Final); + _Reverse_common(_Mid, _Final); if constexpr (random_access_iterator<_It>) { _Reverse_common(_First, _Final);