Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<deque>: Properly destroy (fancy) pointers to blocks in the internal map #2775

Merged
merged 13 commits into from
Jun 12, 2022
46 changes: 23 additions & 23 deletions stl/inc/deque
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ private:

void _Construct_n(size_type _Count, const _Ty& _Val) { // construct from _Count * _Val
_Tidy_guard<deque> _Guard{this};
for (; 0 < _Count; --_Count) {
for (; _Count > 0; --_Count) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change requested: We generally try to avoid mixing unrelated cleanups into behavioral changes (as this increases the difficulty of reviewing and understanding source history). This isn't an ironclad rule - we're human and it's easier to fold changes into an existing PR, so a small amount of cleanup can be fine. In this case I think it's fine, but I wanted to mention it.

_Emplace_back_internal(_Val);
}

Expand Down Expand Up @@ -964,7 +964,7 @@ public:
_Newcapacity = _Block_size * _Minimum_map_size;
}

if ((empty() && 0 < _Mapsize())
if ((empty() && _Mapsize() > 0)
|| (!empty() && size() <= _Newcapacity && _Newcapacity < _Oldcapacity)) { // worth shrinking, do it
deque _Tmp(_STD make_move_iterator(begin()), _STD make_move_iterator(end()));
swap(_Tmp);
Expand All @@ -976,7 +976,7 @@ public:
emplace_back();
}

while (_Newsize < _Mysize()) {
while (_Mysize() > _Newsize) {
pop_back();
}
}
Expand All @@ -987,7 +987,7 @@ public:
_Emplace_back_internal(_Val);
}

