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>: Fix single-element insertion of deque #4022

Merged
merged 12 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 29 additions & 21 deletions stl/inc/deque
Original file line number Diff line number Diff line change
Expand Up @@ -871,13 +871,35 @@ public:
_STL_VERIFY(_Off <= _Mysize(), "deque emplace iterator outside range");
#endif // _ITERATOR_DEBUG_LEVEL == 2

if (_Off <= _Mysize() / 2) { // closer to front, push to front then rotate
emplace_front(_STD forward<_Valty>(_Val)...);
_STD rotate(begin(), _Next_iter(begin()), begin() + static_cast<difference_type>(1 + _Off));
} else { // closer to back, push to back then rotate
emplace_back(_STD forward<_Valty>(_Val)...);
_STD rotate(begin() + static_cast<difference_type>(_Off), _Prev_iter(end()), end());
_Orphan_all();
if (_Off == 0) { // at the beginning
_Emplace_front_internal(_STD forward<_Valty>(_Val)...);
} else if (_Off <= _Mysize() / 2) { // closer to front
_Alloc_temporary2<_Alty> _Obj(_Getal(), _STD forward<_Valty>(_Val)...); // handle aliasing
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved

_Emplace_front_internal(_STD move(*_Unchecked_begin()));

auto _Moving_dst = _STD _Next_iter(_Unchecked_begin());
auto _Moving_src_begin = _STD _Next_iter(_Moving_dst);
auto _Moving_src_end = _Moving_dst + static_cast<difference_type>(_Off);

auto _Moving_dst_end = _STD _Move_unchecked(_Moving_src_begin, _Moving_src_end, _Moving_dst);
*_Moving_dst_end = _STD move(_Obj._Get_value());
} else if (_Off != _Mysize()) { // closer to back
_Alloc_temporary2<_Alty> _Obj(_Getal(), _STD forward<_Valty>(_Val)...); // handle aliasing

_Emplace_back_internal(_STD move(*_STD _Prev_iter(_Unchecked_end())));

auto _Moving_dst_end = _STD _Prev_iter(_Unchecked_end());
auto _Moving_src_end = _STD _Prev_iter(_Moving_dst_end);
auto _Moving_src_begin = _Unchecked_begin() + static_cast<difference_type>(_Off);

_STD _Move_backward_unchecked(_Moving_src_begin, _Moving_src_end, _Moving_dst_end);
*_Moving_src_begin = _STD move(_Obj._Get_value());
} else { // at the end
_Emplace_back_internal(_STD forward<_Valty>(_Val)...);
}

return begin() + static_cast<difference_type>(_Off);
}

Expand Down Expand Up @@ -1267,21 +1289,7 @@ public:
}

iterator insert(const_iterator _Where, const _Ty& _Val) {
size_type _Off = static_cast<size_type>(_Where - begin());

#if _ITERATOR_DEBUG_LEVEL == 2
_STL_VERIFY(_Off <= _Mysize(), "deque insert iterator outside range");
#endif // _ITERATOR_DEBUG_LEVEL == 2

if (_Off <= _Mysize() / 2) { // closer to front, push to front then copy
push_front(_Val);
_STD rotate(begin(), _Next_iter(begin()), begin() + static_cast<difference_type>(1 + _Off));
} else { // closer to back, push to back then copy
push_back(_Val);
_STD rotate(begin() + static_cast<difference_type>(_Off), _Prev_iter(end()), end());
}

return begin() + static_cast<difference_type>(_Off);
return emplace(_Where, _Val);
}

