From d5f74351907e4edf6d0af2baf216287c7bd3aeab Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Wed, 13 Sep 2023 07:19:03 +0800 Subject: [PATCH 01/10] Fix single-element insertion of `deque` --- stl/inc/deque | 63 ++++-- .../test.cpp | 196 +++++++++++++++++- 2 files changed, 233 insertions(+), 26 deletions(-) diff --git a/stl/inc/deque b/stl/inc/deque index 2800f21144..0602896963 100644 --- a/stl/inc/deque +++ b/stl/inc/deque @@ -871,13 +871,48 @@ 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()); + constexpr bool _Uses_move_assignment = + disjunction_v, negation>>; + + _Orphan_all(); + if (_Off == 0) { // at the beginning + _Emplace_front_internal(_STD forward<_Valty>(_Val)...); + } else if (_Off <= _Mysize() / 2) { // closer to front, push to front then rotate + auto& _Al = _Getal(); + _Alloc_temporary2<_Alty> _Obj(_Al, _STD forward<_Valty>(_Val)...); // handle aliasing + + auto _Old_unwrapped_begin = _Unchecked_begin(); + auto _Old_unwrapped_where = _Old_unwrapped_begin + static_cast(_Off); + + _Emplace_front_internal(_STD move_if_noexcept(*_Old_unwrapped_begin)); + if constexpr (_Uses_move_assignment) { + _STD _Move_unchecked(_STD _Next_iter(_Old_unwrapped_begin), _Old_unwrapped_where, _Old_unwrapped_begin); + *_STD _Prev_iter(_Old_unwrapped_where) = _STD move(_Obj._Get_value()); + } else { + _STD _Copy_unchecked(_STD _Next_iter(_Old_unwrapped_begin), _Old_unwrapped_where, _Old_unwrapped_begin); + *_STD _Prev_iter(_Old_unwrapped_where) = _STD as_const(_Obj._Get_value()); + } + } else if (_Off != _Mysize()) { // closer to back, push to back then rotate + auto& _Al = _Getal(); + _Alloc_temporary2<_Alty> _Obj(_Al, _STD forward<_Valty>(_Val)...); // handle aliasing + + auto _Old_unwrapped_end = _Unchecked_end(); + auto _Old_unwrapped_where = _Unchecked_begin() + static_cast(_Off); + + _Emplace_back_internal(_STD move_if_noexcept(*_STD _Prev_iter(_Old_unwrapped_end))); + if constexpr (_Uses_move_assignment) { + (void) _STD _Move_backward_unchecked( + _Old_unwrapped_where, _STD _Prev_iter(_Old_unwrapped_end), _Old_unwrapped_end); + *_Old_unwrapped_where = _STD move(_Obj._Get_value()); + } else { + (void) _STD _Copy_backward_unchecked( + _Old_unwrapped_where, _STD _Prev_iter(_Old_unwrapped_end), _Old_unwrapped_end); + *_Old_unwrapped_where = _STD as_const(_Obj._Get_value()); + } + } else { // at the end + _Emplace_back_internal(_STD forward<_Valty>(_Val)...); } + return begin() + static_cast(_Off); } @@ -1267,21 +1302,7 @@ public: } iterator insert(const_iterator _Where, const _Ty& _Val) { - size_type _Off = static_cast(_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(1 + _Off)); - } else { // closer to back, push to back then copy - push_back(_Val); - _STD rotate(begin() + static_cast(_Off), _Prev_iter(end()), end()); - } - - return begin() + static_cast(_Off); + return emplace(_Where, _Val); } iterator insert(const_iterator _Where, _CRT_GUARDOVERFLOW size_type _Count, const _Ty& _Val) { 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..a07448e26f 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 @@ -3,13 +3,14 @@ #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,190 @@ 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 ThrowingConstruction { + explicit ThrowingConstruction() = default; +}; + +class UniqueError : public std::exception {}; + +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 + 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; + +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() + 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}); + } + + 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{}); + 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; + + explicit ThrowingMovable(int n) noexcept : payload{n} {} + + ThrowingMovable& operator=(ThrowingMovable&& other) { + 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::difference_type; + + deque 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] == ThrowingMovable{42}); + } + { + auto it = d.emplace(d.end() - Diff{3}, 1729); + assert(it == d.end() - Diff{4}); + assert(d[d.size() - 4] == ThrowingMovable{1729}); + } + + 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() + 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(); +} From 2226fc5c45f1ba4aca5252f3fa7cf85161b72a11 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Wed, 13 Sep 2023 08:56:13 +0800 Subject: [PATCH 02/10] Remove unused parameter name --- tests/std/tests/Dev10_860421_deque_push_back_pop_front/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a07448e26f..4e523ff703 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 @@ -155,7 +155,7 @@ class ThrowingMovable { explicit ThrowingMovable(int n) noexcept : payload{n} {} - ThrowingMovable& operator=(ThrowingMovable&& other) { + ThrowingMovable& operator=(ThrowingMovable&&) { throw UniqueError{}; } ThrowingMovable& operator=(const ThrowingMovable&) noexcept = default; From 04c8e4d21fbb5b8590314b74e73aec7896064511 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 15 Sep 2023 01:18:58 +0800 Subject: [PATCH 03/10] Address @achabense's review comments --- stl/inc/deque | 53 +++++++------------ .../test.cpp | 11 ---- 2 files changed, 20 insertions(+), 44 deletions(-) diff --git a/stl/inc/deque b/stl/inc/deque index 0602896963..02d1572cc3 100644 --- a/stl/inc/deque +++ b/stl/inc/deque @@ -871,44 +871,31 @@ public: _STL_VERIFY(_Off <= _Mysize(), "deque emplace iterator outside range"); #endif // _ITERATOR_DEBUG_LEVEL == 2 - constexpr bool _Uses_move_assignment = - disjunction_v, negation>>; - _Orphan_all(); if (_Off == 0) { // at the beginning _Emplace_front_internal(_STD forward<_Valty>(_Val)...); - } else if (_Off <= _Mysize() / 2) { // closer to front, push to front then rotate - auto& _Al = _Getal(); - _Alloc_temporary2<_Alty> _Obj(_Al, _STD forward<_Valty>(_Val)...); // handle aliasing + } else if (_Off <= _Mysize() / 2) { // closer to front + _Alloc_temporary2<_Alty> _Obj(_Getal(), _STD forward<_Valty>(_Val)...); // handle aliasing - auto _Old_unwrapped_begin = _Unchecked_begin(); - auto _Old_unwrapped_where = _Old_unwrapped_begin + static_cast(_Off); + _Emplace_front_internal(_STD move(*_Unchecked_begin())); - _Emplace_front_internal(_STD move_if_noexcept(*_Old_unwrapped_begin)); - if constexpr (_Uses_move_assignment) { - _STD _Move_unchecked(_STD _Next_iter(_Old_unwrapped_begin), _Old_unwrapped_where, _Old_unwrapped_begin); - *_STD _Prev_iter(_Old_unwrapped_where) = _STD move(_Obj._Get_value()); - } else { - _STD _Copy_unchecked(_STD _Next_iter(_Old_unwrapped_begin), _Old_unwrapped_where, _Old_unwrapped_begin); - *_STD _Prev_iter(_Old_unwrapped_where) = _STD as_const(_Obj._Get_value()); - } - } else if (_Off != _Mysize()) { // closer to back, push to back then rotate - auto& _Al = _Getal(); - _Alloc_temporary2<_Alty> _Obj(_Al, _STD forward<_Valty>(_Val)...); // handle aliasing - - auto _Old_unwrapped_end = _Unchecked_end(); - auto _Old_unwrapped_where = _Unchecked_begin() + static_cast(_Off); - - _Emplace_back_internal(_STD move_if_noexcept(*_STD _Prev_iter(_Old_unwrapped_end))); - if constexpr (_Uses_move_assignment) { - (void) _STD _Move_backward_unchecked( - _Old_unwrapped_where, _STD _Prev_iter(_Old_unwrapped_end), _Old_unwrapped_end); - *_Old_unwrapped_where = _STD move(_Obj._Get_value()); - } else { - (void) _STD _Copy_backward_unchecked( - _Old_unwrapped_where, _STD _Prev_iter(_Old_unwrapped_end), _Old_unwrapped_end); - *_Old_unwrapped_where = _STD as_const(_Obj._Get_value()); - } + 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 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(_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)...); } 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 4e523ff703..cad1cb07a9 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 @@ -184,17 +184,6 @@ void test_exception_safety_for_throwing_movable() { d.emplace_back(i); } - { - auto it = d.emplace(d.begin() + Diff{3}, 42); - assert(it == d.begin() + Diff{3}); - assert(d[3] == ThrowingMovable{42}); - } - { - auto it = d.emplace(d.end() - Diff{3}, 1729); - assert(it == d.end() - Diff{4}); - assert(d[d.size() - 4] == ThrowingMovable{1729}); - } - const auto d_orig = d; try { d.emplace_front(ThrowingMovable{}); From 2c9138392317cb3ce33c9e1ae84621b0b0695d16 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 18 Sep 2023 13:25:48 -0700 Subject: [PATCH 04/10] Cleanup `_HAS_CXX20` comments. --- .../tests/Dev10_860421_deque_push_back_pop_front/test.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 cad1cb07a9..82f28e5004 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 @@ -76,7 +76,7 @@ class NonswappableMovable { #if _HAS_CXX20 friend bool operator==(const NonswappableMovable&, const NonswappableMovable&) = default; -#else // ^^^ C++20 or later / C++17 or earlier +#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv friend bool operator==(const NonswappableMovable& lhs, const NonswappableMovable& rhs) noexcept { return lhs.payload == rhs.payload; } @@ -84,7 +84,7 @@ class NonswappableMovable { friend bool operator!=(const NonswappableMovable& lhs, const NonswappableMovable& rhs) noexcept { return lhs.payload != rhs.payload; } -#endif // ^^^ C++17 or earlier ^^^ +#endif // ^^^ !_HAS_CXX20 ^^^ friend void swap(NonswappableMovable, NonswappableMovable) = delete; @@ -162,7 +162,7 @@ class ThrowingMovable { #if _HAS_CXX20 friend bool operator==(const ThrowingMovable&, const ThrowingMovable&) = default; -#else // ^^^ C++20 or later / C++17 or earlier +#else // ^^^ _HAS_CXX20 / !_HAS_CXX20 vvv friend bool operator==(const ThrowingMovable& lhs, const ThrowingMovable& rhs) noexcept { return lhs.payload == rhs.payload; } @@ -170,7 +170,7 @@ class ThrowingMovable { friend bool operator!=(const ThrowingMovable& lhs, const ThrowingMovable& rhs) noexcept { return lhs.payload != rhs.payload; } -#endif // ^^^ C++17 or earlier ^^^ +#endif // ^^^ !_HAS_CXX20 ^^^ private: int payload = -1; From 71d5858b4af8c3705130520ed5054995672d126a Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 18 Sep 2023 13:27:21 -0700 Subject: [PATCH 05/10] `UniqueError` can be an empty `struct` (with an explicit default ctor for safety). --- .../tests/Dev10_860421_deque_push_back_pop_front/test.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 82f28e5004..dc6b3bd080 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 @@ -3,7 +3,6 @@ #include #include -#include #include #include #include @@ -55,7 +54,9 @@ struct ThrowingConstruction { explicit ThrowingConstruction() = default; }; -class UniqueError : public std::exception {}; +struct UniqueError { + explicit UniqueError() = default; +}; class NonswappableMovable { public: From 50e444b560c1d469631d7cf824333fe3466402b3 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 18 Sep 2023 13:28:16 -0700 Subject: [PATCH 06/10] Add `Tag` for naming clarity. --- .../test.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) 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 dc6b3bd080..2c82a501cf 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 @@ -50,8 +50,8 @@ void test_391805() { // - support for single-element insertion of non-swappable type, and // - exception safety for single-element insertion. -struct ThrowingConstruction { - explicit ThrowingConstruction() = default; +struct ThrowingConstructionTag { + explicit ThrowingConstructionTag() = default; }; struct UniqueError { @@ -65,7 +65,7 @@ class NonswappableMovable { NonswappableMovable(const NonswappableMovable&) = default; explicit NonswappableMovable(int n) noexcept : payload{n} {} - explicit NonswappableMovable(ThrowingConstruction) { + explicit NonswappableMovable(ThrowingConstructionTag) { throw UniqueError{}; } @@ -118,28 +118,28 @@ void test_exception_safety_for_nonswappable_movable() { const auto d_orig = d; try { - d.emplace_front(ThrowingConstruction{}); + d.emplace_front(ThrowingConstructionTag{}); assert(false); } catch (const UniqueError&) { } assert(d == d_orig); try { - d.emplace_back(ThrowingConstruction{}); + d.emplace_back(ThrowingConstructionTag{}); assert(false); } catch (const UniqueError&) { } assert(d == d_orig); try { - d.emplace(d.begin() + Diff{2}, ThrowingConstruction{}); + d.emplace(d.begin() + Diff{2}, ThrowingConstructionTag{}); assert(false); } catch (const UniqueError&) { } assert(d == d_orig); try { - d.emplace(d.end() - Diff{2}, ThrowingConstruction{}); + d.emplace(d.end() - Diff{2}, ThrowingConstructionTag{}); assert(false); } catch (const UniqueError&) { } From be3871c0ed2cc9549f840d9ad673920db1489b2f Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 18 Sep 2023 13:29:03 -0700 Subject: [PATCH 07/10] `swap` should take modifiable lvalue references. --- tests/std/tests/Dev10_860421_deque_push_back_pop_front/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 2c82a501cf..bbd91731de 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 @@ -87,7 +87,7 @@ class NonswappableMovable { } #endif // ^^^ !_HAS_CXX20 ^^^ - friend void swap(NonswappableMovable, NonswappableMovable) = delete; + friend void swap(NonswappableMovable&, NonswappableMovable&) = delete; private: int payload = -1; From 09075488a6e290db3ccd9b8575073f160e74777c Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 18 Sep 2023 13:29:47 -0700 Subject: [PATCH 08/10] `noexcept = default;` => `= default;` --- .../std/tests/Dev10_860421_deque_push_back_pop_front/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 bbd91731de..ff0f142786 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 @@ -152,14 +152,14 @@ class ThrowingMovable { ThrowingMovable(ThrowingMovable&&) { throw UniqueError{}; } - ThrowingMovable(const ThrowingMovable&) noexcept = default; + ThrowingMovable(const ThrowingMovable&) = default; explicit ThrowingMovable(int n) noexcept : payload{n} {} ThrowingMovable& operator=(ThrowingMovable&&) { throw UniqueError{}; } - ThrowingMovable& operator=(const ThrowingMovable&) noexcept = default; + ThrowingMovable& operator=(const ThrowingMovable&) = default; #if _HAS_CXX20 friend bool operator==(const ThrowingMovable&, const ThrowingMovable&) = default; From f764cfb79a90811f86e71b6c3950cd8757125d29 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 18 Sep 2023 13:35:00 -0700 Subject: [PATCH 09/10] Rearrange control flow to extract `_Alloc_temporary2<_Alty> _Obj`. --- stl/inc/deque | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/stl/inc/deque b/stl/inc/deque index 02d1572cc3..69082e0c07 100644 --- a/stl/inc/deque +++ b/stl/inc/deque @@ -874,30 +874,30 @@ public: _Orphan_all(); if (_Off == 0) { // at the beginning _Emplace_front_internal(_STD forward<_Valty>(_Val)...); - } else if (_Off <= _Mysize() / 2) { // closer to front + } 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 - _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); + if (_Off <= _Mysize() / 2) { // closer to front + _Emplace_front_internal(_STD move(*_Unchecked_begin())); - 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 + 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); - _Emplace_back_internal(_STD move(*_STD _Prev_iter(_Unchecked_end()))); + 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); + 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()); - } else { // at the end - _Emplace_back_internal(_STD forward<_Valty>(_Val)...); + _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); From 53a97fd2f702b21963516194b684094b461008d3 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Mon, 18 Sep 2023 14:43:15 -0700 Subject: [PATCH 10/10] Test emplace() with begin() and end(), and assert the full contents. --- .../test.cpp | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) 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 ff0f142786..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,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +#include #include #include #include @@ -105,6 +106,11 @@ void test_exception_safety_for_nonswappable_movable() { 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}); @@ -115,6 +121,16 @@ void test_exception_safety_for_nonswappable_movable() { 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 { @@ -131,6 +147,13 @@ void test_exception_safety_for_nonswappable_movable() { } 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); @@ -144,6 +167,13 @@ void test_exception_safety_for_nonswappable_movable() { } catch (const UniqueError&) { } assert(d == d_orig); + + try { + d.emplace(d.end(), ThrowingConstructionTag{}); + assert(false); + } catch (const UniqueError&) { + } + assert(d == d_orig); } class ThrowingMovable { @@ -200,6 +230,13 @@ void test_exception_safety_for_throwing_movable() { } 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); @@ -213,6 +250,13 @@ void test_exception_safety_for_throwing_movable() { } catch (const UniqueError&) { } assert(d == d_orig); + + try { + d.emplace(d.end(), ThrowingMovable{}); + assert(false); + } catch (const UniqueError&) { + } + assert(d == d_orig); } int main() {