From 2e4c239031242191495d8c7e2051d8a6b6521ec4 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Sun, 12 Sep 2021 12:54:44 +0700 Subject: [PATCH 1/7] delete `friend constexpr void iter_swap` for `class transform_view` `iterator` --- stl/inc/ranges | 9 --------- 1 file changed, 9 deletions(-) diff --git a/stl/inc/ranges b/stl/inc/ranges index 3b90541501f..31f383e2033 100644 --- a/stl/inc/ranges +++ b/stl/inc/ranges @@ -1988,15 +1988,6 @@ namespace ranges { return *_It; } } - - friend constexpr void iter_swap(const _Iterator& _Left, const _Iterator& _Right) _NOEXCEPT_IDL0(noexcept( - _RANGES iter_swap(_Left._Current, _Right._Current))) requires indirectly_swappable> { -#if _ITERATOR_DEBUG_LEVEL != 0 - _Left._Check_dereference(); - _Right._Check_dereference(); -#endif // _ITERATOR_DEBUG_LEVEL != 0 - _RANGES iter_swap(_Left._Current, _Right._Current); - } }; template From 109398cf62315094fa57f1e3e6797aa02dd5fc07 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Sun, 12 Sep 2021 13:24:01 +0700 Subject: [PATCH 2/7] fix tests --- .../tests/P0896R4_views_transform/test.cpp | 11 --------- .../P0896R4_views_transform_death/test.cpp | 23 ------------------- 2 files changed, 34 deletions(-) diff --git a/tests/std/tests/P0896R4_views_transform/test.cpp b/tests/std/tests/P0896R4_views_transform/test.cpp index 7f1d108a9c9..0bc79fbe4e1 100644 --- a/tests/std/tests/P0896R4_views_transform/test.cpp +++ b/tests/std/tests/P0896R4_views_transform/test.cpp @@ -508,17 +508,6 @@ struct iterator_instantiator { assert(ranges::iter_move(i0) == add8(mutable_ints[0])); // NB: moving from int leaves it unchanged STATIC_ASSERT(NOEXCEPT_IDL0(ranges::iter_move(i0))); - - if constexpr (forward_iterator) { - auto i1 = ranges::next(i0); - ranges::iter_swap(i0, i1); - assert(mutable_ints[0] == 1); - assert(mutable_ints[1] == 0); - ranges::iter_swap(i1, i0); - assert(mutable_ints[0] == 0); - assert(mutable_ints[1] == 1); - STATIC_ASSERT(NOEXCEPT_IDL0(ranges::iter_swap(i0, i1))); - } } { // Validate increments diff --git a/tests/std/tests/P0896R4_views_transform_death/test.cpp b/tests/std/tests/P0896R4_views_transform_death/test.cpp index 29ca67cda9d..c869f5f0cc6 100644 --- a/tests/std/tests/P0896R4_views_transform_death/test.cpp +++ b/tests/std/tests/P0896R4_views_transform_death/test.cpp @@ -287,26 +287,6 @@ void test_iter_move_value_initialized_iterator() { (void) ranges::iter_move(i); // cannot dereference value-initialized transform_view iterator } -void test_iter_swap_value_initialized_iterators() { - ranges::iterator_t i0{}; - ranges::iterator_t i1{}; - (void) ranges::iter_swap(i0, i1); // cannot dereference value-initialized transform_view iterator -} - -void test_iter_swap_value_initialized_iterator_left() { - ranges::iterator_t i0{}; - TV r{some_ints, lambda}; - ranges::iterator_t i1 = r.begin(); - (void) ranges::iter_swap(i0, i1); // cannot dereference value-initialized transform_view iterator -} - -void test_iter_swap_value_initialized_iterator_right() { - TV r{some_ints, lambda}; - ranges::iterator_t i0 = r.begin(); - ranges::iterator_t i1{}; - (void) ranges::iter_swap(i0, i1); // cannot dereference value-initialized transform_view iterator -} - void test_sentinel_compare_value_initialized() { auto r = ranges::subrange{counted_iterator{some_ints, ranges::distance(some_ints)}, default_sentinel} | views::transform(lambda); @@ -382,9 +362,6 @@ int main(int argc, char* argv[]) { test_operator_minus_incompatible_different, test_operator_minus_incompatible_value_initialized, test_iter_move_value_initialized_iterator, - test_iter_swap_value_initialized_iterators, - test_iter_swap_value_initialized_iterator_left, - test_iter_swap_value_initialized_iterator_right, test_sentinel_compare_value_initialized, test_sentinel_difference_value_initialized, test_flipped_sentinel_difference_value_initialized, From ef6b748a83c6e321a2f0b30cb9fdd43a4b780700 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Sun, 12 Sep 2021 18:28:01 +0700 Subject: [PATCH 3/7] add test Co-authored-by: Michael Schellenberger Costa --- tests/std/include/range_algorithm_support.hpp | 5 +++++ tests/std/tests/P0896R4_views_transform/test.cpp | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/tests/std/include/range_algorithm_support.hpp b/tests/std/include/range_algorithm_support.hpp index 2ebee42e336..9a8594c753e 100644 --- a/tests/std/include/range_algorithm_support.hpp +++ b/tests/std/include/range_algorithm_support.hpp @@ -1449,3 +1449,8 @@ template concept CanBool = requires(R&& r) { std::forward(r) ? true : false; }; + +template +concept CanIterSwap = requires(I&& i1, I&& i2) { + ranges::iter_swap(std::forward(i1), std::forward(i2)); +}; diff --git a/tests/std/tests/P0896R4_views_transform/test.cpp b/tests/std/tests/P0896R4_views_transform/test.cpp index 0bc79fbe4e1..55c36dcb2b0 100644 --- a/tests/std/tests/P0896R4_views_transform/test.cpp +++ b/tests/std/tests/P0896R4_views_transform/test.cpp @@ -508,6 +508,11 @@ struct iterator_instantiator { assert(ranges::iter_move(i0) == add8(mutable_ints[0])); // NB: moving from int leaves it unchanged STATIC_ASSERT(NOEXCEPT_IDL0(ranges::iter_move(i0))); + + if constexpr (forward_iterator) { + // LWG-3520 forbids iter_swap for transform_view::iterator + STATIC_ASSERT(!CanIterSwap); + } } { // Validate increments From a456b6c5ba7792281d825eaaf4e638acb91db298 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Sun, 12 Sep 2021 19:20:38 +0700 Subject: [PATCH 4/7] check unconditionally --- tests/std/tests/P0896R4_views_transform/test.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/std/tests/P0896R4_views_transform/test.cpp b/tests/std/tests/P0896R4_views_transform/test.cpp index 55c36dcb2b0..33408528426 100644 --- a/tests/std/tests/P0896R4_views_transform/test.cpp +++ b/tests/std/tests/P0896R4_views_transform/test.cpp @@ -509,10 +509,8 @@ struct iterator_instantiator { assert(ranges::iter_move(i0) == add8(mutable_ints[0])); // NB: moving from int leaves it unchanged STATIC_ASSERT(NOEXCEPT_IDL0(ranges::iter_move(i0))); - if constexpr (forward_iterator) { - // LWG-3520 forbids iter_swap for transform_view::iterator - STATIC_ASSERT(!CanIterSwap); - } + // LWG-3520 forbids iter_swap for transform_view::iterator + STATIC_ASSERT(!CanIterSwap); } { // Validate increments From d9c637df6c1939814d280f7fedf8566c9f90467c Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Sun, 12 Sep 2021 21:06:32 +0700 Subject: [PATCH 5/7] `iter_swap` on `transform_view::iterator` is still possible Co-authored-by: Adam Bucior <35536269+AdamBucior@users.noreply.github.com> --- tests/std/tests/P0896R4_views_transform/test.cpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/std/tests/P0896R4_views_transform/test.cpp b/tests/std/tests/P0896R4_views_transform/test.cpp index 33408528426..b5a4c74715a 100644 --- a/tests/std/tests/P0896R4_views_transform/test.cpp +++ b/tests/std/tests/P0896R4_views_transform/test.cpp @@ -509,7 +509,6 @@ struct iterator_instantiator { assert(ranges::iter_move(i0) == add8(mutable_ints[0])); // NB: moving from int leaves it unchanged STATIC_ASSERT(NOEXCEPT_IDL0(ranges::iter_move(i0))); - // LWG-3520 forbids iter_swap for transform_view::iterator STATIC_ASSERT(!CanIterSwap); } @@ -776,4 +775,19 @@ int main() { (void) views::transform(Fn{})(span{}); } + + { // Validate that iter_swap works when result of transformation is a lvalue reference + char out[] = "hello"; + auto v = ranges::transform_view{out, [](char& i) -> char& { return i; }}; + auto i1 = v.begin(); + auto i2 = v.begin() + 1; + + assert(*i1 == 'h'); + assert(*i2 == 'e'); + + ranges::iter_swap(i1, i2); + + assert(*i1 == 'e'); + assert(*i2 == 'h'); + } } From fd1c32ad349afa5c3f73190ba95a131f4c90fe90 Mon Sep 17 00:00:00 2001 From: Igor Zhukov Date: Sun, 12 Sep 2021 21:19:25 +0700 Subject: [PATCH 6/7] better variable names --- tests/std/tests/P0896R4_views_transform/test.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/std/tests/P0896R4_views_transform/test.cpp b/tests/std/tests/P0896R4_views_transform/test.cpp index b5a4c74715a..607e0b310b3 100644 --- a/tests/std/tests/P0896R4_views_transform/test.cpp +++ b/tests/std/tests/P0896R4_views_transform/test.cpp @@ -777,10 +777,10 @@ int main() { } { // Validate that iter_swap works when result of transformation is a lvalue reference - char out[] = "hello"; - auto v = ranges::transform_view{out, [](char& i) -> char& { return i; }}; - auto i1 = v.begin(); - auto i2 = v.begin() + 1; + char base[] = "hello"; + auto v = ranges::transform_view{base, [](char& c) -> char& { return c; }}; + auto i1 = v.begin(); + auto i2 = v.begin() + 1; assert(*i1 == 'h'); assert(*i2 == 'e'); From fe60f6aa4c792c14697bc8e5eafd230bb47ef212 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 16 Sep 2021 19:43:38 -0700 Subject: [PATCH 7/7] Update comment to say "an lvalue" --- tests/std/tests/P0896R4_views_transform/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/P0896R4_views_transform/test.cpp b/tests/std/tests/P0896R4_views_transform/test.cpp index 607e0b310b3..1579df5c1d8 100644 --- a/tests/std/tests/P0896R4_views_transform/test.cpp +++ b/tests/std/tests/P0896R4_views_transform/test.cpp @@ -776,7 +776,7 @@ int main() { (void) views::transform(Fn{})(span{}); } - { // Validate that iter_swap works when result of transformation is a lvalue reference + { // Validate that iter_swap works when result of transformation is an lvalue reference char base[] = "hello"; auto v = ranges::transform_view{base, [](char& c) -> char& { return c; }}; auto i1 = v.begin();