From 672b8a32ddeb1d14949efbfa7558309f5fd8dd60 Mon Sep 17 00:00:00 2001 From: Ahana Mukhopadhyay Date: Thu, 18 Jun 2020 11:31:13 -0400 Subject: [PATCH 1/3] Implement ranges::reverse --- stl/inc/algorithm | 71 +++++++++++++++++++ tests/std/test.lst | 1 + .../tests/P0896R4_ranges_alg_reverse/env.lst | 4 ++ .../tests/P0896R4_ranges_alg_reverse/test.cpp | 49 +++++++++++++ 4 files changed, 125 insertions(+) create mode 100644 tests/std/tests/P0896R4_ranges_alg_reverse/env.lst create mode 100644 tests/std/tests/P0896R4_ranges_alg_reverse/test.cpp diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 13801421fc7..335a2254627 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -3567,6 +3567,77 @@ _FwdIt2 unique_copy(_ExPo&&, _FwdIt1 _First, _FwdIt1 _Last, _FwdIt2 _Dest) noexc } #endif // _HAS_CXX17 +#ifdef __cpp_lib_concepts +namespace ranges { + // VARIABLE ranges::reverse + class _Reverse_fn : private _Not_quite_object { + public: + using _Not_quite_object::_Not_quite_object; + + // clang-format off + template _Se> + requires permutable<_It> + constexpr _It operator()(_It _First, _Se _Last) const { + _Adl_verify_range(_First, _Last); + auto _UFirst = _Get_unwrapped(_STD move(_First)); + auto _ULast = _Get_final_iterator_unwrapped<_It>(_UFirst, _STD move(_Last)); + _Seek_wrapped(_First, _ULast); + _Reverse_unchecked(_STD move(_UFirst), _STD move(_ULast)); + return _First; + } + + template + requires permutable> + constexpr borrowed_iterator_t<_Rng> operator()(_Rng&& _Range) const { + auto _ULast = _Get_final_iterator_unwrapped(_Range); + _Reverse_unchecked(_Ubegin(_Range), _ULast); + return _Rewrap_iterator(_Range, _STD move(_ULast)); + } + // clang-format on + private: + template + static constexpr void _Reverse_unchecked(_It _First, _It _Last) { + _STL_INTERNAL_STATIC_ASSERT(bidirectional_iterator<_It>); + _STL_INTERNAL_STATIC_ASSERT(permutable<_It>); + +#if _USE_STD_VECTOR_ALGORITHMS + using _Elem = remove_pointer_t<_It>; + constexpr bool _Allow_vectorization = + contiguous_iterator<_It> && conjunction_v<_Is_trivially_swappable<_Elem>, negation>>; + + if constexpr (_Allow_vectorization && sizeof(_Elem) == 1) { + if (!_STD is_constant_evaluated()) { + __std_reverse_trivially_swappable_1(_STD to_address(_First), _STD to_address(_Last)); + return; + } + } else if constexpr (_Allow_vectorization && sizeof(_Elem) == 2) { + if (!_STD is_constant_evaluated()) { + __std_reverse_trivially_swappable_2(_STD to_address(_First), _STD to_address(_Last)); + return; + } + } else if constexpr (_Allow_vectorization && sizeof(_Elem) == 4) { + if (!_STD is_constant_evaluated()) { + __std_reverse_trivially_swappable_4(_STD to_address(_First), _STD to_address(_Last)); + return; + } + } else if constexpr (_Allow_vectorization && sizeof(_Elem) == 8) { + if (!_STD is_constant_evaluated()) { + __std_reverse_trivially_swappable_8(_STD to_address(_First), _STD to_address(_Last)); + return; + } + } +#endif // _USE_STD_VECTOR_ALGORITHMS + + for (; _First != _Last && _First != --_Last; ++_First) { + _RANGES iter_swap(_First, _Last); + } + } + }; + + inline constexpr _Reverse_fn reverse{_Not_quite_object::_Construct_tag{}}; +} // namespace ranges +#endif // __cpp_lib_concepts + // FUNCTION TEMPLATE reverse_copy template _CONSTEXPR20 _OutIt reverse_copy(_BidIt _First, _BidIt _Last, _OutIt _Dest) { diff --git a/tests/std/test.lst b/tests/std/test.lst index 98930beaa8b..577ea9565bf 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -262,6 +262,7 @@ tests\P0896R4_ranges_alg_mismatch tests\P0896R4_ranges_alg_move tests\P0896R4_ranges_alg_none_of tests\P0896R4_ranges_alg_replace_if +tests\P0896R4_ranges_alg_reverse 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_reverse/env.lst b/tests/std/tests/P0896R4_ranges_alg_reverse/env.lst new file mode 100644 index 00000000000..f3ccc8613c6 --- /dev/null +++ b/tests/std/tests/P0896R4_ranges_alg_reverse/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_reverse/test.cpp b/tests/std/tests/P0896R4_ranges_alg_reverse/test.cpp new file mode 100644 index 00000000000..2cc693ebf40 --- /dev/null +++ b/tests/std/tests/P0896R4_ranges_alg_reverse/test.cpp @@ -0,0 +1,49 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include +#include +#include +#include +#include + +#include + +using namespace std; + +// Validate dangling story +STATIC_ASSERT(same_as{})), ranges::dangling>); +STATIC_ASSERT(same_as{})), int*>); + +struct instantiator { + static constexpr int expected[] = {1367, 42, 13}; + + template + static constexpr void call() { + using ranges::reverse, ranges::equal, ranges::iterator_t; + + { // Validate iterator + sentinel overload + int input[] = {13, 42, 1367}; + Bidi wrapped_input{input}; + auto result = reverse(wrapped_input.begin(), wrapped_input.end()); + STATIC_ASSERT(same_as>); + assert(result == wrapped_input.end()); + assert(equal(input, expected)); + } + { // Validate range overload + int input[] = {13, 42, 1367}; + Bidi wrapped_input{input}; + auto result = reverse(wrapped_input); + STATIC_ASSERT(same_as>); + assert(result == wrapped_input.end()); + assert(equal(input, expected)); + } + } +}; + +int main() { +#if defined(__clang__) || defined(__EDG__) // TRANSITION, VSO-938163 + STATIC_ASSERT((test_bidi(), true)); +#endif // TRANSITION, VSO-938163 + test_bidi(); +} From 4167b5083822ef4d69a19b0608846bf909275fb4 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Mon, 13 Jul 2020 11:39:04 -0700 Subject: [PATCH 2/3] Review comments --- stl/inc/algorithm | 80 ++++----- .../tests/P0896R4_ranges_alg_reverse/test.cpp | 162 ++++++++++++++++-- 2 files changed, 188 insertions(+), 54 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 335a2254627..4daaed5bed0 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -3570,6 +3570,44 @@ _FwdIt2 unique_copy(_ExPo&&, _FwdIt1 _First, _FwdIt1 _Last, _FwdIt2 _Dest) noexc #ifdef __cpp_lib_concepts namespace ranges { // VARIABLE ranges::reverse + // clang-format off XXX + // concept-constrained for strict enforcement as it is used by several algorithms + template + // requires permutable<_It> + constexpr void _Reverse_common(_It _First, _It _Last) { +#if _USE_STD_VECTOR_ALGORITHMS + if constexpr (contiguous_iterator<_It>) { + using _Elem = remove_reference_t>; + constexpr size_t _Nx = sizeof(_Elem); + constexpr bool _Allow_vectorization = + conjunction_v<_Is_trivially_swappable<_Elem>, negation>>; + + if constexpr (_Allow_vectorization && _Nx <= 8 && (_Nx & (_Nx - 1)) == 0) { + if (!_STD is_constant_evaluated()) { + _Elem* const _First_addr = _STD to_address(_First); + _Elem* const _Last_addr = _STD to_address(_Last); + if constexpr (_Nx == 1) { + __std_reverse_trivially_swappable_1(_First_addr, _Last_addr); + } else if constexpr (_Nx == 2) { + __std_reverse_trivially_swappable_2(_First_addr, _Last_addr); + } else if constexpr (_Nx == 4) { + __std_reverse_trivially_swappable_4(_First_addr, _Last_addr); + } else { + __std_reverse_trivially_swappable_8(_First_addr, _Last_addr); + } + + return; + } + } + } +#endif // _USE_STD_VECTOR_ALGORITHMS + + for (; _First != _Last && _First != --_Last; ++_First) { + _RANGES iter_swap(_First, _Last); + } + } + // clang-format on + class _Reverse_fn : private _Not_quite_object { public: using _Not_quite_object::_Not_quite_object; @@ -3582,7 +3620,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); - _Reverse_unchecked(_STD move(_UFirst), _STD move(_ULast)); + _RANGES _Reverse_common(_STD move(_UFirst), _STD move(_ULast)); return _First; } @@ -3590,48 +3628,10 @@ namespace ranges { requires permutable> constexpr borrowed_iterator_t<_Rng> operator()(_Rng&& _Range) const { auto _ULast = _Get_final_iterator_unwrapped(_Range); - _Reverse_unchecked(_Ubegin(_Range), _ULast); + _RANGES _Reverse_common(_Ubegin(_Range), _ULast); return _Rewrap_iterator(_Range, _STD move(_ULast)); } // clang-format on - private: - template - static constexpr void _Reverse_unchecked(_It _First, _It _Last) { - _STL_INTERNAL_STATIC_ASSERT(bidirectional_iterator<_It>); - _STL_INTERNAL_STATIC_ASSERT(permutable<_It>); - -#if _USE_STD_VECTOR_ALGORITHMS - using _Elem = remove_pointer_t<_It>; - constexpr bool _Allow_vectorization = - contiguous_iterator<_It> && conjunction_v<_Is_trivially_swappable<_Elem>, negation>>; - - if constexpr (_Allow_vectorization && sizeof(_Elem) == 1) { - if (!_STD is_constant_evaluated()) { - __std_reverse_trivially_swappable_1(_STD to_address(_First), _STD to_address(_Last)); - return; - } - } else if constexpr (_Allow_vectorization && sizeof(_Elem) == 2) { - if (!_STD is_constant_evaluated()) { - __std_reverse_trivially_swappable_2(_STD to_address(_First), _STD to_address(_Last)); - return; - } - } else if constexpr (_Allow_vectorization && sizeof(_Elem) == 4) { - if (!_STD is_constant_evaluated()) { - __std_reverse_trivially_swappable_4(_STD to_address(_First), _STD to_address(_Last)); - return; - } - } else if constexpr (_Allow_vectorization && sizeof(_Elem) == 8) { - if (!_STD is_constant_evaluated()) { - __std_reverse_trivially_swappable_8(_STD to_address(_First), _STD to_address(_Last)); - return; - } - } -#endif // _USE_STD_VECTOR_ALGORITHMS - - for (; _First != _Last && _First != --_Last; ++_First) { - _RANGES iter_swap(_First, _Last); - } - } }; inline constexpr _Reverse_fn reverse{_Not_quite_object::_Construct_tag{}}; diff --git a/tests/std/tests/P0896R4_ranges_alg_reverse/test.cpp b/tests/std/tests/P0896R4_ranges_alg_reverse/test.cpp index 2cc693ebf40..53d0251a472 100644 --- a/tests/std/tests/P0896R4_ranges_alg_reverse/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_reverse/test.cpp @@ -3,7 +3,9 @@ #include #include +#include #include +#include #include #include @@ -15,35 +17,167 @@ using namespace std; STATIC_ASSERT(same_as{})), ranges::dangling>); STATIC_ASSERT(same_as{})), int*>); +struct nontrivial_int { + int val; + + constexpr nontrivial_int(int i) noexcept : val{i} {} + constexpr nontrivial_int(const nontrivial_int& that) noexcept : val{that.val} {} + constexpr nontrivial_int& operator=(const nontrivial_int& that) noexcept { + val = that.val; + return *this; + } + + auto operator<=>(const nontrivial_int&) const = default; +}; + struct instantiator { - static constexpr int expected[] = {1367, 42, 13}; + static constexpr nontrivial_int expected_odd[] = {1367, 42, 13}; + static constexpr nontrivial_int expected_even[] = {1729, 1367, 42, 13}; - template + template static constexpr void call() { using ranges::reverse, ranges::equal, ranges::iterator_t; - { // Validate iterator + sentinel overload - int input[] = {13, 42, 1367}; - Bidi wrapped_input{input}; + { // Validate iterator + sentinel overload, odd length + nontrivial_int input[] = {13, 42, 1367}; + R wrapped_input{input}; + auto result = reverse(wrapped_input.begin(), wrapped_input.end()); + STATIC_ASSERT(same_as>); + assert(result == wrapped_input.end()); + assert(equal(input, expected_odd)); + } + { // Validate range overload, odd length + nontrivial_int input[] = {13, 42, 1367}; + R wrapped_input{input}; + auto result = reverse(wrapped_input); + STATIC_ASSERT(same_as>); + assert(result == wrapped_input.end()); + assert(equal(input, expected_odd)); + } + { // Validate iterator + sentinel overload, even length + nontrivial_int input[] = {13, 42, 1367, 1729}; + R wrapped_input{input}; + auto result = reverse(wrapped_input.begin(), wrapped_input.end()); + STATIC_ASSERT(same_as>); + assert(result == wrapped_input.end()); + assert(equal(input, expected_even)); + } + { // Validate range overload, even length + nontrivial_int input[] = {13, 42, 1367, 1729}; + R wrapped_input{input}; + auto result = reverse(wrapped_input); + STATIC_ASSERT(same_as>); + assert(result == wrapped_input.end()); + assert(equal(input, expected_even)); + } + { // Validate iterator + sentinel overload, empty range + R wrapped_input{}; auto result = reverse(wrapped_input.begin(), wrapped_input.end()); - STATIC_ASSERT(same_as>); + STATIC_ASSERT(same_as>); assert(result == wrapped_input.end()); - assert(equal(input, expected)); } - { // Validate range overload - int input[] = {13, 42, 1367}; - Bidi wrapped_input{input}; + { // Validate range overload, empty range + R wrapped_input{}; auto result = reverse(wrapped_input); - STATIC_ASSERT(same_as>); + STATIC_ASSERT(same_as>); + assert(result == wrapped_input.end()); + } + } +}; + +template +struct bytes { + unsigned char storage[N]; + + constexpr bytes(unsigned char base) { + iota(storage, storage + N, base); + } + + bool operator==(const bytes&) const = default; +}; + +struct test_vector { + template + static constexpr void call() { + using ranges::reverse, ranges::equal, ranges::iterator_t; + + { // Validate iterator + sentinel overload, vectorizable odd length + ranges::range_value_t input[]{0x10, 0x20, 0x30}; + R wrapped_input{input}; + auto result = reverse(wrapped_input.begin(), wrapped_input.end()); + STATIC_ASSERT(same_as>); + assert(result == wrapped_input.end()); + assert(equal(input, initializer_list>{0x30, 0x20, 0x10})); + } + { // Validate range overload, vectorizable odd length + ranges::range_value_t input[]{0x10, 0x20, 0x30}; + R wrapped_input{input}; + auto result = reverse(wrapped_input); + STATIC_ASSERT(same_as>); + assert(result == wrapped_input.end()); + assert(equal(input, initializer_list>{0x30, 0x20, 0x10})); + } + + { // Validate iterator + sentinel overload, vectorizable even length + ranges::range_value_t input[]{0x10, 0x20, 0x30, 0x40}; + R wrapped_input{input}; + auto result = reverse(wrapped_input.begin(), wrapped_input.end()); + STATIC_ASSERT(same_as>); + assert(result == wrapped_input.end()); + assert(equal(input, initializer_list>{0x40, 0x30, 0x20, 0x10})); + } + { // Validate range overload, vectorizable even length + ranges::range_value_t input[]{0x10, 0x20, 0x30, 0x40}; + R wrapped_input{input}; + auto result = reverse(wrapped_input); + STATIC_ASSERT(same_as>); + assert(result == wrapped_input.end()); + assert(equal(input, initializer_list>{0x40, 0x30, 0x20, 0x10})); + } + + { // Validate iterator + sentinel overload, vectorizable empty + R wrapped_input{}; + auto result = reverse(wrapped_input.begin(), wrapped_input.end()); + STATIC_ASSERT(same_as>); + assert(result == wrapped_input.end()); + } + { // Validate range overload, vectorizable empty + R wrapped_input{}; + auto result = reverse(wrapped_input); + STATIC_ASSERT(same_as>); assert(result == wrapped_input.end()); - assert(equal(input, expected)); } } }; int main() { #if defined(__clang__) || defined(__EDG__) // TRANSITION, VSO-938163 - STATIC_ASSERT((test_bidi(), true)); + STATIC_ASSERT((test_bidi(), true)); +#endif // TRANSITION, VSO-938163 + test_bidi(); + +#if defined(__clang__) || defined(__EDG__) // TRANSITION, VSO-938163 + STATIC_ASSERT((test_contiguous>(), true)); +#endif // TRANSITION, VSO-938163 + test_contiguous>(); + +#if defined(__clang__) || defined(__EDG__) // TRANSITION, VSO-938163 + STATIC_ASSERT((test_contiguous>(), true)); +#endif // TRANSITION, VSO-938163 + test_contiguous>(); + +#if defined(__clang__) || defined(__EDG__) // TRANSITION, VSO-938163 + STATIC_ASSERT((test_contiguous>(), true)); +#endif // TRANSITION, VSO-938163 + test_contiguous>(); + +#if defined(__clang__) || defined(__EDG__) // TRANSITION, VSO-938163 + STATIC_ASSERT((test_contiguous>(), true)); +#endif // TRANSITION, VSO-938163 + test_contiguous>(); + +#if defined(__clang__) || defined(__EDG__) // TRANSITION, VSO-938163 + STATIC_ASSERT((test_contiguous>(), true)); #endif // TRANSITION, VSO-938163 - test_bidi(); + test_contiguous>(); } From 91d26d71fdf3377ace702e719b38fae86c438253 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Mon, 13 Jul 2020 17:36:23 -0700 Subject: [PATCH 3/3] Re-constrain and disable clang-format for _Reverse_common --- stl/inc/algorithm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 4daaed5bed0..8b95b15b57e 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -3570,10 +3570,10 @@ _FwdIt2 unique_copy(_ExPo&&, _FwdIt1 _First, _FwdIt1 _Last, _FwdIt2 _Dest) noexc #ifdef __cpp_lib_concepts namespace ranges { // VARIABLE ranges::reverse - // clang-format off XXX + // clang-format off // concept-constrained for strict enforcement as it is used by several algorithms template - // requires permutable<_It> + requires permutable<_It> constexpr void _Reverse_common(_It _First, _It _Last) { #if _USE_STD_VECTOR_ALGORITHMS if constexpr (contiguous_iterator<_It>) {