From 385f1335956684135d0766d70a4a5cb7153c09af Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 22 Sep 2023 05:43:16 +0800 Subject: [PATCH] ``: Fix single-element insertion of `deque` (#4022) Co-authored-by: Stephan T. Lavavej --- stl/inc/deque | 34 ++- .../test.cpp | 230 +++++++++++++++++- 2 files changed, 253 insertions(+), 11 deletions(-) diff --git a/stl/inc/deque b/stl/inc/deque index 938390450b..964b8c4f77 100644 --- a/stl/inc/deque +++ b/stl/inc/deque @@ -1112,13 +1112,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(1 + _Off)); - } else { // closer to back, push to back then rotate - emplace_back(_STD forward<_Valty>(_Val)...); - _STD rotate(begin() + static_cast(_Off), _Prev_iter(end()), end()); + _Orphan_all(); + if (_Off == 0) { // at the beginning + _Emplace_front_internal(_STD forward<_Valty>(_Val)...); + } else if (_Off == _Mysize()) { // at the end + _Emplace_back_internal(_STD forward<_Valty>(_Val)...); + } else { + _Alloc_temporary2<_Alty> _Obj(_Getal(), _STD forward<_Valty>(_Val)...); // handle aliasing + + if (_Off <= _Mysize() / 2) { // closer to front + _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(_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 { // closer to back + _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(_Off); + + _STD _Move_backward_unchecked(_Moving_src_begin, _Moving_src_end, _Moving_dst_end); + *_Moving_src_begin = _STD move(_Obj._Get_value()); + } } + return begin() + static_cast(_Off); } diff --git a/tests/std/tests/Dev10_860421_deque_push_back_pop_front/test.cpp b/tests/std/tests/Dev10_860421_deque_push_back_pop_front/test.cpp index 3a9e78d92c..72ff7bd48c 100644 --- a/tests/std/tests/Dev10_860421_deque_push_back_pop_front/test.cpp +++ b/tests/std/tests/Dev10_860421_deque_push_back_pop_front/test.cpp @@ -1,15 +1,16 @@ // Copyright (c) Microsoft Corporation. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +#include #include #include +#include +#include #include using namespace std; -void test_391805(); - -int main() { +void test_push_back_pop_front() { deque d; for (int n = 0; n < 1000; ++n) { @@ -31,8 +32,6 @@ int main() { assert(d[i] == *v[i]); } } - - test_391805(); } // Also test Dev10-391805 "STL: Prefast error in deque". @@ -47,3 +46,224 @@ void test_391805() { assert(d.size() == 4 && d[0] == 40 && d[1] == 30 && d[2] == 10 && d[3] == 20); } + +// Also test GH-1023 ": std::deque::insert performance" +// - support for single-element insertion of non-swappable type, and +// - exception safety for single-element insertion. + +struct ThrowingConstructionTag { + explicit ThrowingConstructionTag() = default; +}; + +struct UniqueError { + explicit UniqueError() = default; +}; + +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(ThrowingConstructionTag) { + 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 // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv + 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 // ^^^ !_HAS_CXX20 ^^^ + + friend void swap(NonswappableMovable&, NonswappableMovable&) = delete; + +private: + int payload = -1; +}; + +#if _HAS_CXX17 +static_assert(!is_swappable_v); +#endif // _HAS_CXX17 + +void test_exception_safety_for_nonswappable_movable() { + using Diff = deque::difference_type; + + deque d; + for (int i = 0; i < 10; ++i) { + d.emplace_back(i); + } + + { + auto it = d.emplace(d.begin(), 33); + assert(it == d.begin()); + assert(d.front() == NonswappableMovable{33}); + } + { + 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}); + } + { + auto it = d.emplace(d.end(), 2023); + assert(it == d.end() - Diff{1}); + assert(d.back() == NonswappableMovable{2023}); + } + { + static constexpr int correct[] = {33, 0, 1, 42, 2, 3, 4, 5, 6, 1729, 7, 8, 9, 2023}; + auto comp = [](const NonswappableMovable& lhs, const int rhs) { return lhs == NonswappableMovable{rhs}; }; + assert(equal(d.begin(), d.end(), begin(correct), end(correct), comp)); + } + + const auto d_orig = d; + try { + d.emplace_front(ThrowingConstructionTag{}); + assert(false); + } catch (const UniqueError&) { + } + assert(d == d_orig); + + try { + d.emplace_back(ThrowingConstructionTag{}); + assert(false); + } catch (const UniqueError&) { + } + assert(d == d_orig); + + try { + d.emplace(d.begin(), ThrowingConstructionTag{}); + assert(false); + } catch (const UniqueError&) { + } + assert(d == d_orig); + + try { + d.emplace(d.begin() + Diff{2}, ThrowingConstructionTag{}); + assert(false); + } catch (const UniqueError&) { + } + assert(d == d_orig); + + try { + d.emplace(d.end() - Diff{2}, ThrowingConstructionTag{}); + assert(false); + } catch (const UniqueError&) { + } + assert(d == d_orig); + + try { + d.emplace(d.end(), ThrowingConstructionTag{}); + assert(false); + } catch (const UniqueError&) { + } + assert(d == d_orig); +} + +class ThrowingMovable { +public: + ThrowingMovable() = default; + ThrowingMovable(ThrowingMovable&&) { + throw UniqueError{}; + } + ThrowingMovable(const ThrowingMovable&) = default; + + explicit ThrowingMovable(int n) noexcept : payload{n} {} + + ThrowingMovable& operator=(ThrowingMovable&&) { + throw UniqueError{}; + } + ThrowingMovable& operator=(const ThrowingMovable&) = default; + +#if _HAS_CXX20 + friend bool operator==(const ThrowingMovable&, const ThrowingMovable&) = default; +#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv + 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 // ^^^ !_HAS_CXX20 ^^^ + +private: + int payload = -1; +}; + +void test_exception_safety_for_throwing_movable() { + using Diff = deque::difference_type; + + deque 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 { + d.emplace(d.begin(), ThrowingMovable{}); + assert(false); + } catch (const UniqueError&) { + } + assert(d == d_orig); + + try { + 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); + + try { + d.emplace(d.end(), 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(); +}