From 5bd5e235f29bd582a2b3d8ef650d47fcb52832c5 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Sun, 14 Jun 2020 19:39:51 -0700 Subject: [PATCH 1/4] Implement ranges::search_n --- stl/inc/algorithm | 160 +++++++++ tests/std/include/range_algorithm_support.hpp | 330 ++++++++++-------- tests/std/test.lst | 1 + .../tests/P0896R4_ranges_alg_search_n/env.lst | 4 + .../P0896R4_ranges_alg_search_n/test.cpp | 121 +++++++ 5 files changed, 472 insertions(+), 144 deletions(-) create mode 100644 tests/std/tests/P0896R4_ranges_alg_search_n/env.lst create mode 100644 tests/std/tests/P0896R4_ranges_alg_search_n/test.cpp diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 049e11eb817..f5329a7233a 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -2223,6 +2223,166 @@ _NODISCARD _FwdIt search_n(_ExPo&& _Exec, const _FwdIt _First, const _FwdIt _Las } #endif // _HAS_CXX17 +#ifdef __cpp_lib_concepts +namespace ranges { + // VARIABLE ranges::search_n + class _Search_n_fn : private _Not_quite_object { + public: + using _Not_quite_object::_Not_quite_object; + + // clang-format off + template _Se, class _Ty, class _Pr = ranges::equal_to, + class _Pj = identity> + requires indirectly_comparable<_It, const _Ty*, _Pr, _Pj> + _NODISCARD constexpr subrange<_It> operator()(_It _First, _Se _Last, iter_difference_t<_It> _Count, + const _Ty& _Val, _Pr _Pred = {}, _Pj _Proj = {}) const { + _Adl_verify_range(_First, _Last); + + if (_Count <= 0) { + return {_First, _First}; + } + + auto _UFirst = _Get_unwrapped(_STD move(_First)); + auto _ULast = _Get_unwrapped(_STD move(_Last)); + + if constexpr (sized_sentinel_for<_Se, _It>) { + const auto _Dist = _ULast - _UFirst; + auto _UResult = _Search_n_sized(_STD move(_UFirst), _Dist, _Val, _Count, _Pass_fn(_Pred), _Pass_fn(_Proj)); + return _Rewrap_subrange>(_First, _STD move(_UResult)); + } else { + auto _UResult = _Search_n_unsized( + _STD move(_UFirst), _STD move(_ULast), _Val, _Count, _Pass_fn(_Pred), _Pass_fn(_Proj)); + return _Rewrap_subrange>(_First, _STD move(_UResult)); + } + } + + template + requires indirectly_comparable, const _Ty*, _Pr, _Pj> + _NODISCARD constexpr borrowed_subrange_t<_Rng> operator()( + _Rng&& _Range, range_difference_t<_Rng> _Count, const _Ty& _Val, _Pr _Pred = {}, _Pj _Proj = {}) const { + auto _First = _RANGES begin(_Range); + + if (_Count <= 0) { + return {_First, _First}; + } + + if constexpr (sized_range<_Rng>) { + const auto _Dist = _RANGES distance(_Range); + + auto _UResult = + _Search_n_sized(_Get_unwrapped(_First), _Dist, _Val, _Count, _Pass_fn(_Pred), _Pass_fn(_Proj)); + return _Rewrap_subrange>(_First, _STD move(_UResult)); + } else { + auto _UResult = _Search_n_unsized(_Get_unwrapped(_First), _Uend(_Range), _Val, + _Count, _Pass_fn(_Pred), _Pass_fn(_Proj)); + return _Rewrap_subrange>(_First, _STD move(_UResult)); + } + } + // clang-format on + + private: + template + _NODISCARD static constexpr subrange<_It> _Search_n_sized(_It _First, iter_difference_t<_It> _Dist, + const _Ty& _Val, const iter_difference_t<_It> _Count, _Pr _Pred, _Pj _Proj) { + _STL_INTERNAL_STATIC_ASSERT(forward_iterator<_It>); + _STL_INTERNAL_STATIC_ASSERT(indirectly_comparable<_It, const _Ty*, _Pr, _Pj>); + _STL_INTERNAL_CHECK(_Count > 0); + // pre: _First + [0, _Dist) is a valid counted range + + if constexpr (bidirectional_iterator<_It>) { + if (_Dist >= _Count) { + auto _Last = _RANGES next(_First, _Count); + auto _Mid1 = _First; + auto _Mid2 = _Last; + for (;;) { + // Invariants: _Last - _First == _Count, [_First, _Mid1) and [_Mid2, _Last) match _Val: + // + // _First _Mid1 _Mid2 _Last + // |=======|????????|========|??????... + + --_Mid2; + if (!_STD invoke(_Pred, _STD invoke(_Proj, *_Mid2), _Val)) { // mismatch; skip past it + ++_Mid2; + const auto _Delta = _RANGES distance(_First, _Mid2); + + if (_Dist - _Delta < _Count) { // not enough space left + _First = _STD move(_Last); + _Dist -= _Count; + break; + } + + _First = _STD move(_Mid2); + _Dist -= _Delta; + _Mid1 = _Last; + _RANGES advance(_Last, _Delta); + _Mid2 = _Last; + continue; + } + + if (_Mid2 == _Mid1) { // [_Mid1, _Mid2) is empty, so [_First, _Last) all match + return {_STD move(_First), _STD move(_Last)}; + } + } + } + } else { + for (; _Dist >= _Count; ++_First, (void) --_Dist) { + if (_STD invoke(_Pred, _STD invoke(_Proj, *_First), _Val)) { + auto _Saved = _First; + for (iter_difference_t<_It> _Len = 0;;) { + ++_First; + if (++_Len == _Count) { // match + return {_STD move(_Saved), _STD move(_First)}; + } + + if (!_STD invoke(_Pred, _STD invoke(_Proj, *_First), _Val)) { // mismatch + _Dist -= _Len; + break; + } + } + } + } + } + + _RANGES advance(_First, _Dist); + return {_First, _First}; + } + + template + _NODISCARD static constexpr subrange<_It> _Search_n_unsized( + _It _First, const _Se _Last, const _Ty& _Val, const iter_difference_t<_It> _Count, _Pr _Pred, _Pj _Proj) { + _STL_INTERNAL_STATIC_ASSERT(forward_iterator<_It>); + _STL_INTERNAL_STATIC_ASSERT(sentinel_for<_Se, _It>); + _STL_INTERNAL_STATIC_ASSERT(indirectly_comparable<_It, const _Ty*, _Pr, _Pj>); + _STL_INTERNAL_CHECK(_Count > 0); + + for (; _First != _Last; ++_First) { + if (_STD invoke(_Pred, _STD invoke(_Proj, *_First), _Val)) { + auto _Saved = _First; + for (auto _Len = _Count;;) { + ++_First; + if (--_Len == 0) { // match + return {_STD move(_Saved), _STD move(_First)}; + } + + if (_First == _Last) { // no more to match against + return {_First, _First}; + } + + if (!_STD invoke(_Pred, _STD invoke(_Proj, *_First), _Val)) { // mismatch + break; + } + } + } + } + + return {_First, _First}; + } + }; + + inline constexpr _Search_n_fn search_n{_Not_quite_object::_Construct_tag{}}; +} // namespace ranges +#endif // __cpp_lib_concepts + // FUNCTION TEMPLATE find_end #if _HAS_IF_CONSTEXPR template diff --git a/tests/std/include/range_algorithm_support.hpp b/tests/std/include/range_algorithm_support.hpp index ff25541c317..1f12ef6431f 100644 --- a/tests/std/include/range_algorithm_support.hpp +++ b/tests/std/include/range_algorithm_support.hpp @@ -62,8 +62,8 @@ struct boolish { }; namespace test { - using std::assignable_from, std::conditional_t, std::copy_constructible, std::derived_from, std::exchange, - std::ptrdiff_t, std::span; + using std::assignable_from, std::conditional_t, std::convertible_to, std::copy_constructible, std::derived_from, + std::exchange, std::ptrdiff_t, std::span; using output = std::output_iterator_tag; using input = std::input_iterator_tag; @@ -120,11 +120,43 @@ namespace test { } }; + // clang-format off + template + concept CanEq = requires(T const& t, U const& u) { + { t == u } -> convertible_to; + }; + + template + concept CanNEq = requires(T const& t, U const& u) { + { t != u } -> convertible_to; + }; + + template + concept CanLt = requires(T const& t, U const& u) { + { t < u } -> convertible_to; + }; + + template + concept CanLtE = requires(T const& t, U const& u) { + { t <= u } -> convertible_to; + }; + + template + concept CanGt = requires(T const& t, U const& u) { + { t > u } -> convertible_to; + }; + + template + concept CanGtE = requires(T const& t, U const& u) { + { t >= u } -> convertible_to; + }; + // clang-format on + template class proxy_reference { Element& ref_; - using ValueType = std::remove_cv_t; + using Value = std::remove_cv_t; public: constexpr explicit proxy_reference(Element& ref) : ref_{ref} {} @@ -137,34 +169,85 @@ namespace test { } // clang-format off - constexpr operator ValueType() const requires derived_from && copy_constructible { + constexpr operator Value() const requires derived_from && copy_constructible { return ref_; } // clang-format on - constexpr void operator=(ValueType const& val) const requires assignable_from { + constexpr void operator=(Value const& val) const requires assignable_from { ref_ = val; } + template + constexpr boolish operator==(proxy_reference that) const requires CanEq { + return {ref_ == that.peek()}; + } + template + constexpr boolish operator!=(proxy_reference that) const requires CanNEq { + return {ref_ != that.peek()}; + } + template + constexpr boolish operator<(proxy_reference that) const requires CanLt { + return {ref_ < that.peek()}; + } + template + constexpr boolish operator>(proxy_reference that) const requires CanGt { + return {ref_ > that.peek()}; + } + template + constexpr boolish operator<=(proxy_reference that) const requires CanLtE { + return {ref_ <= that.peek()}; + } + template + constexpr boolish operator>=(proxy_reference that) const requires CanGtE { + return {ref_ >= that.peek()}; + } + + // clang-format off + friend constexpr boolish operator==(proxy_reference ref, Value const& val) requires CanEq { + return {ref.ref_ == val}; + } + friend constexpr boolish operator==(Value const& val, proxy_reference ref) requires CanEq { + return {ref.ref_ == val}; + } + friend constexpr boolish operator!=(proxy_reference ref, Value const& val) requires CanNEq { + return {ref.ref_ != val}; + } + friend constexpr boolish operator!=(Value const& val, proxy_reference ref) requires CanNEq { + return {ref.ref_ != val}; + } + friend constexpr boolish operator<(Value const& val, proxy_reference ref) requires CanLt { + return {val < ref.ref_}; + } + friend constexpr boolish operator<(proxy_reference ref, Value const& val) requires CanLt { + return {ref.ref_ < val}; + } + friend constexpr boolish operator>(Value const& val, proxy_reference ref) requires CanGt { + return {val > ref.ref_}; + } + friend constexpr boolish operator>(proxy_reference ref, Value const& val) requires CanGt { + return {ref.ref_ > val}; + } + friend constexpr boolish operator<=(Value const& val, proxy_reference ref) requires CanLtE { + return {val <= ref.ref_}; + } + friend constexpr boolish operator<=(proxy_reference ref, Value const& val) requires CanLtE { + return {ref.ref_ <= val}; + } + friend constexpr boolish operator>=(Value const& val, proxy_reference ref) requires CanGtE { + return {val >= ref.ref_}; + } + friend constexpr boolish operator>=(proxy_reference ref, Value const& val) requires CanGtE { + return {ref.ref_ >= val}; + } + // clang-format on + constexpr Element& peek() const noexcept { return ref_; } }; // clang-format off - template - constexpr boolish operator==(proxy_reference x, proxy_reference y) requires requires { - { x.peek() == y.peek() } -> std::convertible_to; - } { - return {x.peek() == y.peek()}; - } - template - constexpr boolish operator!=(proxy_reference x, proxy_reference y) requires requires { - { x.peek() == y.peek() } -> std::convertible_to; - } { - return !(x == y); - } - template }, @@ -566,75 +649,64 @@ struct with_writable_iterators { }; template -struct with_input_ranges { +struct with_contiguous_ranges { template static constexpr void call() { using namespace test; - // For all ranges, IsCommon implies Eq. - // For single-pass ranges, Eq is uninteresting without IsCommon (there's only one valid iterator - // value at a time, and no reason to compare it with itself for equality). - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); + // Ditto always Eq; !IsSized && SizedSentinel is uninteresting (ranges::size still works), as is + // !IsSized && IsCommon. contiguous also implies !Proxy. Continuation::template call>(); - + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); + } +}; - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); +template +struct with_random_ranges { + template + static constexpr void call() { + using namespace test; - // forward always has Eq; !IsSized && Diff is uninteresting (sized_range is sized_range). - Continuation::template call>(); - Continuation::template call>(); + // Ditto always Eq; !IsSized && SizedSentinel is uninteresting (ranges::size works either way), as is + // !IsSized && IsCommon. Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); + + with_contiguous_ranges::template call(); + } +}; + +template +struct with_bidirectional_ranges { + template + static constexpr void call() { + using namespace test; // Ditto always Eq; !IsSized && Diff is uninteresting (ranges::size still works). Continuation::template call>(); - // Ditto always Eq; !IsSized && SizedSentinel is uninteresting (ranges::size works either way), as is - // !IsSized && IsCommon. - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - - // Ditto always Eq; !IsSized && SizedSentinel is uninteresting (ranges::size still works), as is - // !IsSized && IsCommon. contiguous also implies !Proxy. - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); + with_random_ranges::template call(); } }; @@ -732,67 +770,56 @@ struct with_forward_ranges { Continuation::template call>(); - // Ditto always Eq; !IsSized && Diff is uninteresting (ranges::size still works). - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); - Continuation::template call>(); + with_bidirectional_ranges::template call(); + } +}; + +template +struct with_input_ranges { + template + static constexpr void call() { + using namespace test; + + // For all ranges, IsCommon implies Eq. + // For single-pass ranges, Eq is uninteresting without IsCommon (there's only one valid iterator + // value at a time, and no reason to compare it with itself for equality). Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); - // Ditto always Eq; !IsSized && SizedSentinel is uninteresting (ranges::size works either way), as is - // !IsSized && IsCommon. - Continuation::template call>(); Continuation::template call>(); - Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); + Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); - // Ditto always Eq; !IsSized && SizedSentinel is uninteresting (ranges::size still works), as is - // !IsSized && IsCommon. contiguous also implies !Proxy. - Continuation::template call>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); Continuation::template call>(); + range>(); + + with_forward_ranges::template call(); } }; @@ -853,6 +880,21 @@ constexpr void test_fwd() { with_forward_ranges::call(); } +template +constexpr void test_bidi() { + with_bidirectional_ranges::call(); +} + +template +constexpr void test_random() { + with_random_ranges::call(); +} + +template +constexpr void test_contiguous() { + with_contiguous_ranges::call(); +} + template constexpr void test_in_in() { with_input_ranges, Element1>::call(); diff --git a/tests/std/test.lst b/tests/std/test.lst index 8672000b159..280f7ced901 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -255,6 +255,7 @@ tests\P0896R4_ranges_alg_mismatch tests\P0896R4_ranges_alg_move tests\P0896R4_ranges_alg_none_of tests\P0896R4_ranges_alg_search +tests\P0896R4_ranges_alg_search_n tests\P0896R4_ranges_iterator_machinery tests\P0896R4_ranges_range_machinery tests\P0896R4_ranges_subrange diff --git a/tests/std/tests/P0896R4_ranges_alg_search_n/env.lst b/tests/std/tests/P0896R4_ranges_alg_search_n/env.lst new file mode 100644 index 00000000000..f3ccc8613c6 --- /dev/null +++ b/tests/std/tests/P0896R4_ranges_alg_search_n/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_search_n/test.cpp b/tests/std/tests/P0896R4_ranges_alg_search_n/test.cpp new file mode 100644 index 00000000000..a1c8192a46d --- /dev/null +++ b/tests/std/tests/P0896R4_ranges_alg_search_n/test.cpp @@ -0,0 +1,121 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include +#include +#include +#include +#include +#include +// +#include + +using namespace std; + +// Validate dangling story +STATIC_ASSERT(same_as{}, 13, 42)), ranges::dangling>); +STATIC_ASSERT(same_as{}, 42, 13)), ranges::subrange>); + +using P = pair; + +struct instantiator { + static constexpr array pairs = { + P{0, 42}, P{1, 42}, P{1, 42}, P{0, 42}, P{1, 42}, P{1, 42}, P{0, 42}, P{1, 42}, P{1, 42}, P{1, 42}, P{0, 42}}; + + template + static constexpr void call() { + const Fwd range{pairs}; + + // defaulted predicate and projections + { + const auto result = ranges::search_n(range, 2, P{1, 42}); + STATIC_ASSERT(same_as>>); + assert(result.begin() == ranges::next(range.begin(), 1)); + assert(result.end() == ranges::next(range.begin(), 3)); + } + { + const auto result = ranges::search_n(ranges::begin(range), ranges::end(range), 2, P{1, 42}); + STATIC_ASSERT(same_as>>); + assert(result.begin() == ranges::next(range.begin(), 1)); + assert(result.end() == ranges::next(range.begin(), 3)); + } + + // explicit predicate + { + const auto result = ranges::search_n(range, 2, P{1, 42}, ranges::equal_to{}); + STATIC_ASSERT(same_as>>); + assert(result.begin() == ranges::next(range.begin(), 1)); + assert(result.end() == ranges::next(range.begin(), 3)); + } + { + const auto result = + ranges::search_n(ranges::begin(range), ranges::end(range), 2, P{1, 42}, ranges::equal_to{}); + STATIC_ASSERT(same_as>>); + assert(result.begin() == ranges::next(range.begin(), 1)); + assert(result.end() == ranges::next(range.begin(), 3)); + } + + // explicit predicate and one projection + constexpr auto cmp = [](auto&& x, auto&& y) { return x == y + 1; }; + { + const auto result = ranges::search_n(range, 3, 0, cmp, get_first); + STATIC_ASSERT(same_as>>); + assert(result.begin() == ranges::next(range.begin(), 7)); + assert(result.end() == ranges::next(range.begin(), 10)); + } + { + const auto result = ranges::search_n(ranges::begin(range), ranges::end(range), 3, 0, cmp, get_first); + STATIC_ASSERT(same_as>>); + assert(result.begin() == ranges::next(range.begin(), 7)); + assert(result.end() == ranges::next(range.begin(), 10)); + } + + // trivial case: empty needle + { + const auto result = ranges::search_n(range, 0, 0, cmp, get_first); + STATIC_ASSERT(same_as>>); + assert(result.begin() == range.begin()); + assert(result.end() == range.begin()); + } + { + const auto result = ranges::search_n(ranges::begin(range), ranges::end(range), 0, 0, cmp, get_first); + STATIC_ASSERT(same_as>>); + assert(result.begin() == range.begin()); + assert(result.end() == range.begin()); + } + + // trivial case: range too small + { + const auto result = ranges::search_n(range, 99999, 0, cmp, get_first); + STATIC_ASSERT(same_as>>); + assert(result.begin() == range.end()); + assert(result.end() == range.end()); + } + { + const auto result = ranges::search_n(ranges::begin(range), ranges::end(range), 99999, 0, cmp, get_first); + STATIC_ASSERT(same_as>>); + assert(result.begin() == range.end()); + assert(result.end() == range.end()); + } + + // negative case + { + const auto result = ranges::search_n(range, 2, -1, cmp, get_first); + STATIC_ASSERT(same_as>>); + assert(result.begin() == range.end()); + assert(result.end() == range.end()); + } + { + const auto result = ranges::search_n(ranges::begin(range), ranges::end(range), 2, -1, cmp, get_first); + STATIC_ASSERT(same_as>>); + assert(result.begin() == range.end()); + assert(result.end() == range.end()); + } + } +}; + +int main() { + using Elem = const P; + STATIC_ASSERT((test_fwd(), true)); + test_fwd(); +} From c7012ab62a9172f3ac65c59b72dca63fd8ced09a Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Wed, 24 Jun 2020 11:25:23 -0700 Subject: [PATCH 2/4] Casey's review comment --- tests/std/tests/P0896R4_ranges_alg_search_n/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/P0896R4_ranges_alg_search_n/test.cpp b/tests/std/tests/P0896R4_ranges_alg_search_n/test.cpp index a1c8192a46d..cb8023d278b 100644 --- a/tests/std/tests/P0896R4_ranges_alg_search_n/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_search_n/test.cpp @@ -7,7 +7,7 @@ #include #include #include -// + #include using namespace std; From 299248de71876e05db8fa5f7267a0b6288fa592c Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Fri, 26 Jun 2020 09:18:38 -0700 Subject: [PATCH 3/4] Review comments --- stl/inc/algorithm | 61 ++++++++++--------- .../P0896R4_ranges_alg_search_n/test.cpp | 2 +- 2 files changed, 33 insertions(+), 30 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index f5329a7233a..bee9c9a78df 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -2290,38 +2290,41 @@ namespace ranges { // pre: _First + [0, _Dist) is a valid counted range if constexpr (bidirectional_iterator<_It>) { - if (_Dist >= _Count) { - auto _Last = _RANGES next(_First, _Count); - auto _Mid1 = _First; - auto _Mid2 = _Last; - for (;;) { - // Invariants: _Last - _First == _Count, [_First, _Mid1) and [_Mid2, _Last) match _Val: - // - // _First _Mid1 _Mid2 _Last - // |=======|????????|========|??????... - - --_Mid2; - if (!_STD invoke(_Pred, _STD invoke(_Proj, *_Mid2), _Val)) { // mismatch; skip past it - ++_Mid2; - const auto _Delta = _RANGES distance(_First, _Mid2); - - if (_Dist - _Delta < _Count) { // not enough space left - _First = _STD move(_Last); - _Dist -= _Count; - break; - } + if (_Dist < _Count) { + _RANGES advance(_First, _Dist); + return {_First, _First}; + } - _First = _STD move(_Mid2); - _Dist -= _Delta; - _Mid1 = _Last; - _RANGES advance(_Last, _Delta); - _Mid2 = _Last; - continue; + auto _Last = _RANGES next(_First, _Count); + auto _Mid1 = _First; + auto _Mid2 = _Last; + for (;;) { + // Invariants: _Last - _First == _Count, [_First, _Mid1) and [_Mid2, _Last) match _Val: + // + // _First _Mid1 _Mid2 _Last + // |=======|????????|========|??????... + + --_Mid2; + if (!_STD invoke(_Pred, _STD invoke(_Proj, *_Mid2), _Val)) { // mismatch; skip past it + ++_Mid2; + const auto _Delta = _RANGES distance(_First, _Mid2); + + if (_Dist - _Delta < _Count) { // not enough space left + _First = _STD move(_Last); + _Dist -= _Count; + break; } - if (_Mid2 == _Mid1) { // [_Mid1, _Mid2) is empty, so [_First, _Last) all match - return {_STD move(_First), _STD move(_Last)}; - } + _First = _STD move(_Mid2); + _Dist -= _Delta; + _Mid1 = _Last; + _RANGES advance(_Last, _Delta); + _Mid2 = _Last; + continue; + } + + if (_Mid2 == _Mid1) { // [_Mid1, _Mid2) is empty, so [_First, _Last) all match + return {_STD move(_First), _STD move(_Last)}; } } } else { diff --git a/tests/std/tests/P0896R4_ranges_alg_search_n/test.cpp b/tests/std/tests/P0896R4_ranges_alg_search_n/test.cpp index cb8023d278b..973e40ff138 100644 --- a/tests/std/tests/P0896R4_ranges_alg_search_n/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_search_n/test.cpp @@ -55,7 +55,7 @@ struct instantiator { assert(result.end() == ranges::next(range.begin(), 3)); } - // explicit predicate and one projection + // explicit predicate and projection constexpr auto cmp = [](auto&& x, auto&& y) { return x == y + 1; }; { const auto result = ranges::search_n(range, 3, 0, cmp, get_first); From 512f690ad4b57d42894d1196b0a76ae5ea26c33e Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Sat, 27 Jun 2020 13:55:03 -0700 Subject: [PATCH 4/4] Review comments --- stl/inc/algorithm | 4 +-- .../P0896R4_ranges_alg_search_n/test.cpp | 28 ++++++++++++++----- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index bee9c9a78df..31312cc941e 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -2234,7 +2234,7 @@ namespace ranges { template _Se, class _Ty, class _Pr = ranges::equal_to, class _Pj = identity> requires indirectly_comparable<_It, const _Ty*, _Pr, _Pj> - _NODISCARD constexpr subrange<_It> operator()(_It _First, _Se _Last, iter_difference_t<_It> _Count, + _NODISCARD constexpr subrange<_It> operator()(_It _First, _Se _Last, const iter_difference_t<_It> _Count, const _Ty& _Val, _Pr _Pred = {}, _Pj _Proj = {}) const { _Adl_verify_range(_First, _Last); @@ -2259,7 +2259,7 @@ namespace ranges { template requires indirectly_comparable, const _Ty*, _Pr, _Pj> _NODISCARD constexpr borrowed_subrange_t<_Rng> operator()( - _Rng&& _Range, range_difference_t<_Rng> _Count, const _Ty& _Val, _Pr _Pred = {}, _Pj _Proj = {}) const { + _Rng&& _Range, const range_difference_t<_Rng> _Count, const _Ty& _Val, _Pr _Pred = {}, _Pj _Proj = {}) const { auto _First = _RANGES begin(_Range); if (_Count <= 0) { diff --git a/tests/std/tests/P0896R4_ranges_alg_search_n/test.cpp b/tests/std/tests/P0896R4_ranges_alg_search_n/test.cpp index 973e40ff138..03eceb8fce2 100644 --- a/tests/std/tests/P0896R4_ranges_alg_search_n/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_search_n/test.cpp @@ -70,6 +70,20 @@ struct instantiator { assert(result.end() == ranges::next(range.begin(), 10)); } + // negative case + { + const auto result = ranges::search_n(range, 4, 0, cmp, get_first); + STATIC_ASSERT(same_as>>); + assert(result.begin() == range.end()); + assert(result.end() == range.end()); + } + { + const auto result = ranges::search_n(ranges::begin(range), ranges::end(range), 4, 0, cmp, get_first); + STATIC_ASSERT(same_as>>); + assert(result.begin() == range.end()); + assert(result.end() == range.end()); + } + // trivial case: empty needle { const auto result = ranges::search_n(range, 0, 0, cmp, get_first); @@ -98,18 +112,18 @@ struct instantiator { assert(result.end() == range.end()); } - // negative case + // trivial case: negative count { - const auto result = ranges::search_n(range, 2, -1, cmp, get_first); + const auto result = ranges::search_n(range, -42, 0, cmp, get_first); STATIC_ASSERT(same_as>>); - assert(result.begin() == range.end()); - assert(result.end() == range.end()); + assert(result.begin() == range.begin()); + assert(result.end() == range.begin()); } { - const auto result = ranges::search_n(ranges::begin(range), ranges::end(range), 2, -1, cmp, get_first); + const auto result = ranges::search_n(ranges::begin(range), ranges::end(range), -42, 0, cmp, get_first); STATIC_ASSERT(same_as>>); - assert(result.begin() == range.end()); - assert(result.end() == range.end()); + assert(result.begin() == range.begin()); + assert(result.end() == range.begin()); } } };