From a6d279c30b7a6b260676def7763ca98d617e74cb Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 22 Jan 2026 12:02:59 -0800 Subject: [PATCH 1/4] Remove FIXME comment: `empty()` is superior to `begin() == end()`, which flat_set no longer says. --- stl/inc/flat_map | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/flat_map b/stl/inc/flat_map index 944d67861b4..eb42c33529e 100644 --- a/stl/inc/flat_map +++ b/stl/inc/flat_map @@ -1099,7 +1099,7 @@ private: } void _Make_invariants_fulfilled() { - if (empty()) { // FIXME maybe consider if (begin() == end()) for consistency with flat_set + if (empty()) { return; } From 7d4136cd64aa1a4589bc3bc1a6027435a624465c Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 22 Jan 2026 12:06:00 -0800 Subject: [PATCH 2/4] Remove FIXME comments about heterogeneous insertion. This appears to have no effect on our implementation; at most it's a wish for the Standard to speak with more clarity. --- stl/inc/flat_set | 4 ---- 1 file changed, 4 deletions(-) diff --git a/stl/inc/flat_set b/stl/inc/flat_set index d5de1e9ff2e..7d6485cc794 100644 --- a/stl/inc/flat_set +++ b/stl/inc/flat_set @@ -536,8 +536,6 @@ private: return pair{_STD _Emplace_with_clear_guard(_Mycont, _Where, _STD forward<_Ty>(_Val)), true}; } else { // heterogeneous insertion - // FIXME: The standard only requires `find(_Val) == find(_Keyval)` (per N4958 [flat.set.modifiers]/2), - // which cannot guarantee `_Can_insert(_Where, _Keyval)`. _STL_INTERNAL_STATIC_ASSERT(_Transparent && is_constructible_v<_Kty, _Ty>); _Kty _Keyval(_STD forward<_Ty>(_Val)); _STL_ASSERT(_Can_insert(_Where, _Keyval), @@ -580,8 +578,6 @@ private: return _STD _Emplace_with_clear_guard(_Mycont, _Where, _STD forward<_Ty>(_Val)); } else { // heterogeneous insertion - // FIXME: The standard only requires `find(_Val) == find(_Keyval)` (per N4958 [flat.set.modifiers]/2), - // which cannot guarantee `_Can_insert(_Where, _Keyval)`. _STL_INTERNAL_STATIC_ASSERT(_Transparent && is_constructible_v<_Kty, _Ty>); _Kty _Keyval(_STD forward<_Ty>(_Val)); _STL_ASSERT(_Can_insert(_Where, _Keyval), From 650bc256779898bb3f5452ac36d89391546d4eed Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 22 Jan 2026 14:04:06 -0800 Subject: [PATCH 3/4] Remove FIXME comment about `erase()`, add `_Clear_guard`s and comments. --- stl/inc/flat_map | 2 ++ stl/inc/flat_set | 20 +++++++++++++++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/stl/inc/flat_map b/stl/inc/flat_map index eb42c33529e..8f8d496fd82 100644 --- a/stl/inc/flat_map +++ b/stl/inc/flat_map @@ -1130,6 +1130,8 @@ private: void _Erase_dupes_if_not_multi() { if constexpr (_IsUnique) { + // No _Clear_guard needed. The only callers are _Insert_range() (which has a _Clear_guard) + // and _Make_invariants_fulfilled() (which is called only by constructors). auto _Sorted_view = _View_to_mutate(); const auto _New_last = _RANGES unique(_Sorted_view, _Erase_dupes_if_not_multi_pred()).begin(); diff --git a/stl/inc/flat_set b/stl/inc/flat_set index 7d6485cc794..570fe59e4f5 100644 --- a/stl/inc/flat_set +++ b/stl/inc/flat_set @@ -377,12 +377,12 @@ public: _Guard._Target = nullptr; } - // FIXME, the "erase" member functions need clear guards to restore the invariant when the underlying - // container doesn't provide the strong guarantee for its "erase" member functions. - // NB: `erase(iterator)` is identical to `erase(const_iterator)` iterator erase(const const_iterator _Where) { - return _Mycont.erase(_Where); + _Clear_guard _Guard{this}; + const auto _Ret = _Mycont.erase(_Where); + _Guard._Target = nullptr; + return _Ret; } size_type erase(const _Kty& _Val) { return _Erase(_Val); @@ -393,7 +393,10 @@ public: return _Erase(_Val); } iterator erase(const const_iterator _First, const const_iterator _Last) { - return _Mycont.erase(_First, _Last); + _Clear_guard _Guard{this}; + const auto _Ret = _Mycont.erase(_First, _Last); + _Guard._Target = nullptr; + return _Ret; } void swap(_Derived& _Other) noexcept(is_nothrow_swappable_v<_Container> && is_nothrow_swappable_v<_Keylt>) { @@ -636,7 +639,9 @@ private: if constexpr (_IsUnique && is_same_v<_Ty, key_type>) { // Optimization restricted due to GH-5992 const const_iterator _Where = lower_bound(_Val); if (_Where != cend() && !_Compare(_Val, *_Where)) { + _Clear_guard _Guard{this}; _Mycont.erase(_Where); + _Guard._Target = nullptr; return 1; } return 0; @@ -644,7 +649,9 @@ private: const auto [_First, _Last] = equal_range(_Val); const auto _Removed = static_cast(_Last - _First); + _Clear_guard _Guard{this}; _Mycont.erase(_First, _Last); + _Guard._Target = nullptr; return _Removed; } } @@ -713,6 +720,9 @@ private: void _Erase_dupes_if_not_multi() { if constexpr (_IsUnique) { + // No _Clear_guard needed. The only callers are _Restore_invariants_after_insert() (which is called + // only by _Insert_range() which has a _Clear_guard) and _Make_invariants_fulfilled() (which is called + // only by constructors which don't need guards and operator=() which has a _Clear_guard). const auto _End = _Mycont.end(); _Mycont.erase(_STD unique(_Mycont.begin(), _End, _Erase_dupes_if_not_multi_pred()), _End); } From 3ee6b0489175da2f3c9deb081df86e2e1f83b3fb Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Thu, 22 Jan 2026 15:38:47 -0800 Subject: [PATCH 4/4] Remove FIXME comment, test remaining constructors. --- tests/std/tests/P0429R9_flat_map/test.cpp | 96 ++++++++++++++++++++++- 1 file changed, 92 insertions(+), 4 deletions(-) diff --git a/tests/std/tests/P0429R9_flat_map/test.cpp b/tests/std/tests/P0429R9_flat_map/test.cpp index 9b2625a1b0d..7be0b081067 100644 --- a/tests/std/tests/P0429R9_flat_map/test.cpp +++ b/tests/std/tests/P0429R9_flat_map/test.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -278,9 +279,36 @@ void test_construction() { } } { - // Test flat_map(key_cont, mapped_cont, comp = key_comp()) + // Test flat_map(const alloc&) + // and flat_map(const key_comp&, const alloc&) + { + MyAllocatorCounter allocation_counter; + flat_map fmap(MyAllocator{}); + assert(!allocation_counter.check_then_reset()); + flat_map fmap1(less{}, MyAllocator{}); + assert(!allocation_counter.check_then_reset()); + + assert(check_key_content(fmap, {})); + assert(check_value_content(fmap, {})); + assert(fmap == fmap1); + } + { + MyAllocatorCounter allocation_counter; + flat_multimap fmmap(MyAllocator{}); + assert(!allocation_counter.check_then_reset()); + flat_multimap fmmap1(less{}, MyAllocator{}); + assert(!allocation_counter.check_then_reset()); + + assert(check_key_content(fmmap, {})); + assert(check_value_content(fmmap, {})); + assert(fmmap == fmmap1); + } + } + { KeyCont keys = {0, 1, 2, 3, 4, 2}; MappedCont vals = {44, 2324, 635462, 433, 5, 7}; + + // Test flat_map(key_cont, mapped_cont, comp = key_comp()) { flat_map fmap(keys, vals); flat_map fmap1(keys, vals, less{}); @@ -302,12 +330,39 @@ void test_construction() { })); assert(fmmap == fmmap1); } + + // Test flat_map(const flat_map&) + // and flat_map(flat_map&&) + { + flat_map fmap(keys, vals); + flat_map fmap1(fmap); + flat_map fmap2(move(fmap)); + + assert(check_key_content(fmap1, {0, 1, 2, 3, 4})); + assert(check_value_content(fmap1, {44, 2324, 635462, 433, 5})); + assert(fmap1 == fmap2); + } + { + flat_multimap fmmap(keys, vals); + flat_multimap fmmap1(fmmap); + flat_multimap fmmap2(move(fmmap)); + + assert(check_key_content(fmmap1, {0, 1, 2, 2, 3, 4})); + assert(check_value_content(fmmap1, {44, 2324, 635462, 7, 433, 5}, + { + {0, 1, subrange_type::equal}, + {2, 3, subrange_type::permutation}, + {4, 5, subrange_type::equal}, + })); + assert(fmmap1 == fmmap2); + } } { - // Test flat_map(const key_cont&, const mapped_cont&, const key_comp&, const alloc&) - // and flat_map(const key_cont&, const mapped_cont&, const alloc&) KeyCont> keys = {0, 1, 2, 3, 4, 2}; MappedCont> vals = {44, 2324, 635462, 433, 5, 7}; + + // Test flat_map(const key_cont&, const mapped_cont&, const key_comp&, const alloc&) + // and flat_map(const key_cont&, const mapped_cont&, const alloc&) { MyAllocatorCounter allocation_counter; flat_map fmap(keys, vals, MyAllocator{}); @@ -335,6 +390,40 @@ void test_construction() { })); assert(fmmap == fmmap1); } + + // Test flat_map(const flat_map&, const alloc&) + // and flat_map(flat_map&&, const alloc&) + { + MyAllocatorCounter allocation_counter; + flat_map fmap(keys, vals, MyAllocator{}); + assert(allocation_counter.check_then_reset()); + flat_map fmap1(fmap); + assert(allocation_counter.check_then_reset()); + flat_map fmap2(move(fmap)); + assert(!allocation_counter.check_then_reset()); + + assert(check_key_content(fmap1, {0, 1, 2, 3, 4})); + assert(check_value_content(fmap1, {44, 2324, 635462, 433, 5})); + assert(fmap1 == fmap2); + } + { + MyAllocatorCounter allocation_counter; + flat_multimap fmmap(keys, vals, MyAllocator{}); + assert(allocation_counter.check_then_reset()); + flat_multimap fmmap1(fmmap); + assert(allocation_counter.check_then_reset()); + flat_multimap fmmap2(move(fmmap)); + assert(!allocation_counter.check_then_reset()); + + assert(check_key_content(fmmap1, {0, 1, 2, 2, 3, 4})); + assert(check_value_content(fmmap1, {44, 2324, 635462, 7, 433, 5}, + { + {0, 1, subrange_type::equal}, + {2, 3, subrange_type::permutation}, + {4, 5, subrange_type::equal}, + })); + assert(fmmap1 == fmmap2); + } } { // Test flat_map(_Sorted_t, key_cont, mapped_cont, comp = key_comp()) @@ -667,7 +756,6 @@ void test_construction() { assert(fmmap == fmmap1); } } - // FIXME, verify that all flat_map and flat_multimap constructors are tested { PackagedCompare comp; {