From 96bcfdfa70f4784c8a5f453b157187bc69e02ef6 Mon Sep 17 00:00:00 2001 From: Adam Bucior <35536269+AdamBucior@users.noreply.github.com> Date: Tue, 6 Jul 2021 17:09:20 +0200 Subject: [PATCH 1/8] Optimize is_permutation for reversed sequences --- stl/inc/algorithm | 60 +++++++++++- stl/inc/xutility | 96 ++++++++++++++++++- .../test.cpp | 26 +++++ .../test.cpp | 39 +++++++- .../test.cpp | 46 +++++++++ 5 files changed, 259 insertions(+), 8 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index da690a21c54..bc11ce429d6 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -1006,8 +1006,6 @@ namespace ranges { --_Final1; --_Final2; if (!_STD invoke(_Pred, _STD invoke(_Proj1, *_Final1), _STD invoke(_Proj2, *_Final2))) { // mismatch - ++_Final1; - ++_Final2; break; } @@ -1017,6 +1015,34 @@ namespace ranges { } // If we get here, _Count > 1, initial elements do not match, and final elements do not match. + const auto _ProjectedPred = [&](_Ty1&& _Left, _Ty2&& _Right) { + return _STD invoke(_Pred, _STD invoke(_Proj1, _STD forward<_Ty1>(_Left)), + _STD invoke(_Proj2, _STD forward<_Ty2>(_Right))); + }; + + for (;;) { + _TrimResult _Res = _Trim_reversed(_First1, _Final1, _First2, _Final2, _ProjectedPred); + if (_Res != _TrimResult::_Continue) { + if (_Res == _TrimResult::_Break) { + break; + } + return _Res == _TrimResult::_ReturnTrue; + } + + _Res = _Trim_equal(_First1, _Final1, _First2, _Final2, _ProjectedPred); + if (_Res != _TrimResult::_Continue) { + if (_Res == _TrimResult::_Break) { + break; + } + return _Res == _TrimResult::_ReturnTrue; + } + } + + ++_Final1; + ++_Final2; + // If we get here, initial elements do not match, final elements do not match, and ranges have length + // at least 2 and at most _Count + return _Match_counts(_STD move(_First1), _STD move(_Final1), _STD move(_First2), _STD move(_Final2), _Pred, _Proj1, _Proj2); } else { @@ -1074,13 +1100,39 @@ namespace ranges { --_Final2; // since ranges have equal lengths, _Final2 cannot equal _First2 if (!_STD invoke(_Pred, _STD invoke(_Proj1, *_Final1), _STD invoke(_Proj2, *_Final2))) { // mismatch - ++_Final1; - ++_Final2; break; } } // If we get here, initial elements do not match, final elements do not match, and ranges have length // at least 2. + + const auto _ProjectedPred = [&](_Ty1&& _Left, _Ty2&& _Right) { + return _STD invoke(_Pred, _STD invoke(_Proj1, _STD forward<_Ty1>(_Left)), + _STD invoke(_Proj2, _STD forward<_Ty2>(_Right))); + }; + + for (;;) { + _TrimResult _Res = _Trim_reversed(_First1, _Final1, _First2, _Final2, _ProjectedPred); + if (_Res != _TrimResult::_Continue) { + if (_Res == _TrimResult::_Break) { + break; + } + return _Res == _TrimResult::_ReturnTrue; + } + + _Res = _Trim_equal(_First1, _Final1, _First2, _Final2, _ProjectedPred); + if (_Res != _TrimResult::_Continue) { + if (_Res == _TrimResult::_Break) { + break; + } + return _Res == _TrimResult::_ReturnTrue; + } + } + + ++_Final1; + ++_Final2; + // If we get here, initial elements do not match, final elements do not match, and ranges have length + // at least 2. } return _Match_counts( diff --git a/stl/inc/xutility b/stl/inc/xutility index b550275eb9e..02e40c3e4a0 100644 --- a/stl/inc/xutility +++ b/stl/inc/xutility @@ -5504,9 +5504,80 @@ _NODISCARD constexpr _Iter_diff_t<_InIt> _Count_pr(_InIt _First, const _InIt _La return _Count; } +enum class _TrimResult : unsigned char { + _Continue, + _Break, + _ReturnFalse, + _ReturnTrue +}; + +template +_NODISCARD _CONSTEXPR20 _TrimResult _Trim_equal( + _BidIt1 &_First1, _BidIt1 &_Last1, _BidIt2 &_First2, _BidIt2 &_Last2, _Pr _Pred) { + _STL_INTERNAL_CHECK(_First1 != _Last1); + _STL_INTERNAL_CHECK(_STD distance(_First1, _Last1) == _STD distance(_First2, _Last2)); + if (_Pred(*_First1, *_First2)) { + do { + ++_First1; + ++_First2; + if (_First1 == _Last1) { + // only one element is left + return _Pred(*_First1, *_First2) ? _TrimResult::_ReturnTrue : _TrimResult::_ReturnFalse; + } + } while (_Pred(*_First1, *_First2)); + } else { + if (!_Pred(*_Last1, *_Last2)) { + // nothing to trim, break + return _TrimResult::_Break; + } + --_Last1; + --_Last2; + } + + while (_Pred(*_Last1, *_Last2)) { + --_Last1; + --_Last2; + } + + // if only one element is left return false, otherwise continue trimming + return _First1 == _Last1 ? _TrimResult::_ReturnFalse : _TrimResult::_Continue; +} + +template +_NODISCARD _CONSTEXPR20 _TrimResult _Trim_reversed( + _BidIt1& _First1, _BidIt1& _Last1, _BidIt2& _First2, _BidIt2& _Last2, _Pr _Pred) { + _STL_INTERNAL_CHECK(_First1 != _Last1); + _STL_INTERNAL_CHECK(_STD distance(_First1, _Last1) == _STD distance(_First2, _Last2)); + if (_Pred(*_First1, *_Last2)) { + do { + ++_First1; + --_Last2; + if (_First1 == _Last1) { + // only one element is left + return _Pred(*_First1, *_First2) ? _TrimResult::_ReturnTrue : _TrimResult::_ReturnFalse; + } + } while (_Pred(*_First1, *_Last2)); + } else { + if (!_Pred(*_Last1, *_First2)) { + // nothing to trim, break + return _TrimResult::_Break; + } + --_Last1; + ++_First2; + } + + while (_Pred(*_Last1, *_First2)) { + --_Last1; + ++_First2; + } + + // if only one element is left return false, otherwise continue trimming + return _First1 == _Last1 ? _TrimResult::_ReturnFalse : _TrimResult::_Continue; +} + template _NODISCARD _CONSTEXPR20 bool _Check_match_counts( - const _FwdIt1 _First1, _FwdIt1 _Last1, const _FwdIt2 _First2, _FwdIt2 _Last2, _Pr _Pred) { + _FwdIt1 _First1, _FwdIt1 _Last1, _FwdIt2 _First2, _FwdIt2 _Last2, _Pr _Pred) { // test if [_First1, _Last1) == permuted [_First2, _Last2), after matching prefix removal _STL_INTERNAL_CHECK(!_Pred(*_First1, *_First2)); _STL_INTERNAL_CHECK(_STD distance(_First1, _Last1) == _STD distance(_First2, _Last2)); @@ -5515,6 +5586,29 @@ _NODISCARD _CONSTEXPR20 bool _Check_match_counts( --_Last1; --_Last2; } while (_Pred(*_Last1, *_Last2)); + + if (_First1 == _Last1) { + return false; + } + + for (;;) { + _TrimResult _Res = _Trim_reversed(_First1, _Last1, _First2, _Last2, _Pred); + if (_Res != _TrimResult::_Continue) { + if (_Res == _TrimResult::_Break) { + break; + } + return _Res == _TrimResult::_ReturnTrue; + } + + _Res = _Trim_equal(_First1, _Last1, _First2, _Last2, _Pred); + if (_Res != _TrimResult::_Continue) { + if (_Res == _TrimResult::_Break) { + break; + } + return _Res == _TrimResult::_ReturnTrue; + } + } + ++_Last1; ++_Last2; } diff --git a/tests/std/tests/Dev11_0000000_dual_range_algorithms/test.cpp b/tests/std/tests/Dev11_0000000_dual_range_algorithms/test.cpp index 8dbef86df81..6f02556e5b1 100644 --- a/tests/std/tests/Dev11_0000000_dual_range_algorithms/test.cpp +++ b/tests/std/tests/Dev11_0000000_dual_range_algorithms/test.cpp @@ -331,6 +331,32 @@ int main() { assert(!is_permutation(a.begin(), a.end(), b.begin(), b.end(), equal_to())); } + { + int arr1[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; + int arr2[] = {9, 8, 7, 6, 5, 4, 3, 2, 1, 0}; + assert(is_permutation(begin(arr1), end(arr1), begin(arr2), end(arr2))); + } + { + int arr1[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; + int arr2[] = {9, 8, 7, 3, 4, 5, 6, 2, 1, 0}; + assert(is_permutation(begin(arr1), end(arr1), begin(arr2), end(arr2))); + } + { + int arr1[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; + int arr2[] = {9, 1, 7, 3, 5, 4, 6, 2, 8, 0}; + assert(is_permutation(begin(arr1), end(arr1), begin(arr2), end(arr2))); + } + { + int arr1[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; + int arr2[] = {9, 1, 7, 5, 6, 3, 4, 2, 8, 0}; + assert(is_permutation(begin(arr1), end(arr1), begin(arr2), end(arr2))); + } + { + int arr1[] = {0, 1, 2, 3, 4, 10, 5, 6, 7, 8, 9}; + int arr2[] = {9, 1, 7, 3, 5, 11, 4, 6, 2, 8, 0}; + assert(!is_permutation(begin(arr1), end(arr1), begin(arr2), end(arr2))); + } + { // Test that _ITERATOR_DEBUG_ARRAY_OVERLOADS is not needed anymore int arr[8] = {}; assert(mismatch(arr, arr, arr, arr) == make_pair(begin(arr), begin(arr))); diff --git a/tests/std/tests/P0202R3_constexpr_algorithm_and_exchange/test.cpp b/tests/std/tests/P0202R3_constexpr_algorithm_and_exchange/test.cpp index 4200898f9e5..2aa7ffb4249 100644 --- a/tests/std/tests/P0202R3_constexpr_algorithm_and_exchange/test.cpp +++ b/tests/std/tests/P0202R3_constexpr_algorithm_and_exchange/test.cpp @@ -492,7 +492,7 @@ constexpr bool test_pop_heap_and_push_heap() { } constexpr bool test_permutations() { - int buff[] = {1, 2, 3, 4}; + int buff[] = {10, 20, 30, 40}; // using std::array here is TRANSITION, DevCom-892153 constexpr array expected[] = { array{10, 20, 30, 40}, @@ -521,6 +521,10 @@ constexpr bool test_permutations() { array{40, 30, 20, 10}, }; + for (const auto& arr : expected) { + assert(is_permutation(begin(buff), end(buff), begin(arr), end(arr))); + } + auto cursor = begin(expected); do { assert(equal(begin(buff), end(buff), cursor->begin(), cursor->end())); @@ -531,11 +535,39 @@ constexpr bool test_permutations() { assert(!prev_permutation(begin(buff), end(buff))); do { - assert(equal(begin(buff), end(buff), cursor->begin(), cursor->end())); --cursor; + assert(equal(begin(buff), end(buff), cursor->begin(), cursor->end())); } while (prev_permutation(begin(buff), end(buff))); - assert(is_sorted(begin(buff), end(buff))); + assert(cursor == begin(expected)); + assert(is_sorted(begin(buff), end(buff), greater<>())); + + { + int arr1[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; + int arr2[] = {9, 8, 7, 6, 5, 4, 3, 2, 1, 0}; + assert(is_permutation(begin(arr1), end(arr1), begin(arr2), end(arr2))); + } + { + int arr1[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; + int arr2[] = {9, 8, 7, 3, 4, 5, 6, 2, 1, 0}; + assert(is_permutation(begin(arr1), end(arr1), begin(arr2), end(arr2))); + } + { + int arr1[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; + int arr2[] = {9, 1, 7, 3, 5, 4, 6, 2, 8, 0}; + assert(is_permutation(begin(arr1), end(arr1), begin(arr2), end(arr2))); + } + { + int arr1[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; + int arr2[] = {9, 1, 7, 5, 6, 3, 4, 2, 8, 0}; + assert(is_permutation(begin(arr1), end(arr1), begin(arr2), end(arr2))); + } + { + int arr1[] = {0, 1, 2, 3, 4, 10, 5, 6, 7, 8, 9}; + int arr2[] = {9, 1, 7, 3, 5, 11, 4, 6, 2, 8, 0}; + assert(!is_permutation(begin(arr1), end(arr1), begin(arr2), end(arr2))); + } + return true; } @@ -593,6 +625,7 @@ constexpr bool test() { && test_is_heap() && test_make_heap_and_sort_heap() && test_pop_heap_and_push_heap() + && test_permutations() ; // clang-format on } diff --git a/tests/std/tests/P0896R4_ranges_alg_is_permutation/test.cpp b/tests/std/tests/P0896R4_ranges_alg_is_permutation/test.cpp index 308e50f45e3..ea364cdcad0 100644 --- a/tests/std/tests/P0896R4_ranges_alg_is_permutation/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_is_permutation/test.cpp @@ -88,6 +88,52 @@ struct instantiator { assert(!ranges::is_permutation(ranges::begin(r1), ranges::end(r1), ranges::begin(r2), ranges::end(r2), ranges::equal_to{}, get_first, identity{})); } + { + int arr1[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; + int arr2[] = {9, 8, 7, 6, 5, 4, 3, 2, 1, 0}; + const Fwd2 r1{arr1}; + const Fwd2 r2{arr2}; + assert(ranges::is_permutation(r1, r2)); + } + { + int arr1[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; + int arr2[] = {9, 8, 7, 3, 4, 5, 6, 2, 1, 0}; + const Fwd2 r1{arr1}; + const Fwd2 r2{arr2}; + assert(ranges::is_permutation(r1, r2)); + } + { + int arr1[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; + int arr2[] = {9, 1, 7, 3, 5, 4, 6, 2, 8, 0}; + const Fwd2 r1{arr1}; + const Fwd2 r2{arr2}; + assert(ranges::is_permutation(r1, r2)); + } + { + int arr1[] = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; + int arr2[] = {9, 1, 7, 5, 6, 3, 4, 2, 8, 0}; + const Fwd2 r1{arr1}; + const Fwd2 r2{arr2}; + assert(ranges::is_permutation(r1, r2)); + } + { + int arr1[] = {0, 1, 2, 3, 4, 10, 5, 6, 7, 8, 9}; + int arr2[] = {9, 1, 7, 3, 5, 11, 4, 6, 2, 8, 0}; + const Fwd2 r1{arr1}; + const Fwd2 r2{arr2}; + assert(!ranges::is_permutation(r1, r2)); + } + { + int arr1[] = {-1, 0, 1, 2, 3, 4, 5, 6, 7, 8}; + int arr2[] = {10, 9, 8, 7, 6, 5, 4, 3, 2, 1}; + const Fwd2 r1{arr1}; + const Fwd2 r2{arr2}; + assert(!ranges::is_permutation(r1, r2)); + assert(!ranges::is_permutation(r1, r2, {}, {}, [](int n) { return n - 1; })); + assert(ranges::is_permutation(r1, r2, {}, {}, [](int n) { return n - 2; })); + assert(ranges::is_permutation( + r1, r2, {}, [](int n) { return n + 1; }, [](int n) { return n - 1; })); + } } }; From ca09bc03f02e0275b6be570a7acf48c2cfcecc42 Mon Sep 17 00:00:00 2001 From: Adam Bucior <35536269+AdamBucior@users.noreply.github.com> Date: Tue, 6 Jul 2021 19:32:27 +0200 Subject: [PATCH 2/8] clang-format --- stl/inc/xutility | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/stl/inc/xutility b/stl/inc/xutility index 02e40c3e4a0..e280fa3567f 100644 --- a/stl/inc/xutility +++ b/stl/inc/xutility @@ -5504,16 +5504,11 @@ _NODISCARD constexpr _Iter_diff_t<_InIt> _Count_pr(_InIt _First, const _InIt _La return _Count; } -enum class _TrimResult : unsigned char { - _Continue, - _Break, - _ReturnFalse, - _ReturnTrue -}; +enum class _TrimResult : unsigned char { _Continue, _Break, _ReturnFalse, _ReturnTrue }; template _NODISCARD _CONSTEXPR20 _TrimResult _Trim_equal( - _BidIt1 &_First1, _BidIt1 &_Last1, _BidIt2 &_First2, _BidIt2 &_Last2, _Pr _Pred) { + _BidIt1& _First1, _BidIt1& _Last1, _BidIt2& _First2, _BidIt2& _Last2, _Pr _Pred) { _STL_INTERNAL_CHECK(_First1 != _Last1); _STL_INTERNAL_CHECK(_STD distance(_First1, _Last1) == _STD distance(_First2, _Last2)); if (_Pred(*_First1, *_First2)) { From 982f0a7311d98d11d6f725ae125c55b66ea5471d Mon Sep 17 00:00:00 2001 From: Adam Bucior <35536269+AdamBucior@users.noreply.github.com> Date: Fri, 20 Aug 2021 22:52:38 +0200 Subject: [PATCH 3/8] Rename _Last to _Back --- stl/inc/xutility | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/stl/inc/xutility b/stl/inc/xutility index 8bcad45b0c5..2770afb09b6 100644 --- a/stl/inc/xutility +++ b/stl/inc/xutility @@ -5306,66 +5306,66 @@ enum class _TrimResult : unsigned char { _Continue, _Break, _ReturnFalse, _Retur template _NODISCARD _CONSTEXPR20 _TrimResult _Trim_equal( - _BidIt1& _First1, _BidIt1& _Last1, _BidIt2& _First2, _BidIt2& _Last2, _Pr _Pred) { - _STL_INTERNAL_CHECK(_First1 != _Last1); - _STL_INTERNAL_CHECK(_STD distance(_First1, _Last1) == _STD distance(_First2, _Last2)); + _BidIt1& _First1, _BidIt1& _Back1, _BidIt2& _First2, _BidIt2& _Back2, _Pr _Pred) { + _STL_INTERNAL_CHECK(_First1 != _Back1); + _STL_INTERNAL_CHECK(_STD distance(_First1, _Back1) == _STD distance(_First2, _Back2)); if (_Pred(*_First1, *_First2)) { do { ++_First1; ++_First2; - if (_First1 == _Last1) { + if (_First1 == _Back1) { // only one element is left return _Pred(*_First1, *_First2) ? _TrimResult::_ReturnTrue : _TrimResult::_ReturnFalse; } } while (_Pred(*_First1, *_First2)); } else { - if (!_Pred(*_Last1, *_Last2)) { + if (!_Pred(*_Back1, *_Back2)) { // nothing to trim, break return _TrimResult::_Break; } - --_Last1; - --_Last2; + --_Back1; + --_Back2; } - while (_Pred(*_Last1, *_Last2)) { - --_Last1; - --_Last2; + while (_Pred(*_Back1, *_Back2)) { + --_Back1; + --_Back2; } // if only one element is left return false, otherwise continue trimming - return _First1 == _Last1 ? _TrimResult::_ReturnFalse : _TrimResult::_Continue; + return _First1 == _Back1 ? _TrimResult::_ReturnFalse : _TrimResult::_Continue; } template _NODISCARD _CONSTEXPR20 _TrimResult _Trim_reversed( - _BidIt1& _First1, _BidIt1& _Last1, _BidIt2& _First2, _BidIt2& _Last2, _Pr _Pred) { - _STL_INTERNAL_CHECK(_First1 != _Last1); - _STL_INTERNAL_CHECK(_STD distance(_First1, _Last1) == _STD distance(_First2, _Last2)); - if (_Pred(*_First1, *_Last2)) { + _BidIt1& _First1, _BidIt1& _Back1, _BidIt2& _First2, _BidIt2& _Back2, _Pr _Pred) { + _STL_INTERNAL_CHECK(_First1 != _Back1); + _STL_INTERNAL_CHECK(_STD distance(_First1, _Back1) == _STD distance(_First2, _Back2)); + if (_Pred(*_First1, *_Back2)) { do { ++_First1; - --_Last2; - if (_First1 == _Last1) { + --_Back2; + if (_First1 == _Back1) { // only one element is left return _Pred(*_First1, *_First2) ? _TrimResult::_ReturnTrue : _TrimResult::_ReturnFalse; } - } while (_Pred(*_First1, *_Last2)); + } while (_Pred(*_First1, *_Back2)); } else { - if (!_Pred(*_Last1, *_First2)) { + if (!_Pred(*_Back1, *_First2)) { // nothing to trim, break return _TrimResult::_Break; } - --_Last1; + --_Back1; ++_First2; } - while (_Pred(*_Last1, *_First2)) { - --_Last1; + while (_Pred(*_Back1, *_First2)) { + --_Back1; ++_First2; } // if only one element is left return false, otherwise continue trimming - return _First1 == _Last1 ? _TrimResult::_ReturnFalse : _TrimResult::_Continue; + return _First1 == _Back1 ? _TrimResult::_ReturnFalse : _TrimResult::_Continue; } template From c9cbb5139ced2285a6e39447975072b8c02b0dc3 Mon Sep 17 00:00:00 2001 From: Adam Bucior <35536269+AdamBucior@users.noreply.github.com> Date: Fri, 27 Aug 2021 12:31:39 +0200 Subject: [PATCH 4/8] workaround --- .../tests/P0202R3_constexpr_algorithm_and_exchange/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/P0202R3_constexpr_algorithm_and_exchange/test.cpp b/tests/std/tests/P0202R3_constexpr_algorithm_and_exchange/test.cpp index 651240e036f..88b2bd1a36b 100644 --- a/tests/std/tests/P0202R3_constexpr_algorithm_and_exchange/test.cpp +++ b/tests/std/tests/P0202R3_constexpr_algorithm_and_exchange/test.cpp @@ -496,8 +496,8 @@ constexpr void test_permutations() { {40, 30, 20, 10}, }; - for (const auto& arr : expected) { - assert(is_permutation(begin(buff), end(buff), begin(arr), end(arr))); + for (size_t i = 0; i < size(expected); ++i) { + assert(is_permutation(begin(buff), end(buff), begin(expected[i]), end(expected[i]))); } size_t cursor = 0; From 6ed3f3854b8f52a64c9fab23472cfd4f3ac344a2 Mon Sep 17 00:00:00 2001 From: Adam Bucior <35536269+AdamBucior@users.noreply.github.com> Date: Fri, 24 Sep 2021 10:09:04 +0200 Subject: [PATCH 5/8] Code review --- stl/inc/algorithm | 8 ++++---- stl/inc/xutility | 34 +++++++++++++++++++++++----------- 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index c8a68bc83ad..34fbd661997 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -994,7 +994,7 @@ namespace ranges { for (;;) { _TrimResult _Res = _Trim_reversed(_First1, _Final1, _First2, _Final2, _ProjectedPred); - if (_Res != _TrimResult::_Continue) { + if (_Res != _TrimResult::_KeepTrimming) { if (_Res == _TrimResult::_Break) { break; } @@ -1002,7 +1002,7 @@ namespace ranges { } _Res = _Trim_equal(_First1, _Final1, _First2, _Final2, _ProjectedPred); - if (_Res != _TrimResult::_Continue) { + if (_Res != _TrimResult::_KeepTrimming) { if (_Res == _TrimResult::_Break) { break; } @@ -1085,7 +1085,7 @@ namespace ranges { for (;;) { _TrimResult _Res = _Trim_reversed(_First1, _Final1, _First2, _Final2, _ProjectedPred); - if (_Res != _TrimResult::_Continue) { + if (_Res != _TrimResult::_KeepTrimming) { if (_Res == _TrimResult::_Break) { break; } @@ -1093,7 +1093,7 @@ namespace ranges { } _Res = _Trim_equal(_First1, _Final1, _First2, _Final2, _ProjectedPred); - if (_Res != _TrimResult::_Continue) { + if (_Res != _TrimResult::_KeepTrimming) { if (_Res == _TrimResult::_Break) { break; } diff --git a/stl/inc/xutility b/stl/inc/xutility index 18934541686..dbea52866fd 100644 --- a/stl/inc/xutility +++ b/stl/inc/xutility @@ -5284,11 +5284,12 @@ _NODISCARD constexpr _Iter_diff_t<_InIt> _Count_pr(_InIt _First, const _InIt _La return _Count; } -enum class _TrimResult : unsigned char { _Continue, _Break, _ReturnFalse, _ReturnTrue }; +enum class _TrimResult : unsigned char { _KeepTrimming, _Break, _ReturnFalse, _ReturnTrue }; template _NODISCARD _CONSTEXPR20 _TrimResult _Trim_equal( _BidIt1& _First1, _BidIt1& _Back1, _BidIt2& _First2, _BidIt2& _Back2, _Pr _Pred) { + // advances the iterators trimming matching prefixes and suffixes from [_First1, _Back1] and [_First2, _Back2] _STL_INTERNAL_CHECK(_First1 != _Back1); _STL_INTERNAL_CHECK(_STD distance(_First1, _Back1) == _STD distance(_First2, _Back2)); if (_Pred(*_First1, *_First2)) { @@ -5309,18 +5310,24 @@ _NODISCARD _CONSTEXPR20 _TrimResult _Trim_equal( --_Back2; } - while (_Pred(*_Back1, *_Back2)) { + for (;;) { + if (_First1 == _Back1) { + // only one element is left, it can't match because it wasn't trimmed by the first loop + return _TrimResult::_ReturnFalse; + } + + if (!_Pred(*_Back1, *_Back2)) { + return _TrimResult::_KeepTrimming; + } --_Back1; --_Back2; } - - // if only one element is left return false, otherwise continue trimming - return _First1 == _Back1 ? _TrimResult::_ReturnFalse : _TrimResult::_Continue; } template _NODISCARD _CONSTEXPR20 _TrimResult _Trim_reversed( _BidIt1& _First1, _BidIt1& _Back1, _BidIt2& _First2, _BidIt2& _Back2, _Pr _Pred) { + // advances the iterators trimming prefix matching suffix from [_First1, _Back1] and [_First2, _Back2] _STL_INTERNAL_CHECK(_First1 != _Back1); _STL_INTERNAL_CHECK(_STD distance(_First1, _Back1) == _STD distance(_First2, _Back2)); if (_Pred(*_First1, *_Back2)) { @@ -5341,13 +5348,18 @@ _NODISCARD _CONSTEXPR20 _TrimResult _Trim_reversed( ++_First2; } - while (_Pred(*_Back1, *_First2)) { + for (;;) { + if (_First1 == _Back1) { + // only one element is left, it can't match because it wasn't trimmed by the first loop + return _TrimResult::_ReturnFalse; + } + + if (!_Pred(*_Back1, *_First2)) { + return _TrimResult::_KeepTrimming; + } --_Back1; ++_First2; } - - // if only one element is left return false, otherwise continue trimming - return _First1 == _Back1 ? _TrimResult::_ReturnFalse : _TrimResult::_Continue; } template @@ -5368,7 +5380,7 @@ _NODISCARD _CONSTEXPR20 bool _Check_match_counts( for (;;) { _TrimResult _Res = _Trim_reversed(_First1, _Last1, _First2, _Last2, _Pred); - if (_Res != _TrimResult::_Continue) { + if (_Res != _TrimResult::_KeepTrimming) { if (_Res == _TrimResult::_Break) { break; } @@ -5376,7 +5388,7 @@ _NODISCARD _CONSTEXPR20 bool _Check_match_counts( } _Res = _Trim_equal(_First1, _Last1, _First2, _Last2, _Pred); - if (_Res != _TrimResult::_Continue) { + if (_Res != _TrimResult::_KeepTrimming) { if (_Res == _TrimResult::_Break) { break; } From 9f1ba80d0b8c3f1c9ff79376fd33ea95df8c12ec Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 11 Nov 2021 16:37:13 -0800 Subject: [PATCH 6/8] Code review feedback: Comments. Co-authored-by: Miya Natsuhara <46756417+mnatsuhara@users.noreply.github.com> --- stl/inc/algorithm | 22 +++++++++++++++++++++- stl/inc/xutility | 6 ++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 34fbd661997..2aa826803e9 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -987,6 +987,9 @@ namespace ranges { } // If we get here, _Count > 1, initial elements do not match, and final elements do not match. + // We've trimmed matching prefixes and matching suffixes. + // Now we need to compare each range's prefix to the other range's suffix. + const auto _ProjectedPred = [&](_Ty1&& _Left, _Ty2&& _Right) { return _STD invoke(_Pred, _STD invoke(_Proj1, _STD forward<_Ty1>(_Left)), _STD invoke(_Proj2, _STD forward<_Ty2>(_Right))); @@ -1013,7 +1016,14 @@ namespace ranges { ++_Final1; ++_Final2; // If we get here, initial elements do not match, final elements do not match, and ranges have length - // at least 2 and at most _Count + // at least 2 and at most _Count. + + // We've trimmed matching prefixes, matching suffixes, + // and each range's prefix matching the other range's suffix. That is, given: + // Range 1: [A, ..., B] + // Range 2: [X, ..., Y] + // we know that A != X, A != Y, B != X, and B != Y. + // (A == B and X == Y are possible but irrelevant.) return _Match_counts(_STD move(_First1), _STD move(_Final1), _STD move(_First2), _STD move(_Final2), _Pred, _Proj1, _Proj2); @@ -1078,6 +1088,9 @@ namespace ranges { // If we get here, initial elements do not match, final elements do not match, and ranges have length // at least 2. + // We've trimmed matching prefixes and matching suffixes. + // Now we need to compare each range's prefix to the other range's suffix. + const auto _ProjectedPred = [&](_Ty1&& _Left, _Ty2&& _Right) { return _STD invoke(_Pred, _STD invoke(_Proj1, _STD forward<_Ty1>(_Left)), _STD invoke(_Proj2, _STD forward<_Ty2>(_Right))); @@ -1105,6 +1118,13 @@ namespace ranges { ++_Final2; // If we get here, initial elements do not match, final elements do not match, and ranges have length // at least 2. + + // We've trimmed matching prefixes, matching suffixes, + // and each range's prefix matching the other range's suffix. That is, given: + // Range 1: [A, ..., B] + // Range 2: [X, ..., Y] + // we know that A != X, A != Y, B != X, and B != Y. + // (A == B and X == Y are possible but irrelevant.) } return _Match_counts( diff --git a/stl/inc/xutility b/stl/inc/xutility index dbea52866fd..22fa9db12fe 100644 --- a/stl/inc/xutility +++ b/stl/inc/xutility @@ -5289,7 +5289,8 @@ enum class _TrimResult : unsigned char { _KeepTrimming, _Break, _ReturnFalse, _R template _NODISCARD _CONSTEXPR20 _TrimResult _Trim_equal( _BidIt1& _First1, _BidIt1& _Back1, _BidIt2& _First2, _BidIt2& _Back2, _Pr _Pred) { - // advances the iterators trimming matching prefixes and suffixes from [_First1, _Back1] and [_First2, _Back2] + // advances the iterators, trimming matching prefixes then matching suffixes + // from [_First1, _Back1] and [_First2, _Back2] _STL_INTERNAL_CHECK(_First1 != _Back1); _STL_INTERNAL_CHECK(_STD distance(_First1, _Back1) == _STD distance(_First2, _Back2)); if (_Pred(*_First1, *_First2)) { @@ -5327,7 +5328,8 @@ _NODISCARD _CONSTEXPR20 _TrimResult _Trim_equal( template _NODISCARD _CONSTEXPR20 _TrimResult _Trim_reversed( _BidIt1& _First1, _BidIt1& _Back1, _BidIt2& _First2, _BidIt2& _Back2, _Pr _Pred) { - // advances the iterators trimming prefix matching suffix from [_First1, _Back1] and [_First2, _Back2] + // advances the iterators, trimming each range's prefix that matches the other range's suffix + // from [_First1, _Back1] and [_First2, _Back2] _STL_INTERNAL_CHECK(_First1 != _Back1); _STL_INTERNAL_CHECK(_STD distance(_First1, _Back1) == _STD distance(_First2, _Back2)); if (_Pred(*_First1, *_Back2)) { From c4b6674902a256172d3bc4743f533c2e43aed764 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 11 Nov 2021 17:49:48 -0800 Subject: [PATCH 7/8] Code review feedback: Rework control flow. Co-authored-by: Miya Natsuhara <46756417+mnatsuhara@users.noreply.github.com> --- stl/inc/algorithm | 36 ++++++------------------------------ stl/inc/xutility | 35 ++++++++++++++++++++--------------- 2 files changed, 26 insertions(+), 45 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 2aa826803e9..1f7f40af808 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -995,22 +995,10 @@ namespace ranges { _STD invoke(_Proj2, _STD forward<_Ty2>(_Right))); }; - for (;;) { - _TrimResult _Res = _Trim_reversed(_First1, _Final1, _First2, _Final2, _ProjectedPred); - if (_Res != _TrimResult::_KeepTrimming) { - if (_Res == _TrimResult::_Break) { - break; - } - return _Res == _TrimResult::_ReturnTrue; - } + const _TrimResult _Res = _Trim_completely(_First1, _Final1, _First2, _Final2, _ProjectedPred); - _Res = _Trim_equal(_First1, _Final1, _First2, _Final2, _ProjectedPred); - if (_Res != _TrimResult::_KeepTrimming) { - if (_Res == _TrimResult::_Break) { - break; - } - return _Res == _TrimResult::_ReturnTrue; - } + if (_Res != _TrimResult::_Break) { + return _Res == _TrimResult::_ReturnTrue; } ++_Final1; @@ -1096,22 +1084,10 @@ namespace ranges { _STD invoke(_Proj2, _STD forward<_Ty2>(_Right))); }; - for (;;) { - _TrimResult _Res = _Trim_reversed(_First1, _Final1, _First2, _Final2, _ProjectedPred); - if (_Res != _TrimResult::_KeepTrimming) { - if (_Res == _TrimResult::_Break) { - break; - } - return _Res == _TrimResult::_ReturnTrue; - } + const _TrimResult _Res = _Trim_completely(_First1, _Final1, _First2, _Final2, _ProjectedPred); - _Res = _Trim_equal(_First1, _Final1, _First2, _Final2, _ProjectedPred); - if (_Res != _TrimResult::_KeepTrimming) { - if (_Res == _TrimResult::_Break) { - break; - } - return _Res == _TrimResult::_ReturnTrue; - } + if (_Res != _TrimResult::_Break) { + return _Res == _TrimResult::_ReturnTrue; } ++_Final1; diff --git a/stl/inc/xutility b/stl/inc/xutility index 22fa9db12fe..a2c6d8d1114 100644 --- a/stl/inc/xutility +++ b/stl/inc/xutility @@ -5364,6 +5364,23 @@ _NODISCARD _CONSTEXPR20 _TrimResult _Trim_reversed( } } +template +_NODISCARD _CONSTEXPR20 _TrimResult _Trim_completely( + _BidIt1& _First1, _BidIt1& _Back1, _BidIt2& _First2, _BidIt2& _Back2, _Pr _Pred) { + // alternates between calling _Trim_reversed and _Trim_equal until no more trimming is possible + _TrimResult _Res = _TrimResult::_KeepTrimming; + + for (bool _Check_reversed = true; _Res == _TrimResult::_KeepTrimming; _Check_reversed = !_Check_reversed) { + if (_Check_reversed) { + _Res = _Trim_reversed(_First1, _Back1, _First2, _Back2, _Pred); + } else { + _Res = _Trim_equal(_First1, _Back1, _First2, _Back2, _Pred); + } + } + + return _Res; +} + template _NODISCARD _CONSTEXPR20 bool _Check_match_counts( _FwdIt1 _First1, _FwdIt1 _Last1, _FwdIt2 _First2, _FwdIt2 _Last2, _Pr _Pred) { @@ -5380,22 +5397,10 @@ _NODISCARD _CONSTEXPR20 bool _Check_match_counts( return false; } - for (;;) { - _TrimResult _Res = _Trim_reversed(_First1, _Last1, _First2, _Last2, _Pred); - if (_Res != _TrimResult::_KeepTrimming) { - if (_Res == _TrimResult::_Break) { - break; - } - return _Res == _TrimResult::_ReturnTrue; - } + const _TrimResult _Res = _Trim_completely(_First1, _Last1, _First2, _Last2, _Pred); - _Res = _Trim_equal(_First1, _Last1, _First2, _Last2, _Pred); - if (_Res != _TrimResult::_KeepTrimming) { - if (_Res == _TrimResult::_Break) { - break; - } - return _Res == _TrimResult::_ReturnTrue; - } + if (_Res != _TrimResult::_Break) { + return _Res == _TrimResult::_ReturnTrue; } ++_Last1; From 6b85a3bf69cf7257a48b21aca1426609d34482be Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 11 Nov 2021 17:58:14 -0800 Subject: [PATCH 8/8] Rename _Break to _HaveWorkAfterTrimming. --- stl/inc/algorithm | 4 ++-- stl/inc/xutility | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 1f7f40af808..d322eae6840 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -997,7 +997,7 @@ namespace ranges { const _TrimResult _Res = _Trim_completely(_First1, _Final1, _First2, _Final2, _ProjectedPred); - if (_Res != _TrimResult::_Break) { + if (_Res != _TrimResult::_HaveWorkAfterTrimming) { return _Res == _TrimResult::_ReturnTrue; } @@ -1086,7 +1086,7 @@ namespace ranges { const _TrimResult _Res = _Trim_completely(_First1, _Final1, _First2, _Final2, _ProjectedPred); - if (_Res != _TrimResult::_Break) { + if (_Res != _TrimResult::_HaveWorkAfterTrimming) { return _Res == _TrimResult::_ReturnTrue; } diff --git a/stl/inc/xutility b/stl/inc/xutility index a2c6d8d1114..7bb819282c8 100644 --- a/stl/inc/xutility +++ b/stl/inc/xutility @@ -5284,7 +5284,7 @@ _NODISCARD constexpr _Iter_diff_t<_InIt> _Count_pr(_InIt _First, const _InIt _La return _Count; } -enum class _TrimResult : unsigned char { _KeepTrimming, _Break, _ReturnFalse, _ReturnTrue }; +enum class _TrimResult : unsigned char { _KeepTrimming, _HaveWorkAfterTrimming, _ReturnFalse, _ReturnTrue }; template _NODISCARD _CONSTEXPR20 _TrimResult _Trim_equal( @@ -5304,8 +5304,8 @@ _NODISCARD _CONSTEXPR20 _TrimResult _Trim_equal( } while (_Pred(*_First1, *_First2)); } else { if (!_Pred(*_Back1, *_Back2)) { - // nothing to trim, break - return _TrimResult::_Break; + // nothing to trim + return _TrimResult::_HaveWorkAfterTrimming; } --_Back1; --_Back2; @@ -5343,8 +5343,8 @@ _NODISCARD _CONSTEXPR20 _TrimResult _Trim_reversed( } while (_Pred(*_First1, *_Back2)); } else { if (!_Pred(*_Back1, *_First2)) { - // nothing to trim, break - return _TrimResult::_Break; + // nothing to trim + return _TrimResult::_HaveWorkAfterTrimming; } --_Back1; ++_First2; @@ -5399,7 +5399,7 @@ _NODISCARD _CONSTEXPR20 bool _Check_match_counts( const _TrimResult _Res = _Trim_completely(_First1, _Last1, _First2, _Last2, _Pred); - if (_Res != _TrimResult::_Break) { + if (_Res != _TrimResult::_HaveWorkAfterTrimming) { return _Res == _TrimResult::_ReturnTrue; }