From 6251a9e4f2cab704ce19221354eed0764e4385a6 Mon Sep 17 00:00:00 2001
From: achabense <60953653+achabense@users.noreply.github.com>
Date: Tue, 10 Oct 2023 23:22:59 +0800
Subject: [PATCH 01/13] add test for set operations and observers

---
 tests/std/tests/P1222R4_flat_set/test.cpp | 132 +++++++++++++++++++++-
 1 file changed, 126 insertions(+), 6 deletions(-)

diff --git a/tests/std/tests/P1222R4_flat_set/test.cpp b/tests/std/tests/P1222R4_flat_set/test.cpp
index e0dad813302..98139fd07af 100644
--- a/tests/std/tests/P1222R4_flat_set/test.cpp
+++ b/tests/std/tests/P1222R4_flat_set/test.cpp
@@ -9,6 +9,7 @@
 #include <iostream>
 #include <memory>
 #include <random>
+#include <tuple>
 #include <utility>
 #include <vector>
 
@@ -505,13 +506,126 @@ void test_erase_if() {
     assert(ranges::equal(fs, erased_result));
 }
 
-// TRANSITION, too simple
-void test_count() {
-    flat_set<int> fs{2};
+template <template <class...> class Set>
+void test_observers() {
+    struct lt_with_state {
+        int state = 0;
+        bool operator()(int l, int r) const {
+            return l < r;
+        }
+    };
+
+    using SetT = Set<int, lt_with_state>;
+
+    SetT fs;
+    static_assert(is_same_v<typename SetT::key_compare, typename SetT::value_compare>);
+    static_assert(is_same_v<decltype(fs.key_comp()), typename SetT::key_compare>);
+    static_assert(is_same_v<decltype(fs.value_comp()), typename SetT::value_compare>);
+    assert(fs.key_comp().state == 0);
+    assert(fs.value_comp().state == 0);
+
+    SetT fs2(lt_with_state{2});
+    assert(fs2.key_comp().state == 2);
+    assert(fs2.value_comp().state == 2);
+}
+
+template <template <class...> class Set>
+void test_set_operations() {
+    using SetT           = Set<int>;
+    using iterator       = SetT::iterator;
+    using const_iterator = SetT::const_iterator;
+
+    SetT fs{3, 2, 11, 11, 3, 8, 11, 20};
+
+    static_assert(is_same_v<decltype(fs.find(0)), iterator>);
+    static_assert(is_same_v<decltype(as_const(fs).find(0)), const_iterator>);
+
+    static_assert(is_same_v<decltype(fs.count(0)), typename SetT::size_type>);
+    static_assert(is_same_v<decltype(fs.contains(0)), bool>);
+
+    static_assert(is_same_v<decltype(fs.lower_bound(0)), iterator>);
+    static_assert(is_same_v<decltype(as_const(fs).lower_bound(0)), const_iterator>);
+    static_assert(is_same_v<decltype(fs.upper_bound(0)), iterator>);
+    static_assert(is_same_v<decltype(as_const(fs).upper_bound(0)), const_iterator>);
+
+    static_assert(is_same_v<decltype(fs.equal_range(0)), pair<iterator, iterator>>);
+    static_assert(is_same_v<decltype(as_const(fs).equal_range(0)), pair<const_iterator, const_iterator>>);
+
+    if constexpr (!_Is_specialization_v<SetT, flat_multiset>) {
+        // flat_set:
+        assert_all_requirements_and_equals(fs, {2, 3, 8, 11, 20});
+
+        assert(fs.find(3) != fs.end());
+        assert(fs.find(4) == fs.end());
+
+        assert(fs.count(1) == 0);
+        assert(fs.count(11) == 1);
+        assert(fs.contains(8));
+        assert(!fs.contains(12));
+
+        assert(fs.lower_bound(-1) == fs.begin());
+        assert(fs.lower_bound(3) == fs.find(3));
+        assert(fs.lower_bound(19) == fs.find(20));
+        assert(fs.lower_bound(20) + 1 == fs.end());
+
+        assert(fs.upper_bound(-1) == fs.begin());
+        assert(fs.upper_bound(20) == fs.end());
+        assert(fs.lower_bound(2) + 2 == fs.upper_bound(3));
+        assert(fs.upper_bound(8) == fs.find(11));
+
+        auto [first, last] = fs.equal_range(3);
+        assert(first + 1 == last);
+        tie(first, last) = fs.equal_range(12);
+        assert(first == last);
+    } else {
+        // flat_multiset:
+        assert_all_requirements_and_equals(fs, {2, 3, 3, 8, 11, 11, 11, 20});
+
+        assert(fs.find(3) != fs.end());
+        assert(fs.find(4) == fs.end());
+
     assert(fs.count(1) == 0);
+        assert(fs.count(11) == 3);
+        assert(fs.contains(8));
+        assert(!fs.contains(12));
+
+        assert(fs.lower_bound(-1) == fs.begin());
+        assert(fs.lower_bound(3) == fs.find(3));
+        assert(fs.lower_bound(19) == fs.find(20));
+        assert(fs.lower_bound(11) + 4 == fs.end());
+
+        assert(fs.upper_bound(-1) == fs.begin());
+        assert(fs.upper_bound(20) == fs.end());
+        assert(fs.lower_bound(3) + 6 == fs.upper_bound(11));
+        assert(fs.upper_bound(11) == fs.find(20));
+
+        auto [first, last] = fs.equal_range(3);
+        assert(first + 2 == last);
+        tie(first, last) = fs.equal_range(12);
+        assert(first == last);
+    }
+}
 
-    flat_multiset<int> fs2{10, 20, 20, 30};
-    assert(fs2.count(20) == 2);
+template <template <class...> class Set>
+void test_set_operations_transparent() {
+    struct shouldnt_convert {
+        int key;
+        /* implicit */ [[noreturn]] operator int() const {
+            abort();
+        }
+    };
+
+    Set<int, key_comparer> fs{0, 3, 5};
+    assert_all_requirements_and_equals(fs, {0, 3, 5});
+
+    assert(fs.find(shouldnt_convert{0}) != fs.end());
+    assert(fs.count(shouldnt_convert{3}) == 1);
+    assert(!fs.contains(shouldnt_convert{1}));
+    assert(fs.lower_bound(shouldnt_convert{-1}) == fs.begin());
+    assert(fs.lower_bound(shouldnt_convert{8}) == fs.end());
+    assert(fs.upper_bound(shouldnt_convert{2}) == fs.find(3));
+    auto [first, last] = fs.equal_range(shouldnt_convert{5});
+    assert(first != last);
 }
 
 int main() {
@@ -548,5 +662,11 @@ int main() {
     test_erase_if<flat_set<int>>();
     test_erase_if<flat_multiset<int>>();
 
-    test_count();
+    test_observers<flat_set>();
+    test_observers<flat_multiset>();
+
+    test_set_operations<flat_set>();
+    test_set_operations<flat_multiset>();
+    test_set_operations_transparent<flat_set>();
+    test_set_operations_transparent<flat_multiset>();
 }

From 7d0255ffc8ba06359e4b8d1a63abee5810ec63c0 Mon Sep 17 00:00:00 2001
From: achabense <60953653+achabense@users.noreply.github.com>
Date: Tue, 10 Oct 2023 23:24:36 +0800
Subject: [PATCH 02/13] add test for noexcept; enhance test for reversibility

---
 tests/std/tests/P1222R4_flat_set/test.cpp | 56 ++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/tests/std/tests/P1222R4_flat_set/test.cpp b/tests/std/tests/P1222R4_flat_set/test.cpp
index 98139fd07af..fae2ac559eb 100644
--- a/tests/std/tests/P1222R4_flat_set/test.cpp
+++ b/tests/std/tests/P1222R4_flat_set/test.cpp
@@ -64,16 +64,43 @@ void assert_reversible_container_requirements(const T& s) {
     static_assert(is_same_v<reverse_iterator<typename T::const_iterator>, typename T::const_reverse_iterator>);
     static_assert(is_same_v<decltype(T{}.rbegin()), typename T::reverse_iterator>);
     static_assert(is_same_v<decltype(T{}.rend()), typename T::reverse_iterator>);
+    static_assert(is_same_v<decltype(s.rbegin()), typename T::const_reverse_iterator>);
+    static_assert(is_same_v<decltype(s.rend()), typename T::const_reverse_iterator>);
     static_assert(is_same_v<decltype(s.crbegin()), typename T::const_reverse_iterator>);
     static_assert(is_same_v<decltype(s.crend()), typename T::const_reverse_iterator>);
     static_assert(is_convertible_v<typename T::reverse_iterator, typename T::const_reverse_iterator>);
 }
 
+template <class T>
+void assert_noexcept_requirements(T& s) {
+    static_assert(noexcept(s.begin()));
+    static_assert(noexcept(s.end()));
+    static_assert(noexcept(s.cbegin()));
+    static_assert(noexcept(s.cend()));
+    static_assert(noexcept(s.rbegin()));
+    static_assert(noexcept(s.rend()));
+    static_assert(noexcept(s.crbegin()));
+    static_assert(noexcept(s.crend()));
+
+    static_assert(noexcept(s.empty()));
+    static_assert(noexcept(s.size()));
+    static_assert(noexcept(s.max_size()));
+
+    if constexpr (!is_const_v<T>) {
+        static_assert(noexcept(s.swap(s)));
+        static_assert(noexcept(ranges::swap(s, s))); // using ADL-swap
+        static_assert(noexcept(s.clear()));
+    }
+}
+
 template <class T>
 void assert_all_requirements_and_equals(const T& s, const initializer_list<typename T::value_type>& il) {
     assert_container_requirements(s);
     assert_reversible_container_requirements(s);
 
+    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();
@@ -132,6 +159,30 @@ void test_constructors() {
     assert_all_requirements_and_equals(flat_multiset<int>(std::move(b), allocator<int>{}), {-1, 1, 2, 7, 7, 7, 100});
 }
 
+template <template <class...> class Set>
+void test_always_reversible() {
+    // Test that flat_meow is unconditionally reversible.
+    class not_reversible : public vector<int> {
+    private:
+        using base = vector<int>;
+
+        using base::const_reverse_iterator;
+        using base::crbegin;
+        using base::crend;
+        using base::rbegin;
+        using base::rend;
+        using base::reverse_iterator;
+
+    public:
+        using base::base;
+    };
+
+    Set<int, std::greater<int>, not_reversible> fs({1, 2, 3});
+    assert_all_requirements_and_equals(fs, {3, 2, 1});
+    assert(fs.rbegin() + 3 == fs.rend());
+    assert(fs.crend() - fs.crbegin() == 3);
+}
+
 template <class C>
 void test_insert_1() {
     using lt = std::less<int>;
@@ -584,7 +635,7 @@ void test_set_operations() {
         assert(fs.find(3) != fs.end());
         assert(fs.find(4) == fs.end());
 
-    assert(fs.count(1) == 0);
+        assert(fs.count(1) == 0);
         assert(fs.count(11) == 3);
         assert(fs.contains(8));
         assert(!fs.contains(12));
@@ -641,6 +692,9 @@ int main() {
     test_constructors<vector<int>>();
     test_constructors<deque<int>>();
 
+    test_always_reversible<flat_set>();
+    test_always_reversible<flat_multiset>();
+
     test_insert_1<vector<int>>();
     test_insert_1<deque<int>>();
     test_insert_2<vector<int>>();

From 6ea761014d11c482037a1fd5a5e0c3d6109037e6 Mon Sep 17 00:00:00 2001
From: achabense <60953653+achabense@users.noreply.github.com>
Date: Wed, 11 Oct 2023 00:00:05 +0800
Subject: [PATCH 03/13] test that flat_multiset should insert before
 upper_bound

---
 tests/std/tests/P1222R4_flat_set/test.cpp | 30 +++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tests/std/tests/P1222R4_flat_set/test.cpp b/tests/std/tests/P1222R4_flat_set/test.cpp
index fae2ac559eb..0ec56f278be 100644
--- a/tests/std/tests/P1222R4_flat_set/test.cpp
+++ b/tests/std/tests/P1222R4_flat_set/test.cpp
@@ -394,6 +394,35 @@ void test_insert_using_invalid_hint() {
     }
 }
 
+void test_insert_upper_bound() {
+    // For flat_multiset's single-element insertion, the key should be inserted before the upper_bound.
+    struct test_position {
+        int key;
+        int extra;
+        bool operator==(const test_position& other) const {
+            return key == other.key && extra == other.extra;
+        }
+    };
+
+    mt19937 eng(24);
+    uniform_int_distribution<int> dist_seq(0, 20);
+
+    vector<test_position> seq(200);
+    for (int e = 0; auto& [key, extra] : seq) {
+        key   = dist_seq(eng);
+        extra = e++;
+    }
+
+    flat_multiset<test_position, key_comparer> fs;
+    for (const auto& val : seq) {
+        fs.insert(val);
+    }
+
+    // The result should be identical to as if doing stable_sort on seq.
+    ranges::stable_sort(seq, key_comparer{});
+    assert(ranges::equal(fs, seq));
+}
+
 template <class T>
 void test_spaceship_operator() {
     static constexpr bool multi  = _Is_specialization_v<T, flat_multiset>;
@@ -701,6 +730,7 @@ int main() {
     test_insert_2<deque<int>>();
     test_insert_transparent();
     test_insert_using_invalid_hint();
+    test_insert_upper_bound();
 
     test_comparer_application();
     test_non_static_comparer();

From 69efd7ee235f05809f91dba14d2390d9ce27797b Mon Sep 17 00:00:00 2001
From: achabense <60953653+achabense@users.noreply.github.com>
Date: Tue, 10 Oct 2023 20:18:00 +0800
Subject: [PATCH 04/13] A. relax the precondition, to make
 `insert([hint,]auto&&)` standard-conformant (though I believe the standard is
 underspecifying the precondition)

---
 stl/inc/flat_set | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/stl/inc/flat_set b/stl/inc/flat_set
index a8c5cd92748..f2e1a3a9279 100644
--- a/stl/inc/flat_set
+++ b/stl/inc/flat_set
@@ -67,7 +67,7 @@ public:
 
     _Base_flat_set() : _Mycont(), _Mycomp() {}
 
-    // TRANSITION, "_Mycomp" may need to be copied, even in move construction / assignment.
+    // FIXME, "_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>
@@ -101,7 +101,7 @@ public:
     template <_Allocator_for<container_type> _Alloc>
     explicit _Base_flat_set(const _Alloc& _Al) : _Mycont(_Al), _Mycomp() {}
 
-    // TRANSITION, an allocator-aware container may not support "C(_First, _Last, _Al)".
+    // FIXME, an allocator-aware container may not support "C(_First, _Last, _Al)".
     template <input_iterator _Iter>
     _Base_flat_set(_Iter _First, _Iter _Last, const key_compare& _Comp = key_compare())
         : _Base_flat_set(container_type(_First, _Last), _Comp) {}
@@ -111,7 +111,7 @@ public:
     template <input_iterator _Iter, _Allocator_for<container_type> _Alloc>
     _Base_flat_set(_Iter _First, _Iter _Last, const _Alloc& _Al) : _Base_flat_set(container_type(_First, _Last, _Al)) {}
 
-    // TRANSITION, an allocator-aware container may not support "C(from_range, _STD forward<_Rng>(_Range), _Al)".
+    // FIXME, an allocator-aware container may not support "C(from_range, _STD forward<_Rng>(_Range), _Al)".
     template <_Container_compatible_range<_Kty> _Rng>
     _Base_flat_set(from_range_t, _Rng&& _Range)
         : _Base_flat_set(container_type(from_range, _STD forward<_Rng>(_Range))) {}
@@ -135,7 +135,7 @@ public:
     _Base_flat_set(_Tsorted _Tsort, _Iter _First, _Iter _Last, const _Alloc& _Al)
         : _Base_flat_set(_Tsort, container_type(_First, _Last, _Al)) {}
 
-    // TRANSITION, an allocator-aware container may not support "C(_Ilist, _Al)".
+    // FIXME, an allocator-aware container may not support "C(_Ilist, _Al)".
     _Base_flat_set(initializer_list<_Kty> _Ilist, const key_compare& _Comp = key_compare())
         : _Base_flat_set(container_type(_Ilist), _Comp) {}
     template <_Allocator_for<container_type> _Alloc>
@@ -213,7 +213,7 @@ public:
     }
 
     // modifiers
-    // TRANSITION, the "insert" and "erase" methods may not be able to restore the invariant, if the underlying
+    // FIXME, the "insert" and "erase" methods may not be able to restore the invariant, if the underlying
     // container is unable to provide strong guarantee for "erase" and "insert" methods.
     template <class... _Args>
     auto emplace(_Args&&... _Vals) {
@@ -497,6 +497,7 @@ private:
     pair<iterator, bool> _Emplace(_Ty&& _Val) {
         const iterator _Where = lower_bound(_Val);
         if (_Where != end() && !_Compare(_Val, *_Where)) {
+            // the set already contains _Val.
             return pair{_Where, false};
         }
 
@@ -504,12 +505,12 @@ private:
             return pair{_Mycont.emplace(_Where, _STD forward<_Ty>(_Val)), true};
         } else {
             // heterogeneous insertion
-            // TRANSITION, the requirements may be stricter than needed by the standard.
             _STL_INTERNAL_STATIC_ASSERT(_Keylt_transparent && is_constructible_v<_Kty, _Ty>);
             _Kty _Keyval(_STD forward<_Ty>(_Val));
-            _STL_ASSERT(_Can_insert(_Where, _Keyval), "The conversion from the heterogeneous key to key_type should "
-                                                      "be consistent with the heterogeneous lookup!");
-            return pair{_Mycont.emplace(_Where, _STD move(_Keyval)), true};
+            // precondition: `find(_Val) == find(_Keyval)` (per N4958 [flat.set.modifiers]/2)
+            // the precondition cannot guarantee `_Can_insert(_Where, _Keyval)` here.
+            _STL_ASSERT(!contains(_Keyval), "find(_Val) == end(), but find(_Keyval) != end()");
+            return pair{_Emplace_hint(_Where, _STD move(_Keyval)), true};
         }
     }
 
@@ -560,7 +561,7 @@ private:
         }
 
         if (_Where != _End && !_Compare(_Val, *_Where)) {
-            // convert const_iterator to iterator
+            // the set already contains _Val; return _Where as iterator type.
             return _Mycont.begin() + (_Where - _Begin);
         }
 
@@ -569,12 +570,12 @@ private:
             return _Mycont.emplace(_Where, _STD forward<_Ty>(_Val));
         } else {
             // heterogeneous insertion
-            // TRANSITION, the requirements may be stricter than needed by the standard.
             _STL_INTERNAL_STATIC_ASSERT(_Keylt_transparent && is_constructible_v<_Kty, _Ty>);
             _Kty _Keyval(_STD forward<_Ty>(_Val));
-            _STL_ASSERT(_Can_insert(_Where, _Keyval), "The conversion from the heterogeneous key to key_type should "
-                                                      "be consistent with the heterogeneous lookup!");
-            return _Mycont.emplace(_Where, _STD move(_Keyval));
+            // precondition: `find(_Val) == find(_Keyval)` (per N4958 [flat.set.modifiers]/2)
+            // the precondition cannot guarantee `_Can_insert(_Where, _Keyval)` here.
+            _STL_ASSERT(!contains(_Keyval), "find(_Val) == end(), but find(_Keyval) != end()");
+            return _Emplace_hint(_Where, _STD move(_Keyval));
         }
     }
 

From fe69fe0adf989e2259664cc5e2a5ea7cb78b33f8 Mon Sep 17 00:00:00 2001
From: achabense <60953653+achabense@users.noreply.github.com>
Date: Wed, 11 Oct 2023 01:29:21 +0800
Subject: [PATCH 05/13] B. copy the comparator (what's more, the default
 move-ctor/assignment may not be invariant-safe...)

---
 stl/inc/flat_set | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/stl/inc/flat_set b/stl/inc/flat_set
index f2e1a3a9279..087f6272c69 100644
--- a/stl/inc/flat_set
+++ b/stl/inc/flat_set
@@ -67,12 +67,12 @@ public:
 
     _Base_flat_set() : _Mycont(), _Mycomp() {}
 
-    // FIXME, "_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,27 @@ 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)
+        : _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) {
+        _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 +309,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) {
@@ -712,7 +733,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>>
@@ -723,7 +749,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>

From dacb4615c761aa3c25a89f1f7af7a7b6cc196b8f Mon Sep 17 00:00:00 2001
From: achabense <60953653+achabense@users.noreply.github.com>
Date: Wed, 11 Oct 2023 02:42:35 +0800
Subject: [PATCH 06/13] nitpick: consistently use `_NODISCARD` for internal
 methods.

---
 stl/inc/flat_set | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/stl/inc/flat_set b/stl/inc/flat_set
index 087f6272c69..ffdebd7f675 100644
--- a/stl/inc/flat_set
+++ b/stl/inc/flat_set
@@ -508,14 +508,14 @@ private:
 
     template <class _Ty>
         requires _Multi // flat_multiset
-    iterator _Emplace(_Ty&& _Val) {
+    _NODISCARD iterator _Emplace(_Ty&& _Val) {
         _STL_INTERNAL_STATIC_ASSERT(is_same_v<remove_cvref_t<_Ty>, _Kty>);
         return _Mycont.emplace(upper_bound(_Val), _STD forward<_Ty>(_Val));
     }
 
     template <class _Ty>
         requires (!_Multi) // flat_set
-    pair<iterator, bool> _Emplace(_Ty&& _Val) {
+    _NODISCARD pair<iterator, bool> _Emplace(_Ty&& _Val) {
         const iterator _Where = lower_bound(_Val);
         if (_Where != end() && !_Compare(_Val, *_Where)) {
             // the set already contains _Val.
@@ -537,7 +537,7 @@ private:
 
     template <class _Ty>
         requires _Multi // flat_multiset
-    iterator _Emplace_hint(const_iterator _Where, _Ty&& _Val) {
+    _NODISCARD iterator _Emplace_hint(const_iterator _Where, _Ty&& _Val) {
         _STL_INTERNAL_STATIC_ASSERT(is_same_v<remove_cvref_t<_Ty>, _Kty>);
 
         const const_iterator _Begin = cbegin();
@@ -563,7 +563,7 @@ private:
 
     template <class _Ty>
         requires (!_Multi) // flat_set
-    iterator _Emplace_hint(const_iterator _Where, _Ty&& _Val) {
+    _NODISCARD iterator _Emplace_hint(const_iterator _Where, _Ty&& _Val) {
         const const_iterator _Begin = cbegin();
         const const_iterator _End   = cend();
 
@@ -608,7 +608,7 @@ private:
     }
 
     template <class _Ty>
-    size_type _Erase(const _Ty& _Val) {
+    _NODISCARD size_type _Erase(const _Ty& _Val) {
         _STL_INTERNAL_STATIC_ASSERT(_Keylt_transparent || is_same_v<_Ty, _Kty>);
 
         if constexpr (!_Multi && is_same_v<_Ty, _Kty>) {

From 3ce33993993c35f26f0efb15d917a59d48e7594b Mon Sep 17 00:00:00 2001
From: achabense <60953653+achabense@users.noreply.github.com>
Date: Wed, 11 Oct 2023 13:37:41 +0800
Subject: [PATCH 07/13] add noexcept; add test coverage for `B. commit`

---
 stl/inc/flat_set                          |   7 +-
 tests/std/tests/P1222R4_flat_set/test.cpp | 133 +++++++++++++++++++++-
 2 files changed, 137 insertions(+), 3 deletions(-)

diff --git a/stl/inc/flat_set b/stl/inc/flat_set
index ffdebd7f675..b07beff49c4 100644
--- a/stl/inc/flat_set
+++ b/stl/inc/flat_set
@@ -154,7 +154,8 @@ public:
         : _Base_flat_set(_Tsort, container_type(_Ilist, _Al)) {}
 
     _Base_flat_set(const _Base_flat_set&) = default;
-    _Base_flat_set(_Base_flat_set&& _Other)
+    _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
     {}
 
@@ -165,7 +166,9 @@ public:
         _Guard._Target = nullptr;
         return *this;
     }
-    _Base_flat_set& operator=(_Base_flat_set&& _Other) {
+    _Base_flat_set& operator=(_Base_flat_set&& _Other) noexcept(
+        is_nothrow_move_assignable_v<container_type>&& is_nothrow_copy_assignable_v<key_compare>) // strengthened
+    {
         _Clear_guard<_Base_flat_set> _Guard{this};
         _Clear_guard<_Base_flat_set> _Always_clear{_STD addressof(_Other)};
         _Mycont        = _STD move(_Other._Mycont);
diff --git a/tests/std/tests/P1222R4_flat_set/test.cpp b/tests/std/tests/P1222R4_flat_set/test.cpp
index 0ec56f278be..3686ddb8c06 100644
--- a/tests/std/tests/P1222R4_flat_set/test.cpp
+++ b/tests/std/tests/P1222R4_flat_set/test.cpp
@@ -94,7 +94,7 @@ void assert_noexcept_requirements(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);
 
@@ -113,6 +113,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: {";
@@ -548,6 +553,131 @@ void test_extract_2() {
     assert_all_requirements_and_equals(fs, {}); // assert empty
 }
 
+void test_invariant_robustness() {
+    static int control_throw        = 0;
+    static constexpr int wont_throw = INT_MAX;
+
+    struct weird_elem {
+        static void countdown() {
+            if (control_throw == wont_throw) {
+                return;
+            }
+            if (--control_throw <= 0) {
+                throw 0; // will be caught by "catch (...)".
+            }
+        }
+
+        int key;
+
+        weird_elem(int k = 0) : key(k) {}
+        bool operator==(const weird_elem& other) const {
+            return key == other.key;
+        }
+
+        weird_elem(const weird_elem& other) : key(other.key) {
+            countdown();
+        }
+
+        weird_elem(weird_elem&& other) : key(exchange(other.key, 0)) {
+            countdown();
+        }
+
+        weird_elem& operator=(const weird_elem& other) {
+            countdown();
+            key = other.key;
+            return *this;
+        }
+
+        weird_elem& operator=(weird_elem&& other) {
+            countdown();
+            key = exchange(other.key, 0);
+            return *this;
+        }
+    };
+
+    class weird_container : public vector<weird_elem> {
+    private:
+        using base = vector<weird_elem>;
+
+    public:
+        using base::base;
+        weird_container(const weird_container&) = default;
+
+        // this copy-assigment cannot provide strong-guarantee for `this`:
+        weird_container& operator=(const weird_container&) = default;
+
+        // this move-ctor cannot provide strong-guarantee for `other`, and even successful, will leave elements of other
+        // in moved state:
+        weird_container(weird_container&& other) noexcept(false) {
+            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 state:
+        weird_container& operator=(weird_container&& other) noexcept(false) {
+            if (size() >= other.size()) {
+                for (size_t i = 0; i < other.size(); ++i) {
+                    at(i) = std::move(other.at(i));
+                }
+                resize(other.size());
+            } else {
+                base::operator=(static_cast<base&&>(other));
+            }
+            return *this;
+        }
+    };
+
+    using SetT = flat_set<weird_elem, key_comparer, weird_container>;
+
+    // copy-assignment
+    {
+        control_throw = wont_throw;
+        SetT fs{1, 2, 3, 4, 5};
+        SetT fs2{6, 7, 8, 9};
+
+        try {
+            control_throw = 3;
+            fs            = fs2;
+        } catch (...) {
+            control_throw = wont_throw;
+            assert_all_requirements(fs);
+        }
+    }
+    // move-ctor
+    {
+        control_throw = wont_throw;
+        SetT fs{1, 2, 3, 4, 5};
+        SetT fs2{std::move(fs)};
+        assert_all_requirements(fs);
+
+        try {
+            control_throw = 2;
+            SetT fs3{std::move(fs2)};
+        } catch (...) {
+            control_throw = wont_throw;
+            assert_all_requirements(fs2);
+        }
+    }
+    // move-assignment
+    {
+        control_throw = wont_throw;
+        SetT fs{1, 2, 3, 4, 5};
+        SetT fs2{6, 7, 8, 9};
+
+        try {
+            control_throw = 2;
+            fs            = std::move(fs2);
+        } catch (...) {
+            control_throw = wont_throw;
+            assert_all_requirements(fs);
+            assert_all_requirements(fs2);
+        }
+    }
+}
+
 // TRANSITION, too simple
 void test_erase_1() {
     flat_set<int> fs{1};
@@ -742,6 +872,7 @@ int main() {
 
     test_erase_1();
     test_erase_2();
+    test_invariant_robustness();
 
     test_erase_if<flat_set<int>>();
     test_erase_if<flat_multiset<int>>();

From 55ce5a075b5f1e1747d20025961484ae0efe90c0 Mon Sep 17 00:00:00 2001
From: achabense <60953653+achabense@users.noreply.github.com>
Date: Wed, 11 Oct 2023 21:08:16 +0800
Subject: [PATCH 08/13] refine the test for `B.`

---
 tests/std/tests/P1222R4_flat_set/test.cpp | 68 ++++++++++++++---------
 1 file changed, 43 insertions(+), 25 deletions(-)

diff --git a/tests/std/tests/P1222R4_flat_set/test.cpp b/tests/std/tests/P1222R4_flat_set/test.cpp
index 3686ddb8c06..7608fdc1dfd 100644
--- a/tests/std/tests/P1222R4_flat_set/test.cpp
+++ b/tests/std/tests/P1222R4_flat_set/test.cpp
@@ -557,7 +557,7 @@ void test_invariant_robustness() {
     static int control_throw        = 0;
     static constexpr int wont_throw = INT_MAX;
 
-    struct weird_elem {
+    struct odd_key {
         static void countdown() {
             if (control_throw == wont_throw) {
                 return;
@@ -569,46 +569,46 @@ void test_invariant_robustness() {
 
         int key;
 
-        weird_elem(int k = 0) : key(k) {}
-        bool operator==(const weird_elem& other) const {
+        odd_key(int k = 0) : key(k) {}
+        bool operator==(const odd_key& other) const {
             return key == other.key;
         }
 
-        weird_elem(const weird_elem& other) : key(other.key) {
+        odd_key(const odd_key& other) : key(other.key) {
             countdown();
         }
 
-        weird_elem(weird_elem&& other) : key(exchange(other.key, 0)) {
+        odd_key(odd_key&& other) : key(exchange(other.key, 0)) {
             countdown();
         }
 
-        weird_elem& operator=(const weird_elem& other) {
+        odd_key& operator=(const odd_key& other) {
             countdown();
             key = other.key;
             return *this;
         }
 
-        weird_elem& operator=(weird_elem&& other) {
+        odd_key& operator=(odd_key&& other) {
             countdown();
             key = exchange(other.key, 0);
             return *this;
         }
     };
 
-    class weird_container : public vector<weird_elem> {
+    class odd_container : public vector<odd_key> {
     private:
-        using base = vector<weird_elem>;
+        using base = vector<odd_key>;
 
     public:
         using base::base;
-        weird_container(const weird_container&) = default;
+        odd_container(const odd_container&) = default;
 
-        // this copy-assigment cannot provide strong-guarantee for `this`:
-        weird_container& operator=(const weird_container&) = default;
+        // this copy-assignment cannot provide strong-guarantee for `this`:
+        odd_container& operator=(const odd_container&) = default;
 
-        // this move-ctor cannot provide strong-guarantee for `other`, and even successful, will leave elements of other
-        // in moved state:
-        weird_container(weird_container&& other) noexcept(false) {
+        // this move-ctor cannot provide strong-guarantee for `other`, and even successful, will leave elements of
+        // `other` in moved state:
+        odd_container(odd_container&& other) {
             reserve(other.size());
             for (auto& e : other) {
                 push_back(std::move(e));
@@ -616,12 +616,10 @@ void test_invariant_robustness() {
         }
 
         // this move-assignment cannot provide strong-guarantee for `this` and `other`, and even successful, will leave
-        // elements of other in moved state:
-        weird_container& operator=(weird_container&& other) noexcept(false) {
+        // elements of `other` in moved state:
+        odd_container& operator=(odd_container&& other) {
             if (size() >= other.size()) {
-                for (size_t i = 0; i < other.size(); ++i) {
-                    at(i) = std::move(other.at(i));
-                }
+                std::move(other.begin(), other.end(), begin());
                 resize(other.size());
             } else {
                 base::operator=(static_cast<base&&>(other));
@@ -630,7 +628,7 @@ void test_invariant_robustness() {
         }
     };
 
-    using SetT = flat_set<weird_elem, key_comparer, weird_container>;
+    using SetT = flat_set<odd_key, key_comparer, odd_container>;
 
     // copy-assignment
     {
@@ -638,43 +636,63 @@ void test_invariant_robustness() {
         SetT fs{1, 2, 3, 4, 5};
         SetT fs2{6, 7, 8, 9};
 
+        assert(ranges::equal(fs, vector{1, 2, 3, 4, 5}, {}, &odd_key::key));
+        assert(ranges::equal(fs2, vector{6, 7, 8, 9}, {}, &odd_key::key));
+
+        bool caught = false;
         try {
-            control_throw = 3;
+            control_throw = 2;
             fs            = fs2;
         } catch (...) {
             control_throw = wont_throw;
             assert_all_requirements(fs);
+            caught = true;
         }
+        assert(caught);
     }
     // move-ctor
     {
         control_throw = wont_throw;
         SetT fs{1, 2, 3, 4, 5};
         SetT fs2{std::move(fs)};
+
         assert_all_requirements(fs);
+        assert(ranges::equal(fs2, vector{1, 2, 3, 4, 5}, {}, &odd_key::key));
 
+        bool caught = false;
         try {
             control_throw = 2;
             SetT fs3{std::move(fs2)};
         } catch (...) {
             control_throw = wont_throw;
             assert_all_requirements(fs2);
+            caught = true;
         }
+        assert(caught);
     }
     // move-assignment
     {
         control_throw = wont_throw;
         SetT fs{1, 2, 3, 4, 5};
-        SetT fs2{6, 7, 8, 9};
+        SetT fs2;
+        SetT fs3{6, 7, 8, 9};
+        fs2 = std::move(fs);
 
+        assert_all_requirements(fs);
+        assert(ranges::equal(fs2, vector{1, 2, 3, 4, 5}, {}, &odd_key::key));
+        assert(ranges::equal(fs3, vector{6, 7, 8, 9}, {}, &odd_key::key));
+
+        bool caught = false;
         try {
             control_throw = 2;
-            fs            = std::move(fs2);
+            fs2           = std::move(fs3);
         } catch (...) {
             control_throw = wont_throw;
-            assert_all_requirements(fs);
             assert_all_requirements(fs2);
+            assert_all_requirements(fs3);
+            caught = true;
         }
+        assert(caught);
     }
 }
 

From 0f13dcbe472d282818840e6257e62e4bfca51688 Mon Sep 17 00:00:00 2001
From: achabense <60953653+achabense@users.noreply.github.com>
Date: Wed, 11 Oct 2023 22:07:12 +0800
Subject: [PATCH 09/13] add test for `A.`

---
 tests/std/tests/P1222R4_flat_set/test.cpp | 27 +++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tests/std/tests/P1222R4_flat_set/test.cpp b/tests/std/tests/P1222R4_flat_set/test.cpp
index 7608fdc1dfd..9f7b32840f1 100644
--- a/tests/std/tests/P1222R4_flat_set/test.cpp
+++ b/tests/std/tests/P1222R4_flat_set/test.cpp
@@ -362,6 +362,32 @@ void test_insert_transparent() {
     assert(detector.converted);
 }
 
+void test_insert_transparent_partially_inconsistent() {
+    // `broken_key` makes inconsistency between transparent searching and conversion.
+    // however, the standard's current specification (per N4958 [flat.set.modifiers]/2) is unable to prohibit types like
+    // this.
+    struct broken_key {
+        int key;
+        explicit operator int() const {
+            return key - 10;
+        }
+    };
+
+    flat_set<int, key_comparer> fs{0, 3, 5};
+
+    // fs.find(10) == fs.end(), but fs.find(10-10) != fs.end(), so this is a precondition violation:
+    // fs.insert(broken_key{ 10 });
+    // fs.find(0) != fs.end(), but fs.find(0-10) == fs.end(), so this is a precondition violation:
+    // fs.insert(broken_key{ 0 });
+
+    // fs.find(2) == fs.end(), and fs.find(2-10) == fs.end(), so this is allowed by the standard:
+    fs.insert(broken_key{2});
+    assert_all_requirements_and_equals(fs, {-8, 0, 3, 5});
+    // fs.find(9) == fs.end(), and fs.find(9-10) == fs.end(), so this is allowed by the standard:
+    fs.insert(fs.end(), broken_key{9});
+    assert_all_requirements_and_equals(fs, {-8, -1, 0, 3, 5});
+}
+
 void test_insert_using_invalid_hint() {
     mt19937 eng(42);
 
@@ -877,6 +903,7 @@ int main() {
     test_insert_2<vector<int>>();
     test_insert_2<deque<int>>();
     test_insert_transparent();
+    test_insert_transparent_partially_inconsistent();
     test_insert_using_invalid_hint();
     test_insert_upper_bound();
 

From ffcbd64808af91421294c0d5ab6c0dd8901777c5 Mon Sep 17 00:00:00 2001
From: achabense <60953653+achabense@users.noreply.github.com>
Date: Thu, 12 Oct 2023 00:21:31 +0800
Subject: [PATCH 10/13] (some commits are extracted to pr4084)

---
 stl/inc/flat_set                          |  37 ++--
 tests/std/tests/P1222R4_flat_set/test.cpp | 247 +---------------------
 2 files changed, 28 insertions(+), 256 deletions(-)

diff --git a/stl/inc/flat_set b/stl/inc/flat_set
index b07beff49c4..41fb5aabb75 100644
--- a/stl/inc/flat_set
+++ b/stl/inc/flat_set
@@ -101,7 +101,7 @@ public:
     template <_Allocator_for<container_type> _Alloc>
     explicit _Base_flat_set(const _Alloc& _Al) : _Mycont(_Al), _Mycomp() {}
 
-    // FIXME, an allocator-aware container may not support "C(_First, _Last, _Al)".
+    // TRANSITION, an allocator-aware container may not support "C(_First, _Last, _Al)".
     template <input_iterator _Iter>
     _Base_flat_set(_Iter _First, _Iter _Last, const key_compare& _Comp = key_compare())
         : _Base_flat_set(container_type(_First, _Last), _Comp) {}
@@ -111,7 +111,7 @@ public:
     template <input_iterator _Iter, _Allocator_for<container_type> _Alloc>
     _Base_flat_set(_Iter _First, _Iter _Last, const _Alloc& _Al) : _Base_flat_set(container_type(_First, _Last, _Al)) {}
 
-    // FIXME, an allocator-aware container may not support "C(from_range, _STD forward<_Rng>(_Range), _Al)".
+    // TRANSITION, an allocator-aware container may not support "C(from_range, _STD forward<_Rng>(_Range), _Al)".
     template <_Container_compatible_range<_Kty> _Rng>
     _Base_flat_set(from_range_t, _Rng&& _Range)
         : _Base_flat_set(container_type(from_range, _STD forward<_Rng>(_Range))) {}
@@ -135,7 +135,7 @@ public:
     _Base_flat_set(_Tsorted _Tsort, _Iter _First, _Iter _Last, const _Alloc& _Al)
         : _Base_flat_set(_Tsort, container_type(_First, _Last, _Al)) {}
 
-    // FIXME, an allocator-aware container may not support "C(_Ilist, _Al)".
+    // TRANSITION, an allocator-aware container may not support "C(_Ilist, _Al)".
     _Base_flat_set(initializer_list<_Kty> _Ilist, const key_compare& _Comp = key_compare())
         : _Base_flat_set(container_type(_Ilist), _Comp) {}
     template <_Allocator_for<container_type> _Alloc>
@@ -237,7 +237,7 @@ public:
     }
 
     // modifiers
-    // FIXME, the "insert" and "erase" methods may not be able to restore the invariant, if the underlying
+    // TRANSITION, the "insert" and "erase" methods may not be able to restore the invariant, if the underlying
     // container is unable to provide strong guarantee for "erase" and "insert" methods.
     template <class... _Args>
     auto emplace(_Args&&... _Vals) {
@@ -511,17 +511,16 @@ private:
 
     template <class _Ty>
         requires _Multi // flat_multiset
-    _NODISCARD iterator _Emplace(_Ty&& _Val) {
+    iterator _Emplace(_Ty&& _Val) {
         _STL_INTERNAL_STATIC_ASSERT(is_same_v<remove_cvref_t<_Ty>, _Kty>);
         return _Mycont.emplace(upper_bound(_Val), _STD forward<_Ty>(_Val));
     }
 
     template <class _Ty>
         requires (!_Multi) // flat_set
-    _NODISCARD pair<iterator, bool> _Emplace(_Ty&& _Val) {
+    pair<iterator, bool> _Emplace(_Ty&& _Val) {
         const iterator _Where = lower_bound(_Val);
         if (_Where != end() && !_Compare(_Val, *_Where)) {
-            // the set already contains _Val.
             return pair{_Where, false};
         }
 
@@ -529,18 +528,18 @@ private:
             return pair{_Mycont.emplace(_Where, _STD forward<_Ty>(_Val)), true};
         } else {
             // heterogeneous insertion
+            // TRANSITION, the requirements may be stricter than needed by the standard.
             _STL_INTERNAL_STATIC_ASSERT(_Keylt_transparent && is_constructible_v<_Kty, _Ty>);
             _Kty _Keyval(_STD forward<_Ty>(_Val));
-            // precondition: `find(_Val) == find(_Keyval)` (per N4958 [flat.set.modifiers]/2)
-            // the precondition cannot guarantee `_Can_insert(_Where, _Keyval)` here.
-            _STL_ASSERT(!contains(_Keyval), "find(_Val) == end(), but find(_Keyval) != end()");
-            return pair{_Emplace_hint(_Where, _STD move(_Keyval)), true};
+            _STL_ASSERT(_Can_insert(_Where, _Keyval), "The conversion from the heterogeneous key to key_type should "
+                                                      "be consistent with the heterogeneous lookup!");
+            return pair{_Mycont.emplace(_Where, _STD move(_Keyval)), true};
         }
     }
 
     template <class _Ty>
         requires _Multi // flat_multiset
-    _NODISCARD iterator _Emplace_hint(const_iterator _Where, _Ty&& _Val) {
+    iterator _Emplace_hint(const_iterator _Where, _Ty&& _Val) {
         _STL_INTERNAL_STATIC_ASSERT(is_same_v<remove_cvref_t<_Ty>, _Kty>);
 
         const const_iterator _Begin = cbegin();
@@ -566,7 +565,7 @@ private:
 
     template <class _Ty>
         requires (!_Multi) // flat_set
-    _NODISCARD iterator _Emplace_hint(const_iterator _Where, _Ty&& _Val) {
+    iterator _Emplace_hint(const_iterator _Where, _Ty&& _Val) {
         const const_iterator _Begin = cbegin();
         const const_iterator _End   = cend();
 
@@ -585,7 +584,7 @@ private:
         }
 
         if (_Where != _End && !_Compare(_Val, *_Where)) {
-            // the set already contains _Val; return _Where as iterator type.
+            // convert const_iterator to iterator
             return _Mycont.begin() + (_Where - _Begin);
         }
 
@@ -594,12 +593,12 @@ private:
             return _Mycont.emplace(_Where, _STD forward<_Ty>(_Val));
         } else {
             // heterogeneous insertion
+            // TRANSITION, the requirements may be stricter than needed by the standard.
             _STL_INTERNAL_STATIC_ASSERT(_Keylt_transparent && is_constructible_v<_Kty, _Ty>);
             _Kty _Keyval(_STD forward<_Ty>(_Val));
-            // precondition: `find(_Val) == find(_Keyval)` (per N4958 [flat.set.modifiers]/2)
-            // the precondition cannot guarantee `_Can_insert(_Where, _Keyval)` here.
-            _STL_ASSERT(!contains(_Keyval), "find(_Val) == end(), but find(_Keyval) != end()");
-            return _Emplace_hint(_Where, _STD move(_Keyval));
+            _STL_ASSERT(_Can_insert(_Where, _Keyval), "The conversion from the heterogeneous key to key_type should "
+                                                      "be consistent with the heterogeneous lookup!");
+            return _Mycont.emplace(_Where, _STD move(_Keyval));
         }
     }
 
@@ -611,7 +610,7 @@ private:
     }
 
     template <class _Ty>
-    _NODISCARD size_type _Erase(const _Ty& _Val) {
+    size_type _Erase(const _Ty& _Val) {
         _STL_INTERNAL_STATIC_ASSERT(_Keylt_transparent || is_same_v<_Ty, _Kty>);
 
         if constexpr (!_Multi && is_same_v<_Ty, _Kty>) {
diff --git a/tests/std/tests/P1222R4_flat_set/test.cpp b/tests/std/tests/P1222R4_flat_set/test.cpp
index 9f7b32840f1..392b27092a9 100644
--- a/tests/std/tests/P1222R4_flat_set/test.cpp
+++ b/tests/std/tests/P1222R4_flat_set/test.cpp
@@ -9,7 +9,6 @@
 #include <iostream>
 #include <memory>
 #include <random>
-#include <tuple>
 #include <utility>
 #include <vector>
 
@@ -64,42 +63,19 @@ void assert_reversible_container_requirements(const T& s) {
     static_assert(is_same_v<reverse_iterator<typename T::const_iterator>, typename T::const_reverse_iterator>);
     static_assert(is_same_v<decltype(T{}.rbegin()), typename T::reverse_iterator>);
     static_assert(is_same_v<decltype(T{}.rend()), typename T::reverse_iterator>);
-    static_assert(is_same_v<decltype(s.rbegin()), typename T::const_reverse_iterator>);
-    static_assert(is_same_v<decltype(s.rend()), typename T::const_reverse_iterator>);
     static_assert(is_same_v<decltype(s.crbegin()), typename T::const_reverse_iterator>);
     static_assert(is_same_v<decltype(s.crend()), typename T::const_reverse_iterator>);
     static_assert(is_convertible_v<typename T::reverse_iterator, typename T::const_reverse_iterator>);
 }
 
-template <class T>
-void assert_noexcept_requirements(T& s) {
-    static_assert(noexcept(s.begin()));
-    static_assert(noexcept(s.end()));
-    static_assert(noexcept(s.cbegin()));
-    static_assert(noexcept(s.cend()));
-    static_assert(noexcept(s.rbegin()));
-    static_assert(noexcept(s.rend()));
-    static_assert(noexcept(s.crbegin()));
-    static_assert(noexcept(s.crend()));
-
-    static_assert(noexcept(s.empty()));
-    static_assert(noexcept(s.size()));
-    static_assert(noexcept(s.max_size()));
-
-    if constexpr (!is_const_v<T>) {
-        static_assert(noexcept(s.swap(s)));
-        static_assert(noexcept(ranges::swap(s, s))); // using ADL-swap
-        static_assert(noexcept(s.clear()));
-    }
-}
-
 template <class T>
 void assert_all_requirements(const T& s) {
     assert_container_requirements(s);
     assert_reversible_container_requirements(s);
 
-    assert_noexcept_requirements(s);
-    assert_noexcept_requirements(const_cast<T&>(s));
+    // in pr4084
+    // assert_noexcept_requirements(s);
+    // assert_noexcept_requirements(const_cast<T&>(s));
 
     auto val_comp = s.value_comp();
     auto begin_it = s.cbegin();
@@ -164,30 +140,6 @@ void test_constructors() {
     assert_all_requirements_and_equals(flat_multiset<int>(std::move(b), allocator<int>{}), {-1, 1, 2, 7, 7, 7, 100});
 }
 
-template <template <class...> class Set>
-void test_always_reversible() {
-    // Test that flat_meow is unconditionally reversible.
-    class not_reversible : public vector<int> {
-    private:
-        using base = vector<int>;
-
-        using base::const_reverse_iterator;
-        using base::crbegin;
-        using base::crend;
-        using base::rbegin;
-        using base::rend;
-        using base::reverse_iterator;
-
-    public:
-        using base::base;
-    };
-
-    Set<int, std::greater<int>, not_reversible> fs({1, 2, 3});
-    assert_all_requirements_and_equals(fs, {3, 2, 1});
-    assert(fs.rbegin() + 3 == fs.rend());
-    assert(fs.crend() - fs.crbegin() == 3);
-}
-
 template <class C>
 void test_insert_1() {
     using lt = std::less<int>;
@@ -362,32 +314,6 @@ void test_insert_transparent() {
     assert(detector.converted);
 }
 
-void test_insert_transparent_partially_inconsistent() {
-    // `broken_key` makes inconsistency between transparent searching and conversion.
-    // however, the standard's current specification (per N4958 [flat.set.modifiers]/2) is unable to prohibit types like
-    // this.
-    struct broken_key {
-        int key;
-        explicit operator int() const {
-            return key - 10;
-        }
-    };
-
-    flat_set<int, key_comparer> fs{0, 3, 5};
-
-    // fs.find(10) == fs.end(), but fs.find(10-10) != fs.end(), so this is a precondition violation:
-    // fs.insert(broken_key{ 10 });
-    // fs.find(0) != fs.end(), but fs.find(0-10) == fs.end(), so this is a precondition violation:
-    // fs.insert(broken_key{ 0 });
-
-    // fs.find(2) == fs.end(), and fs.find(2-10) == fs.end(), so this is allowed by the standard:
-    fs.insert(broken_key{2});
-    assert_all_requirements_and_equals(fs, {-8, 0, 3, 5});
-    // fs.find(9) == fs.end(), and fs.find(9-10) == fs.end(), so this is allowed by the standard:
-    fs.insert(fs.end(), broken_key{9});
-    assert_all_requirements_and_equals(fs, {-8, -1, 0, 3, 5});
-}
-
 void test_insert_using_invalid_hint() {
     mt19937 eng(42);
 
@@ -425,35 +351,6 @@ void test_insert_using_invalid_hint() {
     }
 }
 
-void test_insert_upper_bound() {
-    // For flat_multiset's single-element insertion, the key should be inserted before the upper_bound.
-    struct test_position {
-        int key;
-        int extra;
-        bool operator==(const test_position& other) const {
-            return key == other.key && extra == other.extra;
-        }
-    };
-
-    mt19937 eng(24);
-    uniform_int_distribution<int> dist_seq(0, 20);
-
-    vector<test_position> seq(200);
-    for (int e = 0; auto& [key, extra] : seq) {
-        key   = dist_seq(eng);
-        extra = e++;
-    }
-
-    flat_multiset<test_position, key_comparer> fs;
-    for (const auto& val : seq) {
-        fs.insert(val);
-    }
-
-    // The result should be identical to as if doing stable_sort on seq.
-    ranges::stable_sort(seq, key_comparer{});
-    assert(ranges::equal(fs, seq));
-}
-
 template <class T>
 void test_spaceship_operator() {
     static constexpr bool multi  = _Is_specialization_v<T, flat_multiset>;
@@ -760,126 +657,13 @@ void test_erase_if() {
     assert(ranges::equal(fs, erased_result));
 }
 
-template <template <class...> class Set>
-void test_observers() {
-    struct lt_with_state {
-        int state = 0;
-        bool operator()(int l, int r) const {
-            return l < r;
-        }
-    };
-
-    using SetT = Set<int, lt_with_state>;
-
-    SetT fs;
-    static_assert(is_same_v<typename SetT::key_compare, typename SetT::value_compare>);
-    static_assert(is_same_v<decltype(fs.key_comp()), typename SetT::key_compare>);
-    static_assert(is_same_v<decltype(fs.value_comp()), typename SetT::value_compare>);
-    assert(fs.key_comp().state == 0);
-    assert(fs.value_comp().state == 0);
-
-    SetT fs2(lt_with_state{2});
-    assert(fs2.key_comp().state == 2);
-    assert(fs2.value_comp().state == 2);
-}
-
-template <template <class...> class Set>
-void test_set_operations() {
-    using SetT           = Set<int>;
-    using iterator       = SetT::iterator;
-    using const_iterator = SetT::const_iterator;
-
-    SetT fs{3, 2, 11, 11, 3, 8, 11, 20};
-
-    static_assert(is_same_v<decltype(fs.find(0)), iterator>);
-    static_assert(is_same_v<decltype(as_const(fs).find(0)), const_iterator>);
-
-    static_assert(is_same_v<decltype(fs.count(0)), typename SetT::size_type>);
-    static_assert(is_same_v<decltype(fs.contains(0)), bool>);
-
-    static_assert(is_same_v<decltype(fs.lower_bound(0)), iterator>);
-    static_assert(is_same_v<decltype(as_const(fs).lower_bound(0)), const_iterator>);
-    static_assert(is_same_v<decltype(fs.upper_bound(0)), iterator>);
-    static_assert(is_same_v<decltype(as_const(fs).upper_bound(0)), const_iterator>);
-
-    static_assert(is_same_v<decltype(fs.equal_range(0)), pair<iterator, iterator>>);
-    static_assert(is_same_v<decltype(as_const(fs).equal_range(0)), pair<const_iterator, const_iterator>>);
-
-    if constexpr (!_Is_specialization_v<SetT, flat_multiset>) {
-        // flat_set:
-        assert_all_requirements_and_equals(fs, {2, 3, 8, 11, 20});
-
-        assert(fs.find(3) != fs.end());
-        assert(fs.find(4) == fs.end());
-
-        assert(fs.count(1) == 0);
-        assert(fs.count(11) == 1);
-        assert(fs.contains(8));
-        assert(!fs.contains(12));
-
-        assert(fs.lower_bound(-1) == fs.begin());
-        assert(fs.lower_bound(3) == fs.find(3));
-        assert(fs.lower_bound(19) == fs.find(20));
-        assert(fs.lower_bound(20) + 1 == fs.end());
-
-        assert(fs.upper_bound(-1) == fs.begin());
-        assert(fs.upper_bound(20) == fs.end());
-        assert(fs.lower_bound(2) + 2 == fs.upper_bound(3));
-        assert(fs.upper_bound(8) == fs.find(11));
-
-        auto [first, last] = fs.equal_range(3);
-        assert(first + 1 == last);
-        tie(first, last) = fs.equal_range(12);
-        assert(first == last);
-    } else {
-        // flat_multiset:
-        assert_all_requirements_and_equals(fs, {2, 3, 3, 8, 11, 11, 11, 20});
-
-        assert(fs.find(3) != fs.end());
-        assert(fs.find(4) == fs.end());
-
-        assert(fs.count(1) == 0);
-        assert(fs.count(11) == 3);
-        assert(fs.contains(8));
-        assert(!fs.contains(12));
-
-        assert(fs.lower_bound(-1) == fs.begin());
-        assert(fs.lower_bound(3) == fs.find(3));
-        assert(fs.lower_bound(19) == fs.find(20));
-        assert(fs.lower_bound(11) + 4 == fs.end());
-
-        assert(fs.upper_bound(-1) == fs.begin());
-        assert(fs.upper_bound(20) == fs.end());
-        assert(fs.lower_bound(3) + 6 == fs.upper_bound(11));
-        assert(fs.upper_bound(11) == fs.find(20));
-
-        auto [first, last] = fs.equal_range(3);
-        assert(first + 2 == last);
-        tie(first, last) = fs.equal_range(12);
-        assert(first == last);
-    }
-}
-
-template <template <class...> class Set>
-void test_set_operations_transparent() {
-    struct shouldnt_convert {
-        int key;
-        /* implicit */ [[noreturn]] operator int() const {
-            abort();
-        }
-    };
-
-    Set<int, key_comparer> fs{0, 3, 5};
-    assert_all_requirements_and_equals(fs, {0, 3, 5});
+// TRANSITION, too simple
+void test_count() {
+    flat_set<int> fs{2};
+    assert(fs.count(1) == 0);
 
-    assert(fs.find(shouldnt_convert{0}) != fs.end());
-    assert(fs.count(shouldnt_convert{3}) == 1);
-    assert(!fs.contains(shouldnt_convert{1}));
-    assert(fs.lower_bound(shouldnt_convert{-1}) == fs.begin());
-    assert(fs.lower_bound(shouldnt_convert{8}) == fs.end());
-    assert(fs.upper_bound(shouldnt_convert{2}) == fs.find(3));
-    auto [first, last] = fs.equal_range(shouldnt_convert{5});
-    assert(first != last);
+    flat_multiset<int> fs2{10, 20, 20, 30};
+    assert(fs2.count(20) == 2);
 }
 
 int main() {
@@ -895,17 +679,12 @@ int main() {
     test_constructors<vector<int>>();
     test_constructors<deque<int>>();
 
-    test_always_reversible<flat_set>();
-    test_always_reversible<flat_multiset>();
-
     test_insert_1<vector<int>>();
     test_insert_1<deque<int>>();
     test_insert_2<vector<int>>();
     test_insert_2<deque<int>>();
     test_insert_transparent();
-    test_insert_transparent_partially_inconsistent();
     test_insert_using_invalid_hint();
-    test_insert_upper_bound();
 
     test_comparer_application();
     test_non_static_comparer();
@@ -922,11 +701,5 @@ int main() {
     test_erase_if<flat_set<int>>();
     test_erase_if<flat_multiset<int>>();
 
-    test_observers<flat_set>();
-    test_observers<flat_multiset>();
-
-    test_set_operations<flat_set>();
-    test_set_operations<flat_multiset>();
-    test_set_operations_transparent<flat_set>();
-    test_set_operations_transparent<flat_multiset>();
+    test_count();
 }

From dce6c9178d21c18b630d0cd72e97f91d58eb990d Mon Sep 17 00:00:00 2001
From: achabense <60953653+achabense@users.noreply.github.com>
Date: Thu, 12 Oct 2023 17:15:06 +0800
Subject: [PATCH 11/13] check the address for move-assignment

---
 stl/inc/flat_set                          | 12 +++++++-----
 tests/std/tests/P1222R4_flat_set/test.cpp |  8 ++++----
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/stl/inc/flat_set b/stl/inc/flat_set
index 41fb5aabb75..3503f01d9f9 100644
--- a/stl/inc/flat_set
+++ b/stl/inc/flat_set
@@ -169,11 +169,13 @@ public:
     _Base_flat_set& operator=(_Base_flat_set&& _Other) noexcept(
         is_nothrow_move_assignable_v<container_type>&& is_nothrow_copy_assignable_v<key_compare>) // strengthened
     {
-        _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;
+        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;
     }
 
diff --git a/tests/std/tests/P1222R4_flat_set/test.cpp b/tests/std/tests/P1222R4_flat_set/test.cpp
index 392b27092a9..a1557eb37e3 100644
--- a/tests/std/tests/P1222R4_flat_set/test.cpp
+++ b/tests/std/tests/P1222R4_flat_set/test.cpp
@@ -529,8 +529,8 @@ void test_invariant_robustness() {
         // this copy-assignment cannot provide strong-guarantee for `this`:
         odd_container& operator=(const odd_container&) = default;
 
-        // this move-ctor cannot provide strong-guarantee for `other`, and even successful, will leave elements of
-        // `other` in moved state:
+        // this move-ctor cannot provide strong-guarantee for `other`, and even successful, can leave elements of
+        // `other` in moved-from state:
         odd_container(odd_container&& other) {
             reserve(other.size());
             for (auto& e : other) {
@@ -538,8 +538,8 @@ void test_invariant_robustness() {
             }
         }
 
-        // this move-assignment cannot provide strong-guarantee for `this` and `other`, and even successful, will leave
-        // elements of `other` in moved state:
+        // this move-assignment cannot provide strong-guarantee for `this` and `other`, and even successful, can leave
+        // elements of `other` in moved-from state:
         odd_container& operator=(odd_container&& other) {
             if (size() >= other.size()) {
                 std::move(other.begin(), other.end(), begin());

From 8bb7bc0aaae668c56fdb73b687828b9a8fe3384e Mon Sep 17 00:00:00 2001
From: achabense <60953653+achabense@users.noreply.github.com>
Date: Fri, 13 Oct 2023 21:02:05 +0800
Subject: [PATCH 12/13] refine test

---
 tests/std/tests/P1222R4_flat_set/test.cpp | 86 ++++++++++++-----------
 1 file changed, 44 insertions(+), 42 deletions(-)

diff --git a/tests/std/tests/P1222R4_flat_set/test.cpp b/tests/std/tests/P1222R4_flat_set/test.cpp
index a1557eb37e3..1621d709b3e 100644
--- a/tests/std/tests/P1222R4_flat_set/test.cpp
+++ b/tests/std/tests/P1222R4_flat_set/test.cpp
@@ -477,15 +477,15 @@ void test_extract_2() {
 }
 
 void test_invariant_robustness() {
-    static int control_throw        = 0;
-    static constexpr int wont_throw = INT_MAX;
+    static int copy_limit          = 2;
+    static constexpr int unlimited = INT_MAX;
 
     struct odd_key {
         static void countdown() {
-            if (control_throw == wont_throw) {
+            if (copy_limit == unlimited) {
                 return;
             }
-            if (--control_throw <= 0) {
+            if (--copy_limit < 0) {
                 throw 0; // will be caught by "catch (...)".
             }
         }
@@ -497,12 +497,14 @@ void test_invariant_robustness() {
             return key == other.key;
         }
 
-        odd_key(const odd_key& other) : key(other.key) {
+        odd_key(const odd_key& other) {
             countdown();
+            key = other.key;
         }
 
-        odd_key(odd_key&& other) : key(exchange(other.key, 0)) {
+        odd_key(odd_key&& other) {
             countdown();
+            key = exchange(other.key, 0);
         }
 
         odd_key& operator=(const odd_key& other) {
@@ -527,9 +529,13 @@ void test_invariant_robustness() {
         odd_container(const odd_container&) = default;
 
         // this copy-assignment cannot provide strong-guarantee for `this`:
-        odd_container& operator=(const odd_container&) = default;
+        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, can leave elements of
+        // 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());
@@ -538,15 +544,11 @@ void test_invariant_robustness() {
             }
         }
 
-        // this move-assignment cannot provide strong-guarantee for `this` and `other`, and even successful, can leave
+        // 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) {
-            if (size() >= other.size()) {
-                std::move(other.begin(), other.end(), begin());
-                resize(other.size());
-            } else {
-                base::operator=(static_cast<base&&>(other));
-            }
+            resize(other.size());
+            std::move(other.begin(), other.end(), begin());
             return *this;
         }
     };
@@ -555,39 +557,39 @@ void test_invariant_robustness() {
 
     // copy-assignment
     {
-        control_throw = wont_throw;
-        SetT fs{1, 2, 3, 4, 5};
-        SetT fs2{6, 7, 8, 9};
+        copy_limit = unlimited;
+        SetT fs1{0, 1, 2, 3, 4};
+        SetT fs2{5, 6, 7, 8, 9};
 
-        assert(ranges::equal(fs, vector{1, 2, 3, 4, 5}, {}, &odd_key::key));
-        assert(ranges::equal(fs2, vector{6, 7, 8, 9}, {}, &odd_key::key));
+        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 {
-            control_throw = 2;
-            fs            = fs2;
+            copy_limit = 2;
+            fs1        = fs2; // will throw after copying 2 odd_key.
         } catch (...) {
-            control_throw = wont_throw;
-            assert_all_requirements(fs);
+            copy_limit = unlimited;
+            assert_all_requirements(fs1);
             caught = true;
         }
         assert(caught);
     }
     // move-ctor
     {
-        control_throw = wont_throw;
-        SetT fs{1, 2, 3, 4, 5};
-        SetT fs2{std::move(fs)};
+        copy_limit = unlimited;
+        SetT fs1{0, 1, 2, 3, 4};
+        SetT fs2{std::move(fs1)};
 
-        assert_all_requirements(fs);
-        assert(ranges::equal(fs2, vector{1, 2, 3, 4, 5}, {}, &odd_key::key));
+        assert_all_requirements(fs1);
+        assert(ranges::equal(fs2, vector{0, 1, 2, 3, 4}, {}, &odd_key::key));
 
         bool caught = false;
         try {
-            control_throw = 2;
-            SetT fs3{std::move(fs2)};
+            copy_limit = 2;
+            SetT fs3{std::move(fs2)}; // will throw after moving 2 odd_key.
         } catch (...) {
-            control_throw = wont_throw;
+            copy_limit = unlimited;
             assert_all_requirements(fs2);
             caught = true;
         }
@@ -595,22 +597,22 @@ void test_invariant_robustness() {
     }
     // move-assignment
     {
-        control_throw = wont_throw;
-        SetT fs{1, 2, 3, 4, 5};
+        copy_limit = unlimited;
+        SetT fs1{0, 1, 2, 3, 4};
         SetT fs2;
-        SetT fs3{6, 7, 8, 9};
-        fs2 = std::move(fs);
+        SetT fs3{5, 6, 7, 8, 9};
+        fs2 = std::move(fs1);
 
-        assert_all_requirements(fs);
-        assert(ranges::equal(fs2, vector{1, 2, 3, 4, 5}, {}, &odd_key::key));
-        assert(ranges::equal(fs3, vector{6, 7, 8, 9}, {}, &odd_key::key));
+        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 {
-            control_throw = 2;
-            fs2           = std::move(fs3);
+            copy_limit = 2;
+            fs2        = std::move(fs3); // will throw after moving 2 odd_key.
         } catch (...) {
-            control_throw = wont_throw;
+            copy_limit = unlimited;
             assert_all_requirements(fs2);
             assert_all_requirements(fs3);
             caught = true;

From 8de2226b24e4cfc2997c73f28a3cf1692eae840a Mon Sep 17 00:00:00 2001
From: "Stephan T. Lavavej" <stl@nuwen.net>
Date: Wed, 18 Oct 2023 12:01:11 -0700
Subject: [PATCH 13/13] Code review feedback.

---
 tests/std/tests/P1222R4_flat_set/test.cpp | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tests/std/tests/P1222R4_flat_set/test.cpp b/tests/std/tests/P1222R4_flat_set/test.cpp
index 1621d709b3e..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 <algorithm>
 #include <cassert>
+#include <climits>
 #include <deque>
 #include <flat_set>
 #include <functional>
@@ -73,7 +74,7 @@ void assert_all_requirements(const T& s) {
     assert_container_requirements(s);
     assert_reversible_container_requirements(s);
 
-    // in pr4084
+    // FIXME, in GH-4084
     // assert_noexcept_requirements(s);
     // assert_noexcept_requirements(const_cast<T&>(s));
 
@@ -477,14 +478,15 @@ void test_extract_2() {
 }
 
 void test_invariant_robustness() {
-    static int copy_limit          = 2;
-    static constexpr int unlimited = INT_MAX;
+    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 (...)".
             }
@@ -493,9 +495,8 @@ void test_invariant_robustness() {
         int key;
 
         odd_key(int k = 0) : key(k) {}
-        bool operator==(const odd_key& other) const {
-            return key == other.key;
-        }
+
+        bool operator==(const odd_key&) const = default;
 
         odd_key(const odd_key& other) {
             countdown();