From 8b14dfc3cc1d7b6b7e4e39190a1e89c3ac4ad098 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 25 Sep 2023 07:11:31 +0800 Subject: [PATCH 1/6] Fix iterator arithmetic in `` --- stl/inc/deque | 101 +++++++++++------- .../test.cpp | 83 +++++++++++--- 2 files changed, 128 insertions(+), 56 deletions(-) diff --git a/stl/inc/deque b/stl/inc/deque index 964b8c4f77..9568236039 100644 --- a/stl/inc/deque +++ b/stl/inc/deque @@ -42,9 +42,7 @@ public: : _Mycont(static_cast(_Pdeque)), _Myoff(_Off) {} _NODISCARD reference operator*() const noexcept { - _Size_type _Block = _Mycont->_Getblock(_Myoff); - _Size_type _Off = _Myoff % _Block_size; - return _Mycont->_Map[_Block][_Off]; + return _Mycont->_At(_Myoff); } _NODISCARD pointer operator->() const noexcept { @@ -74,7 +72,7 @@ public: } _Deque_unchecked_const_iterator& operator+=(const difference_type _Off) noexcept { - _Myoff += _Off; + _Myoff = static_cast<_Size_type>(_Myoff + _Off); return *this; } @@ -91,7 +89,7 @@ public: } _Deque_unchecked_const_iterator& operator-=(const difference_type _Off) noexcept { - _Myoff -= _Off; + _Myoff = static_cast<_Size_type>(_Myoff - _Off); return *this; } @@ -263,9 +261,7 @@ public: "cannot deference out of range deque iterator"); #endif // _ITERATOR_DEBUG_LEVEL != 0 - _Size_type _Block = _Mycont->_Getblock(_Myoff); - _Size_type _Off = _Myoff % _Block_size; - return _Mycont->_Map[_Block][_Off]; + return _Mycont->_At(_Myoff); } _NODISCARD pointer operator->() const noexcept { @@ -317,7 +313,7 @@ public: } #endif // _ITERATOR_DEBUG_LEVEL != 0 - _Myoff += _Off; + _Myoff = static_cast<_Size_type>(_Myoff + _Off); return *this; } @@ -546,6 +542,8 @@ public: using _Mapptr = typename _Val_types::_Mapptr; private: + using _Map_difference_type = typename iterator_traits<_Mapptr>::difference_type; + static constexpr size_t _Bytes = sizeof(value_type); public: @@ -557,9 +555,21 @@ public: _Deque_val() noexcept : _Map(), _Mapsize(0), _Myoff(0), _Mysize(0) {} - size_type _Getblock(size_type _Off) const noexcept { + _Map_difference_type _Getblock(size_type _Off) const noexcept { // NB: _Mapsize and _Block_size are guaranteed to be powers of 2 - return (_Off / _Block_size) & (_Mapsize - 1); + return static_cast<_Map_difference_type>((_Off / _Block_size) & (_Mapsize - 1)); + } + + reference _At(size_type _Idx) noexcept { + const auto _Block = _Getblock(_Idx); + const auto _Off = static_cast(_Idx % _Block_size); + return _Map[_Block][_Off]; + } + + const_reference _At(size_type _Idx) const noexcept { + const auto _Block = _Getblock(_Idx); + const auto _Off = static_cast(_Idx % _Block_size); + return _Map[_Block][_Off]; } _Mapptr _Map; // pointer to array of pointers to blocks @@ -584,6 +594,8 @@ private: using _Mapptr = typename _Alpty_traits::pointer; using _Alproxy_ty = _Rebind_alloc_t<_Alty, _Container_proxy>; + using _Map_difference_type = typename iterator_traits<_Mapptr>::difference_type; + using _Scary_val = _Deque_val, _Deque_simple_types<_Ty>, _Deque_iter_types<_Ty, typename _Alty_traits::size_type, typename _Alty_traits::difference_type, typename _Alty_traits::pointer, typename _Alty_traits::const_pointer, _Mapptr>>>; @@ -987,7 +999,7 @@ public: _STL_VERIFY(_Pos < _Mysize(), "deque subscript out of range"); #endif // _CONTAINER_DEBUG_LEVEL > 0 - return *(_Unchecked_begin() + static_cast(_Pos)); + return _Get_data()._At(_Pos); } _NODISCARD reference operator[](size_type _Pos) noexcept /* strengthened */ { @@ -995,7 +1007,7 @@ public: _STL_VERIFY(_Pos < _Mysize(), "deque subscript out of range"); #endif // _CONTAINER_DEBUG_LEVEL > 0 - return *(_Unchecked_begin() + static_cast(_Pos)); + return _Get_data()._At(_Pos); } _NODISCARD const_reference at(size_type _Pos) const { @@ -1003,7 +1015,7 @@ public: _Xran(); } - return *(begin() + static_cast(_Pos)); + return _Get_data()._At(_Pos); } _NODISCARD reference at(size_type _Pos) { @@ -1011,7 +1023,7 @@ public: _Xran(); } - return *(begin() + static_cast(_Pos)); + return _Get_data()._At(_Pos); } _NODISCARD reference front() noexcept /* strengthened */ { @@ -1053,14 +1065,13 @@ private: _Growmap(1); } _Myoff() &= _Mapsize() * _Block_size - 1; - size_type _Newoff = _Myoff() + _Mysize(); - size_type _Block = _Getblock(_Newoff); + const auto _Newoff = static_cast(_Myoff() + _Mysize()); + const auto _Block = _Getblock(_Newoff); if (_Map()[_Block] == nullptr) { _Map()[_Block] = _Getal().allocate(_Block_size); } - _Alty_traits::construct( - _Getal(), _Unfancy(_Map()[_Block] + _Newoff % _Block_size), _STD forward<_Tys>(_Vals)...); + _Alty_traits::construct(_Getal(), _Address_at(_Newoff), _STD forward<_Tys>(_Vals)...); ++_Mysize(); } @@ -1071,14 +1082,13 @@ private: _Growmap(1); } _Myoff() &= _Mapsize() * _Block_size - 1; - size_type _Newoff = _Myoff() != 0 ? _Myoff() : _Mapsize() * _Block_size; - const size_type _Block = _Getblock(--_Newoff); + const auto _Newoff = static_cast((_Myoff() != 0 ? _Myoff() : _Mapsize() * _Block_size) - 1); + const auto _Block = _Getblock(_Newoff); if (_Map()[_Block] == nullptr) { _Map()[_Block] = _Getal().allocate(_Block_size); } - _Alty_traits::construct( - _Getal(), _Unfancy(_Map()[_Block] + _Newoff % _Block_size), _STD forward<_Tys>(_Vals)...); + _Alty_traits::construct(_Getal(), _Address_at(_Newoff), _STD forward<_Tys>(_Vals)...); _Myoff() = _Newoff; ++_Mysize(); @@ -1401,8 +1411,7 @@ public: _STL_REPORT_ERROR("deque empty before pop"); } else { // something to erase, do it _Orphan_off(_Myoff()); - size_type _Block = _Getblock(_Myoff()); - _Alty_traits::destroy(_Getal(), _Unfancy(_Map()[_Block] + _Myoff() % _Block_size)); + _Alty_traits::destroy(_Getal(), _Address_at(_Myoff())); if (--_Mysize() == 0) { _Myoff() = 0; } else { @@ -1411,8 +1420,7 @@ public: } #else // ^^^ _ITERATOR_DEBUG_LEVEL == 2 / _ITERATOR_DEBUG_LEVEL < 2 vvv - size_type _Block = _Getblock(_Myoff()); - _Alty_traits::destroy(_Getal(), _Unfancy(_Map()[_Block] + _Myoff() % _Block_size)); + _Alty_traits::destroy(_Getal(), _Address_at(_Myoff())); if (--_Mysize() == 0) { _Myoff() = 0; } else { @@ -1428,8 +1436,7 @@ public: } else { // something to erase, do it size_type _Newoff = _Myoff() + _Mysize() - 1; _Orphan_off(_Newoff); - size_type _Block = _Getblock(_Newoff); - _Alty_traits::destroy(_Getal(), _Unfancy(_Map()[_Block] + _Newoff % _Block_size)); + _Alty_traits::destroy(_Getal(), _Address_at(_Newoff)); if (--_Mysize() == 0) { _Myoff() = 0; } @@ -1437,8 +1444,7 @@ public: #else // ^^^ _ITERATOR_DEBUG_LEVEL == 2 / _ITERATOR_DEBUG_LEVEL < 2 vvv size_type _Newoff = _Myoff() + _Mysize() - 1; - size_type _Block = _Getblock(_Newoff); - _Alty_traits::destroy(_Getal(), _Unfancy(_Map()[_Block] + _Newoff % _Block_size)); + _Alty_traits::destroy(_Getal(), _Address_at(_Newoff)); if (--_Mysize() == 0) { _Myoff() = 0; } @@ -1543,9 +1549,10 @@ private: size_type _Allocsize = _Newsize; - size_type _Myboff = _Myoff() / _Block_size; - _Mapptr _Newmap = _Allocate_at_least_helper(_Almap, _Allocsize); - _Mapptr _Myptr = _Newmap + _Myboff; + const auto _Myboff = static_cast(_Myoff() / _Block_size); + const auto _Map_off = static_cast<_Map_difference_type>(_Myboff); + _Mapptr _Newmap = _Allocate_at_least_helper(_Almap, _Allocsize); + _Mapptr _Myptr = _Newmap + _Map_off; _STL_ASSERT(_Allocsize >= _Newsize, "_Allocsize >= _Newsize"); while (_Newsize <= _Allocsize / 2) { _Newsize *= 2; @@ -1553,19 +1560,21 @@ private: _Count = _Newsize - _Mapsize(); - _Myptr = _STD uninitialized_copy(_Map() + _Myboff, _Map() + _Mapsize(), _Myptr); // copy initial to end + const auto _Map_count = static_cast<_Map_difference_type>(_Count); + + _Myptr = _STD uninitialized_copy(_Map() + _Map_off, _Map() + _Map_distance(), _Myptr); // copy initial to end if (_Myboff <= _Count) { // increment greater than offset of initial block - _Myptr = _STD uninitialized_copy(_Map(), _Map() + _Myboff, _Myptr); // copy rest of old + _Myptr = _STD uninitialized_copy(_Map(), _Map() + _Map_off, _Myptr); // copy rest of old _Uninitialized_value_construct_n_unchecked1(_Myptr, _Count - _Myboff); // clear suffix of new _Uninitialized_value_construct_n_unchecked1(_Newmap, _Myboff); // clear prefix of new } else { // increment not greater than offset of initial block - _STD uninitialized_copy(_Map(), _Map() + _Count, _Myptr); // copy more old - _Myptr = _STD uninitialized_copy(_Map() + _Count, _Map() + _Myboff, _Newmap); // copy rest of old + _STD uninitialized_copy(_Map(), _Map() + _Map_count, _Myptr); // copy more old + _Myptr = _STD uninitialized_copy(_Map() + _Map_count, _Map() + _Map_off, _Newmap); // copy rest of old _Uninitialized_value_construct_n_unchecked1(_Myptr, _Count); // clear rest to initial block } if (_Map() != nullptr) { - _Destroy_range(_Map(), _Map() + _Mapsize()); + _Destroy_range(_Map(), _Map() + _Map_distance()); _Almap.deallocate(_Map(), _Mapsize()); // free storage for old } @@ -1582,7 +1591,7 @@ private: } if (_Map() != nullptr) { - for (size_type _Block = _Mapsize(); _Block > 0;) { // free storage for a block and destroy pointer + for (auto _Block = _Map_distance(); _Block > 0;) { // free storage for a block and destroy pointer if (_Map()[--_Block]) { // free block _Getal().deallocate(_Map()[_Block], _Block_size); } @@ -1617,7 +1626,7 @@ private: } #endif // _ITERATOR_DEBUG_LEVEL == 2 - size_type _Getblock(size_type _Off) const noexcept { + _Map_difference_type _Getblock(size_type _Off) const noexcept { return _Get_data()._Getblock(_Off); } @@ -1673,6 +1682,16 @@ private: return _Get_data()._Mysize; } + _Map_difference_type _Map_distance() const noexcept { + return static_cast<_Map_difference_type>(_Get_data()._Mapsize); + } + + _Ty* _Address_at(size_type _Idx) noexcept { + const auto _Block = _Getblock(_Idx); + const auto _Off = static_cast(_Idx % _Block_size); + return _STD _Unfancy(_Get_data()._Map[_Block] + _Off); + } + _Compressed_pair<_Alty, _Scary_val> _Mypair; }; diff --git a/tests/std/tests/GH_002769_handle_deque_block_pointers/test.cpp b/tests/std/tests/GH_002769_handle_deque_block_pointers/test.cpp index 28d2d7abf2..492938f08c 100644 --- a/tests/std/tests/GH_002769_handle_deque_block_pointers/test.cpp +++ b/tests/std/tests/GH_002769_handle_deque_block_pointers/test.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -12,7 +13,7 @@ using namespace std; size_t counter = 0; -template +template class counting_ptr { private: T* p_; @@ -24,13 +25,16 @@ class counting_ptr { template friend struct ptr_counting_allocator; + template + friend struct inconsistent_difference_allocator; + public: #ifdef __cpp_lib_concepts using iterator_concept = contiguous_iterator_tag; #endif // __cpp_lib_concepts using iterator_category = random_access_iterator_tag; using value_type = T; - using difference_type = ptrdiff_t; + using difference_type = Diff; using pointer = T*; using reference = add_lvalue_reference_t; @@ -90,46 +94,46 @@ class counting_ptr { return tmp; } - template , int> = 0> + template , int> = 0> T& operator[](I n) const noexcept { return p_[n]; } - template , int> = 0> + template , int> = 0> counting_ptr& operator+=(I n) noexcept { p_ += n; return *this; } - template , int> = 0> + template , int> = 0> counting_ptr& operator-=(I n) noexcept { p_ -= n; return *this; } - template , int> = 0> + template , int> = 0> friend counting_ptr operator+(const counting_ptr& p, I n) noexcept { auto tmp = p; tmp += n; return tmp; } - template , int> = 0> + template , int> = 0> friend counting_ptr operator+(I n, const counting_ptr& p) noexcept { auto tmp = p; tmp += n; return tmp; } - template , int> = 0> + template , int> = 0> friend counting_ptr operator-(const counting_ptr& p, I n) noexcept { auto tmp = p; tmp -= n; return tmp; } - friend ptrdiff_t operator-(const counting_ptr& lhs, const counting_ptr& rhs) noexcept { - return lhs.p_ - rhs.p_; + friend Diff operator-(const counting_ptr& lhs, const counting_ptr& rhs) noexcept { + return static_cast(lhs.p_ - rhs.p_); } friend bool operator==(const counting_ptr& p, nullptr_t) noexcept { @@ -138,8 +142,6 @@ class counting_ptr { #if _HAS_CXX20 friend bool operator==(const counting_ptr& lhs, const counting_ptr& rhs) = default; - - friend auto operator<=>(const counting_ptr& lhs, const counting_ptr& rhs) = default; #else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv friend bool operator==(nullptr_t, const counting_ptr& p) noexcept { return p.p_ == nullptr; @@ -211,6 +213,21 @@ struct ptr_counting_allocator { #endif // !_HAS_CXX20 }; +void test_gh_2769() { + { + deque> dq{3, 1, 4, 1, 5, 9}; + dq.insert(dq.end(), {2, 6, 5, 3, 5, 8}); + } + assert(counter == 0); + + { + deque> dq(979, 323); + dq.insert(dq.begin(), 84, 62); + dq.erase(dq.begin() + 64, dq.begin() + 338); + } + assert(counter == 0); +} + size_t count_limit = 0; struct live_counter { @@ -261,19 +278,55 @@ void test_gh_3717() { } } -int main() { +template +struct inconsistent_difference_allocator { + using value_type = T; + using size_type = size_t; + using difference_type = conditional_t, int64_t, int32_t>; + using pointer = counting_ptr; + + inconsistent_difference_allocator() = default; + + template + constexpr inconsistent_difference_allocator(inconsistent_difference_allocator) noexcept {} + + pointer allocate(size_type n) { + return pointer{allocator{}.allocate(n)}; + } + + void deallocate(pointer p, size_type n) { + allocator{}.deallocate(p.operator->(), n); + } + + template + friend constexpr bool operator==(inconsistent_difference_allocator, inconsistent_difference_allocator) noexcept { + return true; + } +#if !_HAS_CXX20 + template + friend constexpr bool operator!=(inconsistent_difference_allocator, inconsistent_difference_allocator) noexcept { + return false; + } +#endif // !_HAS_CXX20 +}; + +void test_inconsistent_difference_types() { { - deque> dq{3, 1, 4, 1, 5, 9}; + deque> dq{3, 1, 4, 1, 5, 9}; dq.insert(dq.end(), {2, 6, 5, 3, 5, 8}); } assert(counter == 0); { - deque> dq(979, 323); + deque> dq(979, 323); dq.insert(dq.begin(), 84, 62); dq.erase(dq.begin() + 64, dq.begin() + 338); } assert(counter == 0); +} +int main() { + test_gh_2769(); test_gh_3717(); + test_inconsistent_difference_types(); } From 21ac71c4bbfc0b7256542f7a90102a6362744001 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 25 Sep 2023 09:10:09 +0800 Subject: [PATCH 2/6] Fix `operator[]`! --- stl/inc/deque | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/deque b/stl/inc/deque index 9568236039..34e5030f7e 100644 --- a/stl/inc/deque +++ b/stl/inc/deque @@ -999,7 +999,7 @@ public: _STL_VERIFY(_Pos < _Mysize(), "deque subscript out of range"); #endif // _CONTAINER_DEBUG_LEVEL > 0 - return _Get_data()._At(_Pos); + return _Get_data()._At(_Myoff() + _Pos); } _NODISCARD reference operator[](size_type _Pos) noexcept /* strengthened */ { @@ -1007,7 +1007,7 @@ public: _STL_VERIFY(_Pos < _Mysize(), "deque subscript out of range"); #endif // _CONTAINER_DEBUG_LEVEL > 0 - return _Get_data()._At(_Pos); + return _Get_data()._At(_Myoff() + _Pos); } _NODISCARD const_reference at(size_type _Pos) const { From 92056a5d0b9803c28026a487b0d2ab3211f8a4dc Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 25 Sep 2023 09:16:44 +0800 Subject: [PATCH 3/6] Fix `deque::at` --- stl/inc/deque | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/deque b/stl/inc/deque index 34e5030f7e..dd1b688b0e 100644 --- a/stl/inc/deque +++ b/stl/inc/deque @@ -1015,7 +1015,7 @@ public: _Xran(); } - return _Get_data()._At(_Pos); + return _Get_data()._At(_Myoff() + _Pos); } _NODISCARD reference at(size_type _Pos) { @@ -1023,7 +1023,7 @@ public: _Xran(); } - return _Get_data()._At(_Pos); + return _Get_data()._At(_Myoff() + _Pos); } _NODISCARD reference front() noexcept /* strengthened */ { From 64ca5e0916af99199f7e81d173b18bb18ad177c5 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 25 Sep 2023 11:41:53 +0800 Subject: [PATCH 4/6] Revert dropping `operator<=>` --- tests/std/tests/GH_002769_handle_deque_block_pointers/test.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/std/tests/GH_002769_handle_deque_block_pointers/test.cpp b/tests/std/tests/GH_002769_handle_deque_block_pointers/test.cpp index 492938f08c..2d0803497b 100644 --- a/tests/std/tests/GH_002769_handle_deque_block_pointers/test.cpp +++ b/tests/std/tests/GH_002769_handle_deque_block_pointers/test.cpp @@ -142,6 +142,8 @@ class counting_ptr { #if _HAS_CXX20 friend bool operator==(const counting_ptr& lhs, const counting_ptr& rhs) = default; + + friend auto operator<=>(const counting_ptr& lhs, const counting_ptr& rhs) = default; #else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv friend bool operator==(nullptr_t, const counting_ptr& p) noexcept { return p.p_ == nullptr; From 6960fa5f1537088172eb40bd3be66689c6b68320 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Thu, 28 Sep 2023 10:07:02 +0800 Subject: [PATCH 5/6] Address naming and `_Address_at` issues per @achabense --- stl/inc/deque | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/stl/inc/deque b/stl/inc/deque index dd1b688b0e..1f2961aafd 100644 --- a/stl/inc/deque +++ b/stl/inc/deque @@ -560,16 +560,22 @@ public: return static_cast<_Map_difference_type>((_Off / _Block_size) & (_Mapsize - 1)); } - reference _At(size_type _Idx) noexcept { - const auto _Block = _Getblock(_Idx); - const auto _Off = static_cast(_Idx % _Block_size); - return _Map[_Block][_Off]; + reference _At(size_type _Off) noexcept { + const auto _Block = _Getblock(_Off); + const auto _Block_off = static_cast(_Off % _Block_size); + return _Map[_Block][_Block_off]; } - const_reference _At(size_type _Idx) const noexcept { - const auto _Block = _Getblock(_Idx); - const auto _Off = static_cast(_Idx % _Block_size); - return _Map[_Block][_Off]; + const_reference _At(size_type _Off) const noexcept { + const auto _Block = _Getblock(_Off); + const auto _Block_off = static_cast(_Off % _Block_size); + return _Map[_Block][_Block_off]; + } + + value_type* _Address_at(size_type _Off) noexcept { + const auto _Block = _Getblock(_Off); + const auto _Block_off = static_cast(_Off % _Block_size); + return _STD _Unfancy(_Map[_Block] + _Block_off); } _Mapptr _Map; // pointer to array of pointers to blocks @@ -1071,7 +1077,7 @@ private: _Map()[_Block] = _Getal().allocate(_Block_size); } - _Alty_traits::construct(_Getal(), _Address_at(_Newoff), _STD forward<_Tys>(_Vals)...); + _Alty_traits::construct(_Getal(), _Get_data()._Address_at(_Newoff), _STD forward<_Tys>(_Vals)...); ++_Mysize(); } @@ -1088,7 +1094,7 @@ private: _Map()[_Block] = _Getal().allocate(_Block_size); } - _Alty_traits::construct(_Getal(), _Address_at(_Newoff), _STD forward<_Tys>(_Vals)...); + _Alty_traits::construct(_Getal(), _Get_data()._Address_at(_Newoff), _STD forward<_Tys>(_Vals)...); _Myoff() = _Newoff; ++_Mysize(); @@ -1411,7 +1417,7 @@ public: _STL_REPORT_ERROR("deque empty before pop"); } else { // something to erase, do it _Orphan_off(_Myoff()); - _Alty_traits::destroy(_Getal(), _Address_at(_Myoff())); + _Alty_traits::destroy(_Getal(), _Get_data()._Address_at(_Myoff())); if (--_Mysize() == 0) { _Myoff() = 0; } else { @@ -1420,7 +1426,7 @@ public: } #else // ^^^ _ITERATOR_DEBUG_LEVEL == 2 / _ITERATOR_DEBUG_LEVEL < 2 vvv - _Alty_traits::destroy(_Getal(), _Address_at(_Myoff())); + _Alty_traits::destroy(_Getal(), _Get_data()._Address_at(_Myoff())); if (--_Mysize() == 0) { _Myoff() = 0; } else { @@ -1436,7 +1442,7 @@ public: } else { // something to erase, do it size_type _Newoff = _Myoff() + _Mysize() - 1; _Orphan_off(_Newoff); - _Alty_traits::destroy(_Getal(), _Address_at(_Newoff)); + _Alty_traits::destroy(_Getal(), _Get_data()._Address_at(_Newoff)); if (--_Mysize() == 0) { _Myoff() = 0; } @@ -1444,7 +1450,7 @@ public: #else // ^^^ _ITERATOR_DEBUG_LEVEL == 2 / _ITERATOR_DEBUG_LEVEL < 2 vvv size_type _Newoff = _Myoff() + _Mysize() - 1; - _Alty_traits::destroy(_Getal(), _Address_at(_Newoff)); + _Alty_traits::destroy(_Getal(), _Get_data()._Address_at(_Newoff)); if (--_Mysize() == 0) { _Myoff() = 0; } @@ -1686,12 +1692,6 @@ private: return static_cast<_Map_difference_type>(_Get_data()._Mapsize); } - _Ty* _Address_at(size_type _Idx) noexcept { - const auto _Block = _Getblock(_Idx); - const auto _Off = static_cast(_Idx % _Block_size); - return _STD _Unfancy(_Get_data()._Map[_Block] + _Off); - } - _Compressed_pair<_Alty, _Scary_val> _Mypair; }; From a4019d47290c7ddc3ee3cb5f79ed22eedc2b5421 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 2 Oct 2023 19:01:52 -0700 Subject: [PATCH 6/6] Code review feedback. --- stl/inc/deque | 30 +++++++++---------- .../test.cpp | 1 + 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/stl/inc/deque b/stl/inc/deque index 1f2961aafd..2f23acdad8 100644 --- a/stl/inc/deque +++ b/stl/inc/deque @@ -42,7 +42,7 @@ public: : _Mycont(static_cast(_Pdeque)), _Myoff(_Off) {} _NODISCARD reference operator*() const noexcept { - return _Mycont->_At(_Myoff); + return _Mycont->_Subscript(_Myoff); } _NODISCARD pointer operator->() const noexcept { @@ -261,7 +261,7 @@ public: "cannot deference out of range deque iterator"); #endif // _ITERATOR_DEBUG_LEVEL != 0 - return _Mycont->_At(_Myoff); + return _Mycont->_Subscript(_Myoff); } _NODISCARD pointer operator->() const noexcept { @@ -560,19 +560,19 @@ public: return static_cast<_Map_difference_type>((_Off / _Block_size) & (_Mapsize - 1)); } - reference _At(size_type _Off) noexcept { + reference _Subscript(size_type _Off) noexcept { const auto _Block = _Getblock(_Off); const auto _Block_off = static_cast(_Off % _Block_size); return _Map[_Block][_Block_off]; } - const_reference _At(size_type _Off) const noexcept { + const_reference _Subscript(size_type _Off) const noexcept { const auto _Block = _Getblock(_Off); const auto _Block_off = static_cast(_Off % _Block_size); return _Map[_Block][_Block_off]; } - value_type* _Address_at(size_type _Off) noexcept { + value_type* _Address_subscript(size_type _Off) noexcept { const auto _Block = _Getblock(_Off); const auto _Block_off = static_cast(_Off % _Block_size); return _STD _Unfancy(_Map[_Block] + _Block_off); @@ -1005,7 +1005,7 @@ public: _STL_VERIFY(_Pos < _Mysize(), "deque subscript out of range"); #endif // _CONTAINER_DEBUG_LEVEL > 0 - return _Get_data()._At(_Myoff() + _Pos); + return _Get_data()._Subscript(_Myoff() + _Pos); } _NODISCARD reference operator[](size_type _Pos) noexcept /* strengthened */ { @@ -1013,7 +1013,7 @@ public: _STL_VERIFY(_Pos < _Mysize(), "deque subscript out of range"); #endif // _CONTAINER_DEBUG_LEVEL > 0 - return _Get_data()._At(_Myoff() + _Pos); + return _Get_data()._Subscript(_Myoff() + _Pos); } _NODISCARD const_reference at(size_type _Pos) const { @@ -1021,7 +1021,7 @@ public: _Xran(); } - return _Get_data()._At(_Myoff() + _Pos); + return _Get_data()._Subscript(_Myoff() + _Pos); } _NODISCARD reference at(size_type _Pos) { @@ -1029,7 +1029,7 @@ public: _Xran(); } - return _Get_data()._At(_Myoff() + _Pos); + return _Get_data()._Subscript(_Myoff() + _Pos); } _NODISCARD reference front() noexcept /* strengthened */ { @@ -1077,7 +1077,7 @@ private: _Map()[_Block] = _Getal().allocate(_Block_size); } - _Alty_traits::construct(_Getal(), _Get_data()._Address_at(_Newoff), _STD forward<_Tys>(_Vals)...); + _Alty_traits::construct(_Getal(), _Get_data()._Address_subscript(_Newoff), _STD forward<_Tys>(_Vals)...); ++_Mysize(); } @@ -1094,7 +1094,7 @@ private: _Map()[_Block] = _Getal().allocate(_Block_size); } - _Alty_traits::construct(_Getal(), _Get_data()._Address_at(_Newoff), _STD forward<_Tys>(_Vals)...); + _Alty_traits::construct(_Getal(), _Get_data()._Address_subscript(_Newoff), _STD forward<_Tys>(_Vals)...); _Myoff() = _Newoff; ++_Mysize(); @@ -1417,7 +1417,7 @@ public: _STL_REPORT_ERROR("deque empty before pop"); } else { // something to erase, do it _Orphan_off(_Myoff()); - _Alty_traits::destroy(_Getal(), _Get_data()._Address_at(_Myoff())); + _Alty_traits::destroy(_Getal(), _Get_data()._Address_subscript(_Myoff())); if (--_Mysize() == 0) { _Myoff() = 0; } else { @@ -1426,7 +1426,7 @@ public: } #else // ^^^ _ITERATOR_DEBUG_LEVEL == 2 / _ITERATOR_DEBUG_LEVEL < 2 vvv - _Alty_traits::destroy(_Getal(), _Get_data()._Address_at(_Myoff())); + _Alty_traits::destroy(_Getal(), _Get_data()._Address_subscript(_Myoff())); if (--_Mysize() == 0) { _Myoff() = 0; } else { @@ -1442,7 +1442,7 @@ public: } else { // something to erase, do it size_type _Newoff = _Myoff() + _Mysize() - 1; _Orphan_off(_Newoff); - _Alty_traits::destroy(_Getal(), _Get_data()._Address_at(_Newoff)); + _Alty_traits::destroy(_Getal(), _Get_data()._Address_subscript(_Newoff)); if (--_Mysize() == 0) { _Myoff() = 0; } @@ -1450,7 +1450,7 @@ public: #else // ^^^ _ITERATOR_DEBUG_LEVEL == 2 / _ITERATOR_DEBUG_LEVEL < 2 vvv size_type _Newoff = _Myoff() + _Mysize() - 1; - _Alty_traits::destroy(_Getal(), _Get_data()._Address_at(_Newoff)); + _Alty_traits::destroy(_Getal(), _Get_data()._Address_subscript(_Newoff)); if (--_Mysize() == 0) { _Myoff() = 0; } diff --git a/tests/std/tests/GH_002769_handle_deque_block_pointers/test.cpp b/tests/std/tests/GH_002769_handle_deque_block_pointers/test.cpp index 2d0803497b..34fde2bafc 100644 --- a/tests/std/tests/GH_002769_handle_deque_block_pointers/test.cpp +++ b/tests/std/tests/GH_002769_handle_deque_block_pointers/test.cpp @@ -215,6 +215,7 @@ struct ptr_counting_allocator { #endif // !_HAS_CXX20 }; +// GH-2769 : For allocators where allocator_traits::pointer is an object, destructors aren't always called void test_gh_2769() { { deque> dq{3, 1, 4, 1, 5, 9};