iterator insert(const_iterator _Where, _CRT_GUARDOVERFLOW size_type _Count, const _Ty& _Val) {
Expand Down
185 changes: 180 additions & 5 deletions tests/std/tests/Dev10_860421_deque_push_back_pop_front/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@

#include <cassert>
#include <deque>
#include <stdexcept>
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
#include <type_traits>
#include <utility>
#include <vector>

using namespace std;

void test_391805();

int main() {
void test_push_back_pop_front() {
deque<int> d;

for (int n = 0; n < 1000; ++n) {
Expand All @@ -31,8 +32,6 @@ int main() {
assert(d[i] == *v[i]);
}
}

test_391805();
}

// Also test Dev10-391805 "STL: Prefast error in deque".
Expand All @@ -47,3 +46,179 @@ void test_391805() {

assert(d.size() == 4 && d[0] == 40 && d[1] == 30 && d[2] == 10 && d[3] == 20);
}

// Also test GH-1023 "<deque>: std::deque::insert performance"
// - support for single-element insertion of non-swappable type, and
// - exception safety for single-element insertion.

struct ThrowingConstruction {
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
explicit ThrowingConstruction() = default;
};

class UniqueError : public std::exception {};
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved

class NonswappableMovable {
public:
NonswappableMovable() = default;
NonswappableMovable(NonswappableMovable&& other) noexcept : payload{exchange(other.payload, -1)} {}
NonswappableMovable(const NonswappableMovable&) = default;

explicit NonswappableMovable(int n) noexcept : payload{n} {}
explicit NonswappableMovable(ThrowingConstruction) {
throw UniqueError{};
}

NonswappableMovable& operator=(NonswappableMovable&& other) noexcept {
payload = exchange(other.payload, -1);
return *this;
}
NonswappableMovable& operator=(const NonswappableMovable&) = default;

#if _HAS_CXX20
friend bool operator==(const NonswappableMovable&, const NonswappableMovable&) = default;
#else // ^^^ C++20 or later / C++17 or earlier
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
friend bool operator==(const NonswappableMovable& lhs, const NonswappableMovable& rhs) noexcept {
return lhs.payload == rhs.payload;
}

friend bool operator!=(const NonswappableMovable& lhs, const NonswappableMovable& rhs) noexcept {
return lhs.payload != rhs.payload;
}
#endif // ^^^ C++17 or earlier ^^^

friend void swap(NonswappableMovable, NonswappableMovable) = delete;
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved

private:
int payload = -1;
};

#if _HAS_CXX17
static_assert(!is_swappable_v<NonswappableMovable>);
#endif // _HAS_CXX17

void test_exception_safety_for_nonswappable_movable() {
using Diff = deque<NonswappableMovable>::difference_type;

deque<NonswappableMovable> d;
for (int i = 0; i < 10; ++i) {
d.emplace_back(i);
}

{
auto it = d.emplace(d.begin() + Diff{3}, 42);
assert(it == d.begin() + Diff{3});
assert(d[3] == NonswappableMovable{42});
}
{
auto it = d.emplace(d.end() - Diff{3}, 1729);
assert(it == d.end() - Diff{4});
assert(d[d.size() - 4] == NonswappableMovable{1729});
}
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved

const auto d_orig = d;
try {
d.emplace_front(ThrowingConstruction{});
assert(false);
} catch (const UniqueError&) {
}
assert(d == d_orig);

try {
d.emplace_back(ThrowingConstruction{});
assert(false);
} catch (const UniqueError&) {
}
assert(d == d_orig);

try {
d.emplace(d.begin() + Diff{2}, ThrowingConstruction{});
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
assert(false);
} catch (const UniqueError&) {
}
assert(d == d_orig);

try {
d.emplace(d.end() - Diff{2}, ThrowingConstruction{});
assert(false);
} catch (const UniqueError&) {
}
assert(d == d_orig);
}

class ThrowingMovable {
public:
ThrowingMovable() = default;
ThrowingMovable(ThrowingMovable&&) {
throw UniqueError{};
}
ThrowingMovable(const ThrowingMovable&) noexcept = default;
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved

explicit ThrowingMovable(int n) noexcept : payload{n} {}

ThrowingMovable& operator=(ThrowingMovable&&) {
throw UniqueError{};
}
ThrowingMovable& operator=(const ThrowingMovable&) noexcept = default;

#if _HAS_CXX20
friend bool operator==(const ThrowingMovable&, const ThrowingMovable&) = default;
#else // ^^^ C++20 or later / C++17 or earlier
friend bool operator==(const ThrowingMovable& lhs, const ThrowingMovable& rhs) noexcept {
return lhs.payload == rhs.payload;
}

friend bool operator!=(const ThrowingMovable& lhs, const ThrowingMovable& rhs) noexcept {
return lhs.payload != rhs.payload;
}
#endif // ^^^ C++17 or earlier ^^^

private:
int payload = -1;
};

void test_exception_safety_for_throwing_movable() {
using Diff = deque<ThrowingMovable>::difference_type;

deque<ThrowingMovable> d;
for (int i = 0; i < 10; ++i) {
d.emplace_back(i);
}

const auto d_orig = d;
try {
d.emplace_front(ThrowingMovable{});
assert(false);
} catch (const UniqueError&) {
}
assert(d == d_orig);

try {
d.emplace_back(ThrowingMovable{});
assert(false);
} catch (const UniqueError&) {
}
assert(d == d_orig);

try {
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
d.emplace(d.begin() + Diff{2}, ThrowingMovable{});
assert(false);
} catch (const UniqueError&) {
}
assert(d == d_orig);

try {
d.emplace(d.end() - Diff{2}, ThrowingMovable{});
assert(false);
} catch (const UniqueError&) {
}
assert(d == d_orig);
}

int main() {
test_push_back_pop_front();

test_391805();

test_exception_safety_for_nonswappable_movable();
test_exception_safety_for_throwing_movable();
}