<flat_set>: the copy/move functions should be user-defined

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<container_type> _Alloc>
     _Base_flat_set(const _Deriv& _Set, const _Alloc& _Al) : _Mycont(_Set._Mycont, _Al), _Mycomp(_Set._Mycomp) {}
     template <_Allocator_for<container_type> _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<container_type>&& is_nothrow_copy_constructible_v<key_compare>) // 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<container_type>&& is_nothrow_copy_assignable_v<key_compare>) // 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<container_type>) /* 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 _Kty, class _Keylt = less<_Kty>, 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 <class _Kty, class _Keylt, class _Container, class _Pred>

tests/std/tests/P1222R4_flat_set/test.cpp

@@ -3,6 +3,7 @@
 
 #include <algorithm>
 #include <cassert>
+#include <climits>
 #include <deque>
 #include <flat_set>
 #include <functional>
@@ -69,10 +70,14 @@ void assert_reversible_container_requirements(const T& s) {
 }
 
 template <class T>
-void assert_all_requirements_and_equals(const T& s, const initializer_list<typename T::value_type>& 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<T&>(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<typen
             }
         }
     }
+}
+
+template <class T>
+void assert_all_requirements_and_equals(const T& s, const initializer_list<typename T::value_type>& 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<odd_key> {
+    private:
+        using base = vector<odd_key>;
+
+    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<odd_key, key_comparer, odd_container>;
+
+    // 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<int> fs{1};
@@ -544,6 +699,7 @@ int main() {
 
     test_erase_1();
     test_erase_2();
+    test_invariant_robustness();
 
     test_erase_if<flat_set<int>>();
     test_erase_if<flat_multiset<int>>();