From 7cce0a01b2c8bdf1b7dbb61bb1a6d2faffd6a85c Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Tue, 21 Jul 2020 10:43:00 -0700 Subject: [PATCH 1/2] Fix thinko in ranges::iter_swap ...by transposing arguments to `_Iter_exchange_move`. Fixes #1067 --- stl/inc/xutility | 12 ++++----- .../test.cpp | 25 +++++++++++++++---- 2 files changed, 26 insertions(+), 11 deletions(-) diff --git a/stl/inc/xutility b/stl/inc/xutility index f80ed07fa86..0c7686610e3 100644 --- a/stl/inc/xutility +++ b/stl/inc/xutility @@ -1028,10 +1028,10 @@ namespace ranges { // clang-format on template - _NODISCARD constexpr iter_value_t> _Iter_exchange_move( - _Xty&& _XVal, _Yty&& _YVal) noexcept(noexcept(iter_value_t>(iter_move(_XVal)))) { - iter_value_t> _Tmp(iter_move(_XVal)); - *_XVal = iter_move(_YVal); + _NODISCARD constexpr iter_value_t> _Iter_exchange_move(_Xty&& _XVal, + _Yty&& _YVal) noexcept(noexcept(iter_value_t>(_RANGES iter_move(_XVal)))) { + iter_value_t> _Tmp(_RANGES iter_move(_XVal)); + *_XVal = _RANGES iter_move(_YVal); return _Tmp; } @@ -1047,7 +1047,7 @@ namespace ranges { return {_St::_Swap, noexcept(_RANGES swap(*_STD declval<_Ty1>(), *_STD declval<_Ty2>()))}; } else if constexpr (_Symmetric_indirectly_movable_storable<_Ty1, _Ty2>) { constexpr auto _Nothrow = noexcept( - *_STD declval<_Ty1>() = _Iter_exchange_move(_STD declval<_Ty1>(), _STD declval<_Ty2>())); + *_STD declval<_Ty1>() = _Iter_exchange_move(_STD declval<_Ty2>(), _STD declval<_Ty1>())); return {_St::_Exchange, _Nothrow}; } else { return {_St::_None}; @@ -1068,7 +1068,7 @@ namespace ranges { _RANGES swap(*static_cast<_Ty1&&>(_Val1), *static_cast<_Ty2&&>(_Val2)); } else if constexpr (_Choice<_Ty1, _Ty2>._Strategy == _St::_Exchange) { *static_cast<_Ty1&&>(_Val1) = - _Iter_exchange_move(static_cast<_Ty1&&>(_Val1), static_cast<_Ty2&&>(_Val2)); + _Iter_exchange_move(static_cast<_Ty2&&>(_Val2), static_cast<_Ty1&&>(_Val1)); } else { static_assert(_Always_false<_Ty1>, "should be unreachable"); } diff --git a/tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp b/tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp index bdb19a25b0a..84a6db14347 100644 --- a/tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp +++ b/tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp @@ -1267,11 +1267,26 @@ namespace iterator_cust_swap_test { // clang-format on constexpr bool test() { - int i0 = 42, i1 = 13; - S s0{i0}, s1{i1}; - ranges::iter_swap(s0, s1); - assert(i0 == 13); - assert(i1 == 42); + { + int i0 = 42, i1 = 13; + S s0{i0}, s1{i1}; + ranges::iter_swap(s0, s1); + assert(i0 == 13); + assert(i1 == 42); + } + +#if !defined(__clang__) && !defined(__EDG__) // TRANSITION, VSO-938163 + if (!std::is_constant_evaluated()) +#endif // TRANSITION, VSO-938163 + { + // Validate iter_swap bullet 3 to defend against regression of GH-1067 "ranges::iter_swap is broken" + int i = 42; + long l = 13; + ranges::iter_swap(&i, &l); + assert(i == 13); + assert(l == 42); + } + return true; } STATIC_ASSERT(test()); From 0332191c652ca9f504f55b4e9132b46f2749dff7 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Tue, 21 Jul 2020 21:48:46 -0700 Subject: [PATCH 2/2] Stephan's review comments --- tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp b/tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp index 84a6db14347..f571db14fd8 100644 --- a/tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp +++ b/tests/std/tests/P0896R4_ranges_iterator_machinery/test.cpp @@ -1268,8 +1268,10 @@ namespace iterator_cust_swap_test { constexpr bool test() { { - int i0 = 42, i1 = 13; - S s0{i0}, s1{i1}; + int i0 = 42; + int i1 = 13; + S s0{i0}; + S s1{i1}; ranges::iter_swap(s0, s1); assert(i0 == 13); assert(i1 == 42);