From 917c274cc9585d989efbee5533b1ead52c810fa8 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Wed, 10 Aug 2022 13:30:37 -0700 Subject: [PATCH 1/3] Implement `(chunk_view|stride_view)::iterator::operator(\+=|-=)` in O(1) Speculatively implements what will be the proposed resolution of an LWG issue filed per GH-2995 once the new working draft is out with `stride_view` and `chunk_view`. Also avoid checking the precondition when it can't be done in O(1). Drive-by: * inline `ranges::advance(i, n)` since it's simply `i += n` for random-access iterators * check for overflow of `-n` when calling `*this += -n` in `operator-=` Partially addresses #2995 --- stl/inc/ranges | 59 +++++++++++-------- .../tests/P1899R3_views_stride_death/test.cpp | 7 +++ .../tests/P2442R1_views_chunk_death/test.cpp | 21 +++++++ 3 files changed, 64 insertions(+), 23 deletions(-) diff --git a/stl/inc/ranges b/stl/inc/ranges index ea7ea2c5a3..1aef77dec2 100644 --- a/stl/inc/ranges +++ b/stl/inc/ranges @@ -5566,35 +5566,35 @@ namespace ranges { requires random_access_range<_Base> { if (_Off > 0) { #if _ITERATOR_DEBUG_LEVEL != 0 - _STL_VERIFY(_Off == 1 || _Count <= (numeric_limits::max)() / (_Off - 1), + _STL_VERIFY(_Count <= (numeric_limits::max)() / _Off, "cannot advance chunk_view iterator past end (integer overflow)"); - _STL_VERIFY(_RANGES distance(_Current, _End) > _Count * (_Off - 1), - "cannot advance chunk_view iterator past end"); + if constexpr (sized_sentinel_for<_Base_sentinel, _Base_iterator>) { + _STL_VERIFY(_RANGES distance(_Current, _End) > _Count * (_Off - 1), + "cannot advance chunk_view iterator past end"); + } #endif // _ITERATOR_DEBUG_LEVEL != 0 - _Missing = _RANGES advance(_Current, _Count * _Off, _End); + + // Per to-be-filed LWG issue (See GH-2995) + if constexpr (sized_sentinel_for<_Base_sentinel, _Base_iterator>) { + _Missing = _RANGES advance(_Current, _Count * _Off, _End); + } else { + _Current += _Count * (_Off - 1); + _Missing = _RANGES advance(_Current, _Count, _End); + } } else if (_Off < 0) { - _RANGES advance(_Current, _Count * _Off + _Missing); + _Current += _Count * _Off + _Missing; _Missing = 0; } return *this; } - constexpr _Iterator& operator-=(const difference_type _Raw_off) /* not strengthened, see _RANGES advance */ + constexpr _Iterator& operator-=(const difference_type _Off) /* not strengthened, see _RANGES advance */ requires random_access_range<_Base> { - const difference_type _Off = -_Raw_off; - if (_Off > 0) { #if _ITERATOR_DEBUG_LEVEL != 0 - _STL_VERIFY(_Off == 1 || _Count <= (numeric_limits::max)() / (_Off - 1), - "cannot advance chunk_view iterator past end (integer overflow)"); - _STL_VERIFY(_RANGES distance(_Current, _End) > _Count * (_Off - 1), - "cannot advance chunk_view iterator past end"); + _STL_VERIFY(_Off != (numeric_limits::min)(), + "cannot advance chunk_view iterator past end (integer overflow)"); #endif // _ITERATOR_DEBUG_LEVEL != 0 - _Missing = _RANGES advance(_Current, _Count * _Off, _End); - } else if (_Off < 0) { - _RANGES advance(_Current, _Count * _Off + _Missing); - _Missing = 0; - } - return *this; + return *this += -_Off; } _NODISCARD constexpr value_type operator[](const difference_type _Off) const @@ -6470,20 +6470,33 @@ namespace ranges { constexpr _Iterator& operator+=(const difference_type _Off) requires random_access_range<_Base> { if (_Off > 0) { #if _ITERATOR_DEBUG_LEVEL != 0 - _STL_VERIFY(_Off == 1 || _Stride <= (numeric_limits::max)() / (_Off - 1), + _STL_VERIFY(_Stride <= (numeric_limits::max)() / _Off, "cannot advance stride_view iterator past end (integer overflow)"); - _STL_VERIFY(_RANGES distance(_Current, _End) > _Stride * (_Off - 1), - "cannot advance stride_view iterator past end"); + if constexpr (sized_sentinel_for<_Base_sentinel, _Base_iterator>) { + _STL_VERIFY(_RANGES distance(_Current, _End) > _Stride * (_Off - 1), + "cannot advance stride_view iterator past end"); + } #endif // _ITERATOR_DEBUG_LEVEL != 0 - _Missing = _RANGES advance(_Current, _Stride * _Off, _End); + + // Per to-be-filed LWG issue (See GH-2995) + if constexpr (sized_sentinel_for<_Base_sentinel, _Base_iterator>) { + _Missing = _RANGES advance(_Current, _Stride * _Off, _End); + } else { + _Current += _Stride * static_cast(_Off - 1); + _Missing = _RANGES advance(_Current, _Stride, _End); + } } else if (_Off < 0) { - _RANGES advance(_Current, _Stride * _Off + _Missing); + _Current += _Stride * _Off + _Missing; _Missing = 0; } return *this; } constexpr _Iterator& operator-=(const difference_type _Off) requires random_access_range<_Base> { +#if _ITERATOR_DEBUG_LEVEL != 0 + _STL_VERIFY(_Off != (numeric_limits::min)(), + "cannot advance stride_view iterator past end (integer overflow)"); +#endif // _ITERATOR_DEBUG_LEVEL != 0 return *this += -_Off; } diff --git a/tests/std/tests/P1899R3_views_stride_death/test.cpp b/tests/std/tests/P1899R3_views_stride_death/test.cpp index 32641b5e06..ffe1afd65e 100644 --- a/tests/std/tests/P1899R3_views_stride_death/test.cpp +++ b/tests/std/tests/P1899R3_views_stride_death/test.cpp @@ -47,6 +47,12 @@ void test_iterator_advance_past_end_with_integer_overflow() { it += (numeric_limits::max)() / 2; // cannot advance stride_view iterator past end (integer overflow) } +void test_iterator_advance_negative_min() { + auto v = ranges::stride_view(some_ints, 3); + auto it = v.begin(); + it -= (numeric_limits::min)(); // cannot advance stride_view iterator past end (integer overflow) +} + int main(int argc, char* argv[]) { std_testing::death_test_executive exec; @@ -58,6 +64,7 @@ int main(int argc, char* argv[]) { test_iterator_postincrement_past_end, test_iterator_advance_past_end, test_iterator_advance_past_end_with_integer_overflow, + test_iterator_advance_negative_min, }); #else // ^^^ test everything / test only _CONTAINER_DEBUG_LEVEL case vvv exec.add_death_tests({ diff --git a/tests/std/tests/P2442R1_views_chunk_death/test.cpp b/tests/std/tests/P2442R1_views_chunk_death/test.cpp index d6f3a4a148..a62cfdfac6 100644 --- a/tests/std/tests/P2442R1_views_chunk_death/test.cpp +++ b/tests/std/tests/P2442R1_views_chunk_death/test.cpp @@ -75,6 +75,24 @@ void test_inner_iterator_dereference_at_end() { (void) *it; // cannot dereference chunk_view end iterator } +void test_fwd_iterator_advance_past_end() { + auto v = chunk_view{span{some_ints}, 2}; + auto it = v.begin(); + it += 5; // cannot advance chunk_view iterator past end +} + +void test_fwd_iterator_advance_past_end_with_integer_overflow() { + auto v = chunk_view{span{some_ints}, 2}; + auto it = v.begin(); + it += (numeric_limits::max)() / 2; // cannot advance chunk_view iterator past end (integer overflow) +} + +void test_fwd_iterator_advance_negative_min() { + auto v = chunk_view{span{some_ints}, 2}; + auto it = v.begin(); + it -= (numeric_limits::min)(); // cannot advance chunk_view iterator past end (integer overflow) +} + int main(int argc, char* argv[]) { std_testing::death_test_executive exec; @@ -88,6 +106,9 @@ int main(int argc, char* argv[]) { test_inner_iterator_postincrement_past_end, test_outer_iterator_dereference_at_end, test_inner_iterator_dereference_at_end, + test_fwd_iterator_advance_past_end, + test_fwd_iterator_advance_past_end_with_integer_overflow, + test_fwd_iterator_advance_negative_min, }); #else // ^^^ test everything / test only _CONTAINER_DEBUG_LEVEL cases vvv exec.add_death_tests({ From 61cc007d51cc63eaefcefc24ebf646d012870c7d Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Wed, 10 Aug 2022 19:06:38 -0700 Subject: [PATCH 2/3] STL's review comment --- stl/inc/ranges | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/stl/inc/ranges b/stl/inc/ranges index 1aef77dec2..db9bee2963 100644 --- a/stl/inc/ranges +++ b/stl/inc/ranges @@ -5578,11 +5578,11 @@ namespace ranges { if constexpr (sized_sentinel_for<_Base_sentinel, _Base_iterator>) { _Missing = _RANGES advance(_Current, _Count * _Off, _End); } else { - _Current += _Count * (_Off - 1); + _Current += static_cast(_Count * (_Off - 1)); _Missing = _RANGES advance(_Current, _Count, _End); } } else if (_Off < 0) { - _Current += _Count * _Off + _Missing; + _Current += static_cast(_Count * _Off + _Missing); _Missing = 0; } return *this; @@ -6482,11 +6482,11 @@ namespace ranges { if constexpr (sized_sentinel_for<_Base_sentinel, _Base_iterator>) { _Missing = _RANGES advance(_Current, _Stride * _Off, _End); } else { - _Current += _Stride * static_cast(_Off - 1); + _Current += static_cast(_Stride * (_Off - 1)); _Missing = _RANGES advance(_Current, _Stride, _End); } } else if (_Off < 0) { - _Current += _Stride * _Off + _Missing; + _Current += static_cast(_Stride * _Off + _Missing); _Missing = 0; } return *this; From 79383d0795748f59b5571b19afe1c8a3079fc10c Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 10 Aug 2022 22:44:33 -0700 Subject: [PATCH 3/3] With random_access_range, subtract instead of distance(). Co-authored-by: S. B. Tam --- stl/inc/ranges | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/ranges b/stl/inc/ranges index db9bee2963..27cac5aa0a 100644 --- a/stl/inc/ranges +++ b/stl/inc/ranges @@ -5569,7 +5569,7 @@ namespace ranges { _STL_VERIFY(_Count <= (numeric_limits::max)() / _Off, "cannot advance chunk_view iterator past end (integer overflow)"); if constexpr (sized_sentinel_for<_Base_sentinel, _Base_iterator>) { - _STL_VERIFY(_RANGES distance(_Current, _End) > _Count * (_Off - 1), + _STL_VERIFY(_End - _Current > _Count * (_Off - 1), // "cannot advance chunk_view iterator past end"); } #endif // _ITERATOR_DEBUG_LEVEL != 0 @@ -6473,7 +6473,7 @@ namespace ranges { _STL_VERIFY(_Stride <= (numeric_limits::max)() / _Off, "cannot advance stride_view iterator past end (integer overflow)"); if constexpr (sized_sentinel_for<_Base_sentinel, _Base_iterator>) { - _STL_VERIFY(_RANGES distance(_Current, _End) > _Stride * (_Off - 1), + _STL_VERIFY(_End - _Current > _Stride * (_Off - 1), // "cannot advance stride_view iterator past end"); } #endif // _ITERATOR_DEBUG_LEVEL != 0