diff --git a/stl/inc/flat_set b/stl/inc/flat_set index a8c5cd92748..3503f01d9f9 100644 --- a/stl/inc/flat_set +++ b/stl/inc/flat_set @@ -67,12 +67,12 @@ public: _Base_flat_set() : _Mycont(), _Mycomp() {} - // TRANSITION, "_Mycomp" may need to be copied, even in move construction / assignment. template <_Allocator_for _Alloc> _Base_flat_set(const _Deriv& _Set, const _Alloc& _Al) : _Mycont(_Set._Mycont, _Al), _Mycomp(_Set._Mycomp) {} template <_Allocator_for _Alloc> _Base_flat_set(_Deriv&& _Set, const _Alloc& _Al) - : _Mycont(_STD move(_Set._Mycont), _Al), _Mycomp(_STD move(_Set._Mycomp)) {} + : _Mycont(_STD move(_Set).extract(), _Al), _Mycomp(_Set._Mycomp) // intentionally copy comparator, see LWG-2227 + {} explicit _Base_flat_set(container_type _Cont, const key_compare& _Comp = key_compare()) : _Mycont(_STD move(_Cont)), _Mycomp(_Comp) { @@ -153,6 +153,32 @@ public: _Base_flat_set(_Tsorted _Tsort, initializer_list<_Kty> _Ilist, const _Alloc& _Al) : _Base_flat_set(_Tsort, container_type(_Ilist, _Al)) {} + _Base_flat_set(const _Base_flat_set&) = default; + _Base_flat_set(_Base_flat_set&& _Other) noexcept( + is_nothrow_move_constructible_v&& is_nothrow_copy_constructible_v) // strengthened + : _Mycont(_STD move(_Other).extract()), _Mycomp(_Other._Mycomp) // intentionally copy comparator, see LWG-2227 + {} + + _Base_flat_set& operator=(const _Base_flat_set& _Other) { + _Clear_guard<_Base_flat_set> _Guard{this}; + _Mycont = _Other._Mycont; + _Mycomp = _Other._Mycomp; + _Guard._Target = nullptr; + return *this; + } + _Base_flat_set& operator=(_Base_flat_set&& _Other) noexcept( + is_nothrow_move_assignable_v&& is_nothrow_copy_assignable_v) // strengthened + { + if (this != _STD addressof(_Other)) { + _Clear_guard<_Base_flat_set> _Guard{this}; + _Clear_guard<_Base_flat_set> _Always_clear{_STD addressof(_Other)}; + _Mycont = _STD move(_Other._Mycont); + _Mycomp = _Other._Mycomp; // intentionally copy comparator, see LWG-2227 + _Guard._Target = nullptr; + } + return *this; + } + _Deriv& operator=(initializer_list<_Kty> _Ilist) { _Clear_guard<_Base_flat_set> _Guard{this}; _Mycont.assign(_Ilist); @@ -288,7 +314,7 @@ public: _NODISCARD container_type extract() && noexcept( is_nothrow_move_constructible_v) /* strengthened */ { // always clears the container (N4950 [flat.set.modifiers]/14 and [flat.multiset.modifiers]/10) - _Clear_guard<_Base_flat_set> _Guard{this}; + _Clear_guard<_Base_flat_set> _Always_clear{this}; return _STD move(_Mycont); } void replace(container_type&& _Cont) { @@ -711,7 +737,12 @@ private: public: using _Mybase::_Mybase; + flat_set(const flat_set&) = default; + flat_set(flat_set&&) = default; + using _Mybase::operator=; + flat_set& operator=(const flat_set&) = default; + flat_set& operator=(flat_set&&) = default; }; _EXPORT_STD template , class _Container = vector<_Kty>> @@ -722,7 +753,12 @@ private: public: using _Mybase::_Mybase; + flat_multiset(const flat_multiset&) = default; + flat_multiset(flat_multiset&&) = default; + using _Mybase::operator=; + flat_multiset& operator=(const flat_multiset&) = default; + flat_multiset& operator=(flat_multiset&&) = default; }; _EXPORT_STD template diff --git a/tests/std/tests/P1222R4_flat_set/test.cpp b/tests/std/tests/P1222R4_flat_set/test.cpp index e0dad813302..e045438b952 100644 --- a/tests/std/tests/P1222R4_flat_set/test.cpp +++ b/tests/std/tests/P1222R4_flat_set/test.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -69,10 +70,14 @@ void assert_reversible_container_requirements(const T& s) { } template -void assert_all_requirements_and_equals(const T& s, const initializer_list& il) { +void assert_all_requirements(const T& s) { assert_container_requirements(s); assert_reversible_container_requirements(s); + // FIXME, in GH-4084 + // assert_noexcept_requirements(s); + // assert_noexcept_requirements(const_cast(s)); + auto val_comp = s.value_comp(); auto begin_it = s.cbegin(); auto end_it = s.cend(); @@ -85,6 +90,11 @@ void assert_all_requirements_and_equals(const T& s, const initializer_list +void assert_all_requirements_and_equals(const T& s, const initializer_list& il) { + assert_all_requirements(s); if (!std::equal(s.begin(), s.end(), il.begin(), il.end())) { cout << "Expected: {"; @@ -467,6 +477,151 @@ void test_extract_2() { assert_all_requirements_and_equals(fs, {}); // assert empty } +void test_invariant_robustness() { + static int copy_limit = 2; + constexpr int unlimited = INT_MAX; + + struct odd_key { + static void countdown() { + if (copy_limit == unlimited) { + return; + } + + if (--copy_limit < 0) { + throw 0; // will be caught by "catch (...)". + } + } + + int key; + + odd_key(int k = 0) : key(k) {} + + bool operator==(const odd_key&) const = default; + + odd_key(const odd_key& other) { + countdown(); + key = other.key; + } + + odd_key(odd_key&& other) { + countdown(); + key = exchange(other.key, 0); + } + + odd_key& operator=(const odd_key& other) { + countdown(); + key = other.key; + return *this; + } + + odd_key& operator=(odd_key&& other) { + countdown(); + key = exchange(other.key, 0); + return *this; + } + }; + + class odd_container : public vector { + private: + using base = vector; + + public: + using base::base; + odd_container(const odd_container&) = default; + + // this copy-assignment cannot provide strong-guarantee for `this`: + odd_container& operator=(const odd_container& other) { + resize(other.size()); + std::copy(other.begin(), other.end(), begin()); + return *this; + } + + // this move-ctor cannot provide strong-guarantee for `other`, and even successful, will leave elements of + // `other` in moved-from state: + odd_container(odd_container&& other) { + reserve(other.size()); + for (auto& e : other) { + push_back(std::move(e)); + } + } + + // this move-assignment cannot provide strong-guarantee for `this` and `other`, and even successful, will leave + // elements of `other` in moved-from state: + odd_container& operator=(odd_container&& other) { + resize(other.size()); + std::move(other.begin(), other.end(), begin()); + return *this; + } + }; + + using SetT = flat_set; + + // copy-assignment + { + copy_limit = unlimited; + SetT fs1{0, 1, 2, 3, 4}; + SetT fs2{5, 6, 7, 8, 9}; + + assert(ranges::equal(fs1, vector{0, 1, 2, 3, 4}, {}, &odd_key::key)); + assert(ranges::equal(fs2, vector{5, 6, 7, 8, 9}, {}, &odd_key::key)); + + bool caught = false; + try { + copy_limit = 2; + fs1 = fs2; // will throw after copying 2 odd_key. + } catch (...) { + copy_limit = unlimited; + assert_all_requirements(fs1); + caught = true; + } + assert(caught); + } + // move-ctor + { + copy_limit = unlimited; + SetT fs1{0, 1, 2, 3, 4}; + SetT fs2{std::move(fs1)}; + + assert_all_requirements(fs1); + assert(ranges::equal(fs2, vector{0, 1, 2, 3, 4}, {}, &odd_key::key)); + + bool caught = false; + try { + copy_limit = 2; + SetT fs3{std::move(fs2)}; // will throw after moving 2 odd_key. + } catch (...) { + copy_limit = unlimited; + assert_all_requirements(fs2); + caught = true; + } + assert(caught); + } + // move-assignment + { + copy_limit = unlimited; + SetT fs1{0, 1, 2, 3, 4}; + SetT fs2; + SetT fs3{5, 6, 7, 8, 9}; + fs2 = std::move(fs1); + + assert_all_requirements(fs1); + assert(ranges::equal(fs2, vector{0, 1, 2, 3, 4}, {}, &odd_key::key)); + assert(ranges::equal(fs3, vector{5, 6, 7, 8, 9}, {}, &odd_key::key)); + + bool caught = false; + try { + copy_limit = 2; + fs2 = std::move(fs3); // will throw after moving 2 odd_key. + } catch (...) { + copy_limit = unlimited; + assert_all_requirements(fs2); + assert_all_requirements(fs3); + caught = true; + } + assert(caught); + } +} + // TRANSITION, too simple void test_erase_1() { flat_set fs{1}; @@ -544,6 +699,7 @@ int main() { test_erase_1(); test_erase_2(); + test_invariant_robustness(); test_erase_if>(); test_erase_if>();