<deque>: Make deque::shrink_to_fit never relocate elements
#4091
Conversation
Properly handle the internal circular buffer!
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
Co-authored-by: achabense <[email protected]>
| { | ||
| deque<ThrowingMovable> deq(128); | ||
| deq.pop_back(); | ||
| deq.emplace_front(0); |
There was a problem hiding this comment.
I was having an assumption that the deque will be in all-full state. However, after debugging I realize that the _Mapsize() is already 64 (which means the capacity is already (roughly) 256 instead of 128), and _First_block_idx==63 and _Last_block_idx==32, which are not closely related as I originally supposed.
No changes requested, as what's going on here is enough to catch out the bug, and I have no idea how to improve the test here (mainly, how to control the relative position of _First_block_idx and _Last_block_idx in an obvious way?). I just want to point out that 128, together with pop-then-push, wrongly gives an impression that deq is in full state and thus it's testing a corner-case - no, this is not a corner case.
There was a problem hiding this comment.
Yeah, deque is really weird. I agree that this coverage is reasonable and I agree that it's not worth the extra effort to try to write a really comprehensive test.
…_difference_type>`.
tests/std/tests/Dev10_860421_deque_push_back_pop_front/test.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/Dev10_860421_deque_push_back_pop_front/test.cpp
Outdated
Show resolved
Hide resolved
tests/std/tests/Dev10_860421_deque_push_back_pop_front/test.cpp
Outdated
Show resolved
Hide resolved
| { | ||
| deque<ThrowingMovable> deq(128); | ||
| deq.pop_back(); | ||
| deq.emplace_front(0); |
There was a problem hiding this comment.
Yeah, deque is really weird. I agree that this coverage is reasonable and I agree that it's not worth the extra effort to try to write a really comprehensive test.
|
Thank you! 😻 After getting up to speed with your excellent overhaul here, I pushed a bunch of changes to make the logic clearer for my cat-sized brain. Please double-check that I didn't mess anything up. This is vastly better than our original implementation of |
|
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
|
Thanks for fixing everyone's favorite STL container! 🛠️ 😹 🎉 |
Fixes #4072 by providing stronger guarantee.