while (_Newsize < _Mysize()) {
while (_Mysize() > _Newsize) {
pop_back();
}
}
Expand Down Expand Up @@ -1181,15 +1181,15 @@ public:
auto _Myfirst = _Unchecked_begin();
const auto _Oldsize = _Mysize();
auto _Assign_count = (_STD min)(_Count, _Oldsize);
for (; 0 < _Assign_count; --_Assign_count) {
for (; _Assign_count > 0; --_Assign_count) {
*_Myfirst = _Val;
++_Myfirst;
}

const auto _Shrink_by = _Oldsize - _Assign_count;
auto _Extend_by = _Count - _Assign_count;
_Erase_last_n(_Shrink_by);
for (; 0 < _Extend_by; --_Extend_by) {
for (; _Extend_by > 0; --_Extend_by) {
_Emplace_back_internal(_Val);
}
}
Expand Down Expand Up @@ -1304,7 +1304,7 @@ public:

auto _Off = static_cast<size_type>(_First - begin());
auto _Count = static_cast<size_type>(_Last - _First);
bool _Moved = 0 < _Off && _Off + _Count < _Mysize();
bool _Moved = _Off > 0 && _Off + _Count < _Mysize();

#else // _ITERATOR_DEBUG_LEVEL == 2
auto _Off = static_cast<size_type>(_First - begin());
Expand All @@ -1317,12 +1317,12 @@ public:

if (_Off < static_cast<size_type>(end() - _Last)) { // closer to front
_STD move_backward(begin(), _First, _Last); // copy over hole
for (; 0 < _Count; --_Count) {
for (; _Count > 0; --_Count) {
pop_front(); // pop copied elements
}
} else { // closer to back
_STD move(_Last, end(), _First); // copy over hole
for (; 0 < _Count; --_Count) {
for (; _Count > 0; --_Count) {
pop_back(); // pop copied elements
}
}
Expand All @@ -1338,7 +1338,7 @@ public:

private:
void _Erase_last_n(size_type _Count) noexcept {
for (; 0 < _Count; --_Count) {
for (; _Count > 0; --_Count) {
pop_back();
}
}
Expand Down Expand Up @@ -1376,17 +1376,17 @@ private:
if (_Off < _Rem) { // closer to front
_Restore_old_size_guard<_Pop_direction::_Front> _Guard{this, _Oldsize};
if (_Off < _Count) { // insert longer than prefix
for (_Num = _Count - _Off; 0 < _Num; --_Num) {
for (_Num = _Count - _Off; _Num > 0; --_Num) {
push_front(_Val); // push excess values
}
for (_Num = _Off; 0 < _Num; --_Num) {
for (_Num = _Off; _Num > 0; --_Num) {
push_front(begin()[static_cast<difference_type>(_Count - 1)]); // push prefix
}

_Mid = begin() + static_cast<difference_type>(_Count);
_STD fill(_Mid, _Mid + static_cast<difference_type>(_Off), _Val); // fill in rest of values
} else { // insert not longer than prefix
for (_Num = _Count; 0 < _Num; --_Num) {
for (_Num = _Count; _Num > 0; --_Num) {
push_front(begin()[static_cast<difference_type>(_Count - 1)]); // push part of prefix
}

Expand All @@ -1402,7 +1402,7 @@ private:
_Restore_old_size_guard<_Pop_direction::_Back> _Guard{this, _Oldsize};
if (_Rem < _Count) { // insert longer than suffix
_Orphan_all();
for (_Num = _Count - _Rem; 0 < _Num; --_Num) {
for (_Num = _Count - _Rem; _Num > 0; --_Num) {
_Emplace_back_internal(_Val); // push excess values
}
for (_Num = 0; _Num < _Rem; ++_Num) {
Expand Down Expand Up @@ -1437,10 +1437,10 @@ private:
}

void _Growmap(size_type _Count) { // grow map by at least _Count pointers, _Mapsize() a power of 2
static_assert(1 < _Minimum_map_size, "The _Xlen() test should always be performed.");
static_assert(_Minimum_map_size > 1, "The _Xlen() test should always be performed.");

_Alpty _Almap(_Getal());
size_type _Newsize = 0 < _Mapsize() ? _Mapsize() : 1;
size_type _Newsize = _Mapsize() > 0 ? _Mapsize() : 1;
while (_Newsize - _Mapsize() < _Count || _Newsize < _Minimum_map_size) {
// scale _Newsize to 2^N >= _Mapsize() + _Count
if (max_size() / _Block_size - _Newsize < _Newsize) {
Expand All @@ -1466,8 +1466,8 @@ private:
_Uninitialized_value_construct_n_unchecked1(_Myptr, _Count); // clear rest to initial block
}

_Destroy_range(_Map() + _Myboff, _Map() + _Mapsize());
if (_Map() != _Mapptr()) {
_Destroy_range(_Map(), _Map() + _Mapsize());
_Almap.deallocate(_Map(), _Mapsize()); // free storage for old
}

Expand All @@ -1483,14 +1483,14 @@ private:
pop_back();
}

for (size_type _Block = _Mapsize(); 0 < _Block;) { // free storage for a block and destroy pointer
if (_Map()[--_Block]) { // free block and destroy its pointer
_Getal().deallocate(_Map()[_Block], _Block_size);
_Destroy_in_place(_Map()[_Block]);
if (_Map() != _Mapptr()) {
for (size_type _Block = _Mapsize(); _Block > 0;) { // free storage for a block and destroy pointer
if (_Map()[--_Block]) { // free block
_Getal().deallocate(_Map()[_Block], _Block_size);
}
_Destroy_in_place(_Map()[_Block]); // destroy pointer to block
}
}

if (_Map() != _Mapptr()) {
_Almap.deallocate(_Map(), _Mapsize()); // free storage for map
}

Expand Down
1 change: 1 addition & 0 deletions tests/std/test.lst
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ tests\GH_002581_common_reference_workaround
tests\GH_002655_alternate_name_broke_linker
tests\GH_002711_Zc_alignedNew-
tests\GH_002760_syncstream_memory_leak
tests\GH_002769_handle_deque_block_pointers
tests\LWG2597_complex_branch_cut
tests\LWG3018_shared_ptr_function
tests\LWG3121_constrained_tuple_forwarding_ctor
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

RUNALL_INCLUDE ..\usual_matrix.lst
Loading