From 56f10d36c1cbabea7da397523a2752bbd34c3704 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Thu, 2 Jul 2020 14:05:10 +0200 Subject: [PATCH 1/7] Modernize ranges::move --- stl/inc/algorithm | 32 +++++-- .../tests/P0896R4_ranges_alg_move/test.cpp | 96 +++++++++---------- 2 files changed, 67 insertions(+), 61 deletions(-) diff --git a/stl/inc/algorithm b/stl/inc/algorithm index 59db8eb1959..6dfabc1c4d6 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -1657,22 +1657,38 @@ namespace ranges { requires indirectly_movable<_It, _Out> constexpr move_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 = _RANGES iter_move(_UFirst); - } + auto _UResult = _Move_unchecked( + _Get_unwrapped(_STD move(_First)), _Get_unwrapped(_STD move(_Last)), _STD move(_Result)); - _Seek_wrapped(_First, _STD move(_UFirst)); - return {_STD move(_First), _STD move(_Result)}; + _Seek_wrapped(_First, _STD move(_UResult.in)); + return {_STD move(_First), _STD move(_UResult.out)}; } template requires indirectly_movable, _Out> constexpr move_result, _Out> operator()(_Rng&& _Range, _Out _Result) const { - return (*this)(_RANGES begin(_Range), _RANGES end(_Range), _STD move(_Result)); + auto _First = _RANGES begin(_Range); + auto _UResult = _Move_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 move_result<_It, _Out> _Move_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_movable<_It, _Out>); + + for (; _First != _Last; ++_First, (void) ++_Result) { + *_Result = _RANGES iter_move(_First); + } + + return {_STD move(_First), _STD move(_Result)}; + } }; inline constexpr _Move_fn move{_Not_quite_object::_Construct_tag{}}; diff --git a/tests/std/tests/P0896R4_ranges_alg_move/test.cpp b/tests/std/tests/P0896R4_ranges_alg_move/test.cpp index 8b1141e7065..cadc2d19e67 100644 --- a/tests/std/tests/P0896R4_ranges_alg_move/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_move/test.cpp @@ -9,6 +9,8 @@ #include +using namespace std; + struct int_wrapper { int val = 10; constexpr int_wrapper() = default; @@ -18,66 +20,54 @@ struct int_wrapper { val = std::exchange(that.val, -1); return *this; } + auto operator<=>(const int_wrapper&) const = default; }; -constexpr void smoke_test() { - using ranges::move, ranges::move_result, ranges::iterator_t; - using std::same_as; +// Validate that move_result aliases in_out_result +STATIC_ASSERT(same_as, ranges::in_out_result>); - // Validate that move_result aliases in_out_result - STATIC_ASSERT(same_as, ranges::in_out_result>); +// Validate dangling story +STATIC_ASSERT(same_as{}, static_cast(nullptr))), + ranges::move_result>); +STATIC_ASSERT( + same_as{}, static_cast(nullptr))), ranges::move_result>); - // Validate dangling story - STATIC_ASSERT( - same_as{}, static_cast(nullptr))), move_result>); - STATIC_ASSERT(same_as{}, static_cast(nullptr))), move_result>); - int const input[] = {13, 53, 12435}; - { - int output[] = {-2, -2, -2}; - auto result = move(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)); - } - { - int output[] = {-2, -2, -2}; - basic_borrowed_range wrapped_input{input}; - auto result = move(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_wrapper input1[3] = {13, 55, 1234}; - int const expected_output[3] = {13, 55, 1234}; - int_wrapper actual_output[3] = {-2, -2, -2}; - basic_borrowed_range wrapped_input{input1}; - auto result = move(wrapped_input.begin(), wrapped_input.end(), basic_borrowed_range{actual_output}.begin()); - assert(result.in == wrapped_input.end()); - assert(result.out == basic_borrowed_range{actual_output}.end()); - for (int i = 0; i < 3; ++i) { - assert(input1[i].val == -1); - assert(actual_output[i].val == expected_output[i]); - } - } -} +struct instantiator { + static constexpr int_wrapper input[3] = {13, 55, 12345}; + static constexpr int_wrapper expected_output[3] = {13, 55, 12345}; + static constexpr int_wrapper expected_input[3] = {-1, -1, -1}; -int main() { - STATIC_ASSERT((smoke_test(), true)); - smoke_test(); -} + template > Write> + static constexpr void call() { + using ranges::move, ranges::move_result, ranges::iterator_t; + { + int_wrapper input[3] = {13, 55, 12345}; + int_wrapper output[3] = {-2, -2, -2}; + Read wrapped_input{input}; -struct instantiator { - template - static void call(In&& in = {}, Out out = {}) { - (void) ranges::move(in, std::move(out)); - (void) ranges::move(ranges::begin(in), ranges::end(in), std::move(out)); + auto result = move(wrapped_input, Write{output}); + STATIC_ASSERT(same_as, Write>>); + assert(result.in == wrapped_input.end()); + assert(result.out.peek() == output + 3); + assert(ranges::equal(output, expected_output)); + assert(ranges::equal(input, expected_input)); + } + { + int_wrapper input[3] = {13, 55, 12345}; + int_wrapper output[3] = {-2, -2, -2}; + Read wrapped_input{input}; + + auto result = move(wrapped_input.begin(), wrapped_input.end(), Write{output}); + assert(result.in == wrapped_input.end()); + assert(result.out.peek() == output + 3); + assert(ranges::equal(output, expected_output)); + assert(ranges::equal(input, expected_input)); + } } }; -template void test_in_write(); +int main() { + STATIC_ASSERT((test_in_write(), true)); + test_in_write(); +} From ecd1236f2da50b8b645cdac9021150a606ab53d1 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Sat, 4 Jul 2020 07:31:45 +0200 Subject: [PATCH 2/7] Fix constness --- tests/std/tests/P0896R4_ranges_alg_move/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/P0896R4_ranges_alg_move/test.cpp b/tests/std/tests/P0896R4_ranges_alg_move/test.cpp index cadc2d19e67..68886f2f478 100644 --- a/tests/std/tests/P0896R4_ranges_alg_move/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_move/test.cpp @@ -68,6 +68,6 @@ struct instantiator { }; int main() { - STATIC_ASSERT((test_in_write(), true)); - test_in_write(); + STATIC_ASSERT((test_in_write(), true)); + test_in_write(); } From cac86a9ffa8efce4f58bb5303fc36f1784ecffd3 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Sun, 5 Jul 2020 13:27:00 +0200 Subject: [PATCH 3/7] Make int_wrapper default constructible/assignable --- tests/std/tests/P0896R4_ranges_alg_move/test.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/std/tests/P0896R4_ranges_alg_move/test.cpp b/tests/std/tests/P0896R4_ranges_alg_move/test.cpp index 68886f2f478..d66b04b16a6 100644 --- a/tests/std/tests/P0896R4_ranges_alg_move/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_move/test.cpp @@ -15,6 +15,8 @@ struct int_wrapper { int val = 10; constexpr int_wrapper() = default; constexpr int_wrapper(int x) : val{x} {} + int_wrapper(const int_wrapper&) = default; + int_wrapper& operator=(const int_wrapper&) = default; constexpr int_wrapper(int_wrapper&& that) : val{std::exchange(that.val, -1)} {} constexpr int_wrapper& operator=(int_wrapper&& that) { val = std::exchange(that.val, -1); @@ -34,7 +36,6 @@ STATIC_ASSERT( struct instantiator { - static constexpr int_wrapper input[3] = {13, 55, 12345}; static constexpr int_wrapper expected_output[3] = {13, 55, 12345}; static constexpr int_wrapper expected_input[3] = {-1, -1, -1}; From 5ecea964ec381a7c2e941a9f458a0220727f79fd Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Mon, 6 Jul 2020 17:45:59 -0700 Subject: [PATCH 4/7] test::proxy_reference fix: `test::proxy_reference` and an arbitrary type `T` must model `common_reference_with` when `Elem&` and `T` do. Drive-by: * Change `proxy_reference`'s conversion operator to `remove_cv_t` into a conversion to `Elem&`, which is both simpler and more generic. * Generalize `proxy_reference`'s assignment from `remove_cv_t const&` into a perfect-forwarding assignment operator. * Extract `ranges::equal` out of `instantiator::call` into a separate function to keep `/analyze` from blowing the compiler's available heap space. --- tests/std/include/range_algorithm_support.hpp | 39 +++++++++++++++++-- .../tests/P0896R4_ranges_alg_move/test.cpp | 13 ++++--- 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/tests/std/include/range_algorithm_support.hpp b/tests/std/include/range_algorithm_support.hpp index b02043749a9..18c14ef32a9 100644 --- a/tests/std/include/range_algorithm_support.hpp +++ b/tests/std/include/range_algorithm_support.hpp @@ -169,14 +169,16 @@ namespace test { } // clang-format off - constexpr operator Value() const requires derived_from && copy_constructible { + constexpr operator Element&() const requires derived_from { return ref_; } - // clang-format on - constexpr void operator=(Value const& val) const requires assignable_from { - ref_ = val; + template + requires (!std::same_as, proxy_reference> && assignable_from) + constexpr void operator=(T&& val) const { + ref_ = std::forward(val); } + // clang-format on template constexpr boolish operator==(proxy_reference that) const requires CanEq { @@ -247,6 +249,35 @@ namespace test { } }; + template + struct common_reference { + Ref ref_; + + common_reference(Ref ref) : ref_{static_cast(ref)} {} + + // clang-format off + template + requires convertible_to + common_reference(proxy_reference pref) : ref_{pref.peek()} {} + // clang-format on + }; +} // namespace test + +// clang-format off +template class TQuals, template class UQuals> + requires std::common_reference_with> +struct std::basic_common_reference<::test::proxy_reference, U, TQuals, UQuals> { + using type = common_reference_t>; +}; + +template class TQuals, template class UQuals> + requires std::common_reference_with, Elem&> +struct std::basic_common_reference, TQuals, UQuals> { + using type = common_reference_t, Elem&>; +}; +// clang-format on + +namespace test { // clang-format off template {}, static_cast STATIC_ASSERT( same_as{}, static_cast(nullptr))), ranges::move_result>); - struct instantiator { static constexpr int_wrapper expected_output[3] = {13, 55, 12345}; static constexpr int_wrapper expected_input[3] = {-1, -1, -1}; + static constexpr void eq(int_wrapper const (&output)[3], int_wrapper const (&input)[3]) { + // Extracted into a separate function to keep /analyze from blowing the compiler heap + assert(ranges::equal(output, expected_output)); + assert(ranges::equal(input, expected_input)); + } + template > Write> static constexpr void call() { using ranges::move, ranges::move_result, ranges::iterator_t; @@ -51,8 +56,7 @@ struct instantiator { STATIC_ASSERT(same_as, Write>>); assert(result.in == wrapped_input.end()); assert(result.out.peek() == output + 3); - assert(ranges::equal(output, expected_output)); - assert(ranges::equal(input, expected_input)); + eq(output, input); } { int_wrapper input[3] = {13, 55, 12345}; @@ -62,8 +66,7 @@ struct instantiator { auto result = move(wrapped_input.begin(), wrapped_input.end(), Write{output}); assert(result.in == wrapped_input.end()); assert(result.out.peek() == output + 3); - assert(ranges::equal(output, expected_output)); - assert(ranges::equal(input, expected_input)); + eq(output, input); } } }; From 84bb9a3a50ca726b8e8cfd45f78982cda7225aef Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Tue, 7 Jul 2020 16:26:01 +0200 Subject: [PATCH 5/7] Language please Co-authored-by: Stephan T. Lavavej --- tests/std/tests/P0896R4_ranges_alg_move/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/P0896R4_ranges_alg_move/test.cpp b/tests/std/tests/P0896R4_ranges_alg_move/test.cpp index 33bd01efa83..82b566b8d71 100644 --- a/tests/std/tests/P0896R4_ranges_alg_move/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_move/test.cpp @@ -39,7 +39,7 @@ struct instantiator { static constexpr int_wrapper expected_input[3] = {-1, -1, -1}; static constexpr void eq(int_wrapper const (&output)[3], int_wrapper const (&input)[3]) { - // Extracted into a separate function to keep /analyze from blowing the compiler heap + // Extracted into a separate function to keep /analyze from exhausting the compiler heap assert(ranges::equal(output, expected_output)); assert(ranges::equal(input, expected_input)); } From 0281bdba137895ed209485d2f0407f1b87fd7049 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Tue, 7 Jul 2020 09:49:49 -0700 Subject: [PATCH 6/7] Rename ref to r in `range_algorithm_support.hpp> --- tests/std/include/range_algorithm_support.hpp | 58 +++++++++---------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/tests/std/include/range_algorithm_support.hpp b/tests/std/include/range_algorithm_support.hpp index 18c14ef32a9..8c4c3e6dd56 100644 --- a/tests/std/include/range_algorithm_support.hpp +++ b/tests/std/include/range_algorithm_support.hpp @@ -159,7 +159,7 @@ namespace test { using Value = std::remove_cv_t; public: - constexpr explicit proxy_reference(Element& ref) : ref_{ref} {} + constexpr explicit proxy_reference(Element& r) : ref_{r} {} proxy_reference(proxy_reference const&) = default; constexpr proxy_reference const& operator=(proxy_reference const& that) const @@ -206,41 +206,41 @@ namespace test { } // clang-format off - friend constexpr boolish operator==(proxy_reference ref, Value const& val) requires CanEq { - return {ref.ref_ == val}; + friend constexpr boolish operator==(proxy_reference r, Value const& val) requires CanEq { + return {r.ref_ == val}; } - friend constexpr boolish operator==(Value const& val, proxy_reference ref) requires CanEq { - return {ref.ref_ == val}; + friend constexpr boolish operator==(Value const& val, proxy_reference r) requires CanEq { + return {r.ref_ == val}; } - friend constexpr boolish operator!=(proxy_reference ref, Value const& val) requires CanNEq { - return {ref.ref_ != val}; + friend constexpr boolish operator!=(proxy_reference r, Value const& val) requires CanNEq { + return {r.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 r) requires CanNEq { + return {r.ref_ != val}; } - friend constexpr boolish operator<(Value const& val, proxy_reference ref) requires CanLt { - return {val < ref.ref_}; + friend constexpr boolish operator<(Value const& val, proxy_reference r) requires CanLt { + return {val < r.ref_}; } - friend constexpr boolish operator<(proxy_reference ref, Value const& val) requires CanLt { - return {ref.ref_ < val}; + friend constexpr boolish operator<(proxy_reference r, Value const& val) requires CanLt { + return {r.ref_ < val}; } - friend constexpr boolish operator>(Value const& val, proxy_reference ref) requires CanGt { - return {val > ref.ref_}; + friend constexpr boolish operator>(Value const& val, proxy_reference r) requires CanGt { + return {val > r.ref_}; } - friend constexpr boolish operator>(proxy_reference ref, Value const& val) requires CanGt { - return {ref.ref_ > val}; + friend constexpr boolish operator>(proxy_reference r, Value const& val) requires CanGt { + return {r.ref_ > val}; } - friend constexpr boolish operator<=(Value const& val, proxy_reference ref) requires CanLtE { - return {val <= ref.ref_}; + friend constexpr boolish operator<=(Value const& val, proxy_reference r) requires CanLtE { + return {val <= r.ref_}; } - friend constexpr boolish operator<=(proxy_reference ref, Value const& val) requires CanLtE { - return {ref.ref_ <= val}; + friend constexpr boolish operator<=(proxy_reference r, Value const& val) requires CanLtE { + return {r.ref_ <= val}; } - friend constexpr boolish operator>=(Value const& val, proxy_reference ref) requires CanGtE { - return {val >= ref.ref_}; + friend constexpr boolish operator>=(Value const& val, proxy_reference r) requires CanGtE { + return {val >= r.ref_}; } - friend constexpr boolish operator>=(proxy_reference ref, Value const& val) requires CanGtE { - return {ref.ref_ >= val}; + friend constexpr boolish operator>=(proxy_reference r, Value const& val) requires CanGtE { + return {r.ref_ >= val}; } // clang-format on @@ -253,7 +253,7 @@ namespace test { struct common_reference { Ref ref_; - common_reference(Ref ref) : ref_{static_cast(ref)} {} + common_reference(Ref r) : ref_{static_cast(r)} {} // clang-format off template @@ -995,11 +995,11 @@ struct get_nth_fn { { return get(std::forward(t)); } template - [[nodiscard]] constexpr decltype(auto) operator()(test::proxy_reference ref) const noexcept + [[nodiscard]] constexpr decltype(auto) operator()(test::proxy_reference r) const noexcept requires requires { - (*this)(ref.peek()); + (*this)(r.peek()); } - { return (*this)(ref.peek()); } + { return (*this)(r.peek()); } }; inline constexpr get_nth_fn<0> get_first; inline constexpr get_nth_fn<1> get_second; From c9fe09838a3a60f00115b986db6087e8b45fe5a6 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Tue, 7 Jul 2020 13:45:30 -0700 Subject: [PATCH 7/7] Derp; int_wrapper is move-only. --- tests/std/tests/P0896R4_ranges_alg_move/test.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/std/tests/P0896R4_ranges_alg_move/test.cpp b/tests/std/tests/P0896R4_ranges_alg_move/test.cpp index 82b566b8d71..c97f016e4bc 100644 --- a/tests/std/tests/P0896R4_ranges_alg_move/test.cpp +++ b/tests/std/tests/P0896R4_ranges_alg_move/test.cpp @@ -15,11 +15,9 @@ struct int_wrapper { int val = 10; constexpr int_wrapper() = default; constexpr int_wrapper(int x) : val{x} {} - int_wrapper(const int_wrapper&) = default; - int_wrapper& operator=(const int_wrapper&) = default; - constexpr int_wrapper(int_wrapper&& that) : val{std::exchange(that.val, -1)} {} + constexpr int_wrapper(int_wrapper&& that) : val{exchange(that.val, -1)} {} constexpr int_wrapper& operator=(int_wrapper&& that) { - val = std::exchange(that.val, -1); + val = exchange(that.val, -1); return *this; } auto operator<=>(const int_wrapper&) const = default;