From 06f66ebe59bcaf1ea16750d5db559d0c93abaa3e Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Wed, 1 Jul 2020 13:01:59 +0200 Subject: [PATCH 01/10] Modernize ranges::copy --- stl/inc/algorithm | 24 +++--- .../tests/P0896R4_ranges_alg_copy/test.cpp | 78 +++++++++---------- 2 files changed, 52 insertions(+), 50 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 59db8eb1959..61efedbc0c6 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -1516,22 +1516,28 @@ namespace ranges { requires indirectly_copyable<_It, _Out> constexpr copy_result<_It, _Out> operator()(_It _First, _Se _Last, _Out _Result) const { _Adl_verify_range(_First, _Last); - auto _UFirst = _Get_unwrapped(_STD move(_First)); - const auto _ULast = _Get_unwrapped(_STD move(_Last)); - for (; _UFirst != _ULast; ++_UFirst, (void) ++_Result) { - *_Result = *_UFirst; - } - - _Seek_wrapped(_First, _STD move(_UFirst)); - return {_STD move(_First), _STD move(_Result)}; + auto _Res = _Copy_unchecked(_Get_unwrapped(_STD move(_First)), _Get_unwrapped(_STD move(_Last)), _STD move(_Result)); + _Seek_wrapped(_First, _STD move(_Res.in)); + return {_STD move(_First), _STD move(_Res.out)}; } template requires indirectly_copyable, _Out> constexpr copy_result, _Out> operator()(_Rng&& _Range, _Out _Result) const { - return (*this)(_RANGES begin(_Range), _RANGES end(_Range), _STD move(_Result)); + auto _Res = _Copy_unchecked(_Ubegin(_Range), _Uend(_Range), _STD move(_Result)); + auto _First = _RANGES begin(_Range); + _Seek_wrapped(_First, _STD move(_Res.in)); + return {_STD move(_First), _STD move(_Res.out)}; } // clang-format on + private: + template + _NODISCARD constexpr copy_result<_It, _Out> _Copy_unchecked(_It _First, _Se _Last, _Out _Result) const { + for (; _First != _Last; ++_First, (void) ++_Result) { + *_Result = *_First; + } + return {_STD move(_First), _STD move(_Result)}; + } }; inline constexpr _Copy_fn copy{_Not_quite_object::_Construct_tag{}}; diff --git a/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp b/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp index cb900599857..99df59a5526 100644 --- a/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp @@ -9,51 +9,47 @@ #include -constexpr void smoke_test() { - using ranges::copy, ranges::copy_result, ranges::iterator_t; - using std::same_as; +using ranges::copy, ranges::copy_result, ranges::iterator_t; +using std::same_as; - // Validate that copy_result aliases in_out_result - STATIC_ASSERT(same_as, ranges::in_out_result>); +// Validate that copy_result aliases in_out_result +STATIC_ASSERT(same_as, ranges::in_out_result>); - // Validate dangling story - STATIC_ASSERT( - same_as{}, static_cast(nullptr))), copy_result>); - STATIC_ASSERT(same_as{}, static_cast(nullptr))), copy_result>); - - int const input[] = {13, 42, 1729}; - { // Validate range overload - int output[] = {-1, -1, -1}; - auto result = copy(basic_borrowed_range{input}, basic_borrowed_range{output}.begin()); - STATIC_ASSERT(same_as>, iterator_t>>>); - assert(result.in == basic_borrowed_range{input}.end()); - assert(result.out == basic_borrowed_range{output}.end()); - assert(ranges::equal(output, input)); - } - { // Validate iterator + sentinel overload - int output[] = {-1, -1, -1}; - basic_borrowed_range wrapped_input{input}; - auto result = copy(wrapped_input.begin(), wrapped_input.end(), basic_borrowed_range{output}.begin()); - STATIC_ASSERT(same_as>, iterator_t>>>); - assert(result.in == wrapped_input.end()); - assert(result.out == basic_borrowed_range{output}.end()); - assert(ranges::equal(output, input)); - } -} - -int main() { - STATIC_ASSERT((smoke_test(), true)); - smoke_test(); -} +// Validate dangling story +STATIC_ASSERT( + same_as{}, static_cast(nullptr))), copy_result>); +STATIC_ASSERT(same_as{}, static_cast(nullptr))), copy_result>); struct instantiator { - template - static void call(In&& in = {}, Out out = {}) { - (void) ranges::copy(in, std::move(out)); - (void) ranges::copy(ranges::begin(in), ranges::end(in), std::move(out)); + static constexpr int input[3] = {13, 42, 1729}; + template + static constexpr void call() { + { // Validate iterator + sentinel overload + int output[3] = {-1, -1, -1}; + In wrapped_input{input}; + auto result = copy(wrapped_input.begin(), wrapped_input.end(), Write{output}); + STATIC_ASSERT(same_as, Write>>); + assert(result.in == wrapped_input.end()); + if constexpr (std::equality_comparable) { + assert(result.out == Write{output + 3}); + } + assert(ranges::equal(output, input)); + } + { // Validate range overload + int output[3] = {-1, -1, -1}; + In wrapped_input{input}; + auto result = copy(wrapped_input, Write{output}); + STATIC_ASSERT(same_as, Write>>); + assert(result.in == wrapped_input.end()); + if constexpr (std::equality_comparable) { + assert(result.out == Write{output + 3}); + } + assert(ranges::equal(output, input)); + } } }; -template void test_in_write(); +int main() { + STATIC_ASSERT((test_in_write(), true)); + test_in_write(); +} From bb061becf54ddb4157d7fcda419b449c2d4e19a4 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Wed, 1 Jul 2020 22:38:23 +0200 Subject: [PATCH 02/10] Assert the requirements Co-authored-by: Casey Carter --- stl/inc/algorithm | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 61efedbc0c6..5bbb4d77d53 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -1533,6 +1533,11 @@ namespace ranges { private: template _NODISCARD constexpr copy_result<_It, _Out> _Copy_unchecked(_It _First, _Se _Last, _Out _Result) const { + _STL_INTERNAL_STATIC_ASSERT(input_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>); + for (; _First != _Last; ++_First, (void) ++_Result) { *_Result = *_First; } From b4cd432078344f56e417bf8315eafeb6e93e6887 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Wed, 1 Jul 2020 22:38:55 +0200 Subject: [PATCH 03/10] whitespace change Co-authored-by: Casey Carter --- tests/std/tests/P0896R4_ranges_alg_copy/test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp b/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp index 99df59a5526..7007ec5d02b 100644 --- a/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp @@ -22,6 +22,7 @@ STATIC_ASSERT(same_as{}, static_cast(nullptr) struct instantiator { static constexpr int input[3] = {13, 42, 1729}; + template static constexpr void call() { { // Validate iterator + sentinel overload From 2803624d774d130bfc3d6becf6e1ddcc51c3ad15 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Wed, 1 Jul 2020 22:44:54 +0200 Subject: [PATCH 04/10] Apply suggestions from code review Co-authored-by: Casey Carter --- tests/std/tests/P0896R4_ranges_alg_copy/test.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp b/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp index 7007ec5d02b..2267f725a14 100644 --- a/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp @@ -31,9 +31,7 @@ struct instantiator { auto result = copy(wrapped_input.begin(), wrapped_input.end(), Write{output}); STATIC_ASSERT(same_as, Write>>); assert(result.in == wrapped_input.end()); - if constexpr (std::equality_comparable) { - assert(result.out == Write{output + 3}); - } + assert(result.out.base() == output + 3); assert(ranges::equal(output, input)); } { // Validate range overload From 7281abd799c8b8bfc2cae9dadd43f43d4f50a68d Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Wed, 1 Jul 2020 23:05:41 +0200 Subject: [PATCH 05/10] Make Casey happy --- stl/inc/algorithm | 21 +++++++++++-------- .../tests/P0896R4_ranges_alg_copy/test.cpp | 11 +++++----- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 5bbb4d77d53..b740d54a9dc 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -1516,23 +1516,25 @@ namespace ranges { requires indirectly_copyable<_It, _Out> constexpr copy_result<_It, _Out> operator()(_It _First, _Se _Last, _Out _Result) const { _Adl_verify_range(_First, _Last); - auto _Res = _Copy_unchecked(_Get_unwrapped(_STD move(_First)), _Get_unwrapped(_STD move(_Last)), _STD move(_Result)); - _Seek_wrapped(_First, _STD move(_Res.in)); - return {_STD move(_First), _STD move(_Res.out)}; + auto _UResult = _Copy_unchecked( + _Get_unwrapped(_STD move(_First)), _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> + // requires indirectly_copyable, _Out> constexpr copy_result, _Out> operator()(_Rng&& _Range, _Out _Result) const { - auto _Res = _Copy_unchecked(_Ubegin(_Range), _Uend(_Range), _STD move(_Result)); - auto _First = _RANGES begin(_Range); - _Seek_wrapped(_First, _STD move(_Res.in)); - return {_STD move(_First), _STD move(_Res.out)}; + auto _UResult = _Copy_unchecked(_Ubegin(_Range), _Uend(_Range), _STD move(_Result)); + auto _First = _RANGES begin(_Range); + _Seek_wrapped(_First, _STD move(_UResult.in)); + return {_STD move(_First), _STD move(_UResult.out)}; } // clang-format on + private: template - _NODISCARD constexpr copy_result<_It, _Out> _Copy_unchecked(_It _First, _Se _Last, _Out _Result) const { + _NODISCARD static constexpr copy_result<_It, _Out> _Copy_unchecked(_It _First, const _Se _Last, _Out _Result) { _STL_INTERNAL_STATIC_ASSERT(input_iterator<_It>); _STL_INTERNAL_STATIC_ASSERT(sentinel_for<_Se, _It>); _STL_INTERNAL_STATIC_ASSERT(weakly_incrementable<_Out>); @@ -1541,6 +1543,7 @@ namespace ranges { for (; _First != _Last; ++_First, (void) ++_Result) { *_Result = *_First; } + return {_STD move(_First), _STD move(_Result)}; } }; diff --git a/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp b/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp index 2267f725a14..87439f138af 100644 --- a/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp @@ -9,8 +9,9 @@ #include +using namespace std; +using same_as; using ranges::copy, ranges::copy_result, ranges::iterator_t; -using std::same_as; // Validate that copy_result aliases in_out_result STATIC_ASSERT(same_as, ranges::in_out_result>); @@ -40,15 +41,13 @@ struct instantiator { auto result = copy(wrapped_input, Write{output}); STATIC_ASSERT(same_as, Write>>); assert(result.in == wrapped_input.end()); - if constexpr (std::equality_comparable) { - assert(result.out == Write{output + 3}); - } + assert(result.out.base() == output + 3); assert(ranges::equal(output, input)); } } }; int main() { - STATIC_ASSERT((test_in_write(), true)); - test_in_write(); + STATIC_ASSERT((test_in_write(), true)); + test_in_write(); } From 565660af4f04fd7df331f686ea66d6fdaf52ed92 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Wed, 1 Jul 2020 23:20:28 +0200 Subject: [PATCH 06/10] beware the input ranges --- stl/inc/algorithm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index b740d54a9dc..67da44550df 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -1523,10 +1523,10 @@ namespace ranges { } template - // requires indirectly_copyable, _Out> + requires indirectly_copyable, _Out> constexpr copy_result, _Out> operator()(_Rng&& _Range, _Out _Result) const { - auto _UResult = _Copy_unchecked(_Ubegin(_Range), _Uend(_Range), _STD move(_Result)); auto _First = _RANGES begin(_Range); + auto _UResult = _Copy_unchecked(_Ubegin(_Range), _Uend(_Range), _STD move(_Result)); _Seek_wrapped(_First, _STD move(_UResult.in)); return {_STD move(_First), _STD move(_UResult.out)}; } From 55e79f9cf8dbfdcbc0156bfad0ab78ad10232a88 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Thu, 2 Jul 2020 14:14:32 +0200 Subject: [PATCH 07/10] Prepare for coming machinery --- stl/inc/algorithm | 4 +--- .../tests/P0896R4_ranges_alg_copy/test.cpp | 24 ++++++++++--------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 67da44550df..64146588c38 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -1525,10 +1525,8 @@ namespace ranges { template requires indirectly_copyable, _Out> constexpr copy_result, _Out> operator()(_Rng&& _Range, _Out _Result) const { - auto _First = _RANGES begin(_Range); auto _UResult = _Copy_unchecked(_Ubegin(_Range), _Uend(_Range), _STD move(_Result)); - _Seek_wrapped(_First, _STD move(_UResult.in)); - return {_STD move(_First), _STD move(_UResult.out)}; + return {_Rewrap_iterator(_Range, _STD move(_UResult.in)), _STD move(_UResult.out)}; } // clang-format on diff --git a/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp b/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp index 87439f138af..841c393c19a 100644 --- a/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp @@ -10,38 +10,40 @@ #include using namespace std; -using same_as; -using ranges::copy, ranges::copy_result, ranges::iterator_t; // Validate that copy_result aliases in_out_result STATIC_ASSERT(same_as, ranges::in_out_result>); // Validate dangling story +STATIC_ASSERT(same_as{}, static_cast(nullptr))), + ranges::copy_result>); STATIC_ASSERT( - same_as{}, static_cast(nullptr))), copy_result>); -STATIC_ASSERT(same_as{}, static_cast(nullptr))), copy_result>); + same_as{}, static_cast(nullptr))), ranges::copy_result>); struct instantiator { static constexpr int input[3] = {13, 42, 1729}; - template + template static constexpr void call() { + using ranges::copy, ranges::copy_result, ranges::iterator_t; { // Validate iterator + sentinel overload int output[3] = {-1, -1, -1}; - In wrapped_input{input}; + Read wrapped_input{input}; + auto result = copy(wrapped_input.begin(), wrapped_input.end(), Write{output}); - STATIC_ASSERT(same_as, Write>>); + STATIC_ASSERT(same_as, Write>>); assert(result.in == wrapped_input.end()); - assert(result.out.base() == output + 3); + assert(result.out.peek() == output + 3); assert(ranges::equal(output, input)); } { // Validate range overload int output[3] = {-1, -1, -1}; - In wrapped_input{input}; + Read wrapped_input{input}; + auto result = copy(wrapped_input, Write{output}); - STATIC_ASSERT(same_as, Write>>); + STATIC_ASSERT(same_as, Write>>); assert(result.in == wrapped_input.end()); - assert(result.out.base() == output + 3); + assert(result.out.peek() == output + 3); assert(ranges::equal(output, input)); } } From 2bb2eff66a2e7b0bba688cd7b31f563caccea253 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 3 Jul 2020 07:22:06 +0200 Subject: [PATCH 08/10] Review comments --- stl/inc/algorithm | 6 ++++-- tests/std/tests/P0896R4_ranges_alg_copy/test.cpp | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 64146588c38..eff48c04e57 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -1525,8 +1525,10 @@ namespace ranges { template requires indirectly_copyable, _Out> constexpr copy_result, _Out> operator()(_Rng&& _Range, _Out _Result) const { - auto _UResult = _Copy_unchecked(_Ubegin(_Range), _Uend(_Range), _STD move(_Result)); - return {_Rewrap_iterator(_Range, _STD move(_UResult.in)), _STD move(_UResult.out)}; + auto _First = _RANGES begin(_Range); + auto _UResult = _Copy_unchecked(_Get_unwrapped(_STD move(_First)), _Uend(_Range), _STD move(_Result)); + _Seek_wrapped(_First, _STD move(_UResult.in)); + return {_STD move(_First), _STD move(_UResult.out)}; } // clang-format on diff --git a/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp b/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp index 841c393c19a..6fd9130e399 100644 --- a/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_copy/test.cpp @@ -12,18 +12,18 @@ using namespace std; // Validate that copy_result aliases in_out_result -STATIC_ASSERT(same_as, ranges::in_out_result>); +STATIC_ASSERT(same_as, ranges::in_out_result>); // Validate dangling story STATIC_ASSERT(same_as{}, static_cast(nullptr))), ranges::copy_result>); STATIC_ASSERT( - same_as{}, static_cast(nullptr))), ranges::copy_result>); + same_as{}, static_cast(nullptr))), ranges::copy_result>); struct instantiator { static constexpr int input[3] = {13, 42, 1729}; - template + template > Write> static constexpr void call() { using ranges::copy, ranges::copy_result, ranges::iterator_t; { // Validate iterator + sentinel overload From 93cd60783fce84e938976ad0af081503bb999142 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Sun, 5 Jul 2020 07:15:17 -0700 Subject: [PATCH 09/10] Promote `_Copy_unchecked` from `_Copy_fn` to namespace scope ... so other algorithms can use it directly. --- stl/inc/algorithm | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index eff48c04e57..59dcfea101e 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -1507,6 +1507,18 @@ namespace ranges { using copy_result = in_out_result<_In, _Out>; // VARIABLE ranges::copy + // clang-format off + template _Se, weakly_incrementable _Out> + requires indirectly_copyable<_It, _Out> + _NODISCARD constexpr copy_result<_It, _Out> _Copy_unchecked(_It _First, const _Se _Last, _Out _Result) { + for (; _First != _Last; ++_First, (void) ++_Result) { + *_Result = *_First; + } + + return {_STD move(_First), _STD move(_Result)}; + } + // clang-format on + class _Copy_fn : private _Not_quite_object { public: using _Not_quite_object::_Not_quite_object; @@ -1516,7 +1528,7 @@ namespace ranges { requires indirectly_copyable<_It, _Out> constexpr copy_result<_It, _Out> operator()(_It _First, _Se _Last, _Out _Result) const { _Adl_verify_range(_First, _Last); - auto _UResult = _Copy_unchecked( + auto _UResult = _RANGES _Copy_unchecked( _Get_unwrapped(_STD move(_First)), _Get_unwrapped(_STD move(_Last)), _STD move(_Result)); _Seek_wrapped(_First, _STD move(_UResult.in)); return {_STD move(_First), _STD move(_UResult.out)}; @@ -1526,26 +1538,11 @@ namespace ranges { requires indirectly_copyable, _Out> constexpr copy_result, _Out> operator()(_Rng&& _Range, _Out _Result) const { auto _First = _RANGES begin(_Range); - auto _UResult = _Copy_unchecked(_Get_unwrapped(_STD move(_First)), _Uend(_Range), _STD move(_Result)); + auto _UResult = _RANGES _Copy_unchecked(_Get_unwrapped(_STD move(_First)), _Uend(_Range), _STD move(_Result)); _Seek_wrapped(_First, _STD move(_UResult.in)); return {_STD move(_First), _STD move(_UResult.out)}; } // clang-format on - - private: - template - _NODISCARD static constexpr copy_result<_It, _Out> _Copy_unchecked(_It _First, const _Se _Last, _Out _Result) { - _STL_INTERNAL_STATIC_ASSERT(input_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>); - - for (; _First != _Last; ++_First, (void) ++_Result) { - *_Result = *_First; - } - - return {_STD move(_First), _STD move(_Result)}; - } }; inline constexpr _Copy_fn copy{_Not_quite_object::_Construct_tag{}}; From 992437d45710608e14ea1381734fcc72441f4ae2 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Sun, 5 Jul 2020 19:34:25 +0200 Subject: [PATCH 10/10] Fix typo --- stl/inc/algorithm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 59dcfea101e..383890a7a60 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -1508,7 +1508,7 @@ namespace ranges { // VARIABLE ranges::copy // clang-format off - template _Se, weakly_incrementable _Out> + template _Se, weakly_incrementable _Out> requires indirectly_copyable<_It, _Out> _NODISCARD constexpr copy_result<_It, _Out> _Copy_unchecked(_It _First, const _Se _Last, _Out _Result) { for (; _First != _Last; ++_First, (void) ++_Result) {