From 536c00d978cb286e39f67dc13f2dec77f9f20828 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Mon, 25 Sep 2023 01:01:18 +0800 Subject: [PATCH 01/11] 1. nitpicks --- 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 d39368763f7..b2fee2338f6 100644 --- a/stl/inc/flat_set +++ b/stl/inc/flat_set @@ -339,11 +339,11 @@ public: } _NODISCARD size_type count(const _Kty& _Val) const { - if constexpr (!_Multi) { - return contains(_Val); - } else { + if constexpr (_Multi) { const auto [_First, _Last] = equal_range(_Val); return static_cast(_Last - _First); + } else { + return contains(_Val); } } template @@ -730,7 +730,7 @@ _EXPORT_STD template _Container::size_type erase_if(flat_set<_Kty, _Keylt, _Container>& _Val, _Pred _Predicate) { // clears the container to maintain the invariants when an exception is thrown (N4950 [flat.set.erasure]/5) _Clear_scope_guard> _Guard{_STD addressof(_Val)}; - const auto _Erased_count = _Erase_remove_if(_Val, _Pass_fn(_Predicate)); + const auto _Erased_count = _STD _Erase_remove_if(_Val, _STD _Pass_fn(_Predicate)); _Guard._Clearable = nullptr; return _Erased_count; } @@ -739,7 +739,7 @@ _EXPORT_STD template _Container::size_type erase_if(flat_multiset<_Kty, _Keylt, _Container>& _Val, _Pred _Predicate) { // clears the container to maintain the invariants when an exception is thrown (N4950 [flat.multiset.erasure]/5) _Clear_scope_guard> _Guard{_STD addressof(_Val)}; - const auto _Erased_count = _Erase_remove_if(_Val, _Pass_fn(_Predicate)); + const auto _Erased_count = _STD _Erase_remove_if(_Val, _STD _Pass_fn(_Predicate)); _Guard._Clearable = nullptr; return _Erased_count; } From 2cb6462ac6b337027911ddefdcc2954069acf16b Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Mon, 25 Sep 2023 02:07:23 +0800 Subject: [PATCH 02/11] 2. in `insert_range`, fall back to standard definition if the container doesn't support `append_range`. --- stl/inc/flat_set | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/stl/inc/flat_set b/stl/inc/flat_set index b2fee2338f6..60bd642f33e 100644 --- a/stl/inc/flat_set +++ b/stl/inc/flat_set @@ -263,7 +263,15 @@ public: template <_Container_compatible_range<_Kty> _Rng> void insert_range(_Rng&& _Range) { const size_type _Old_size = size(); - _Get_cont().append_range(_STD forward<_Rng>(_Range)); + + _Container& _Cont = _Get_cont(); + if constexpr (requires { _Cont.append_range(_STD forward<_Rng>(_Range)); }) { + _Cont.append_range(_STD forward<_Rng>(_Range)); + } else { + for (const auto& _Val : _Range) { + _Cont.insert(_Cont.end(), _Val); + } + } _Restore_invariants_after_insert(_Old_size); } From a251a3e6a723df4fa57059a99eafe0ca5349b694 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Mon, 25 Sep 2023 12:39:23 +0800 Subject: [PATCH 03/11] 3. though not necessary for correctness, add `_Different_from` to let `foo(auto&&)` not interfere with `foo(key)`. --- stl/inc/flat_set | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/stl/inc/flat_set b/stl/inc/flat_set index 60bd642f33e..ede1e5467cd 100644 --- a/stl/inc/flat_set +++ b/stl/inc/flat_set @@ -228,25 +228,25 @@ public: } } - auto insert(const value_type& _Val) { + auto insert(const _Kty& _Val) { return _Emplace(_Val); } - auto insert(value_type&& _Val) { + auto insert(_Kty&& _Val) { return _Emplace(_STD move(_Val)); } - template + template <_Different_from<_Kty> _Other> requires (!_Multi && _Keylt_transparent && is_constructible_v<_Kty, _Other>) auto insert(_Other&& _Val) { return _Emplace(_STD forward<_Other>(_Val)); } - iterator insert(const_iterator _Hint, const value_type& _Val) { + iterator insert(const_iterator _Hint, const _Kty& _Val) { return _Emplace_hint(_Hint, _Val); } - iterator insert(const_iterator _Hint, value_type&& _Val) { + iterator insert(const_iterator _Hint, _Kty&& _Val) { return _Emplace_hint(_Hint, _STD move(_Val)); } - template + template <_Different_from<_Kty> _Other> requires (!_Multi && _Keylt_transparent && is_constructible_v<_Kty, _Other>) iterator insert(const_iterator _Hint, _Other&& _Val) { return _Emplace_hint(_Hint, _STD forward<_Other>(_Val)); @@ -301,7 +301,7 @@ public: size_type erase(const _Kty& _Val) { return _Erase(_Val); } - template + template <_Different_from<_Kty> _Other> requires ( _Keylt_transparent && !is_convertible_v<_Other, iterator> && !is_convertible_v<_Other, const_iterator>) size_type erase(_Other&& _Val) { From bf6b06a274e157345d6ed83914ac0829953cc8ec Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Mon, 25 Sep 2023 15:26:01 +0800 Subject: [PATCH 04/11] 4.1. update debug methods --- stl/inc/flat_set | 52 +++++++++++++++++++++--------------------------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/stl/inc/flat_set b/stl/inc/flat_set index ede1e5467cd..0047e7b6ccd 100644 --- a/stl/inc/flat_set +++ b/stl/inc/flat_set @@ -86,7 +86,7 @@ public: _Base_flat_set(_Tsorted, container_type _Cont, const key_compare& _Comp = key_compare()) : _My_pair(_One_then_variadic_args_t{}, _Comp, _STD move(_Cont)) { - _Assert_after_sorted_input(); + _STL_ASSERT(_Check_sorted(cbegin(), cend()), _Msg_not_sorted); } template <_Allocator_for _Alloc> _Base_flat_set(_Tsorted _Tsort, const container_type& _Cont, const _Alloc& _Al) @@ -288,8 +288,8 @@ public: return _STD move(_Get_cont()); } void replace(container_type&& _Cont) { + _STL_ASSERT(_Check_sorted(_Cont.cbegin(), _Cont.cend()), _Msg_not_sorted); _Get_cont() = _STD move(_Cont); - _Assert_after_sorted_input(); } iterator erase(iterator _Where) { @@ -438,27 +438,25 @@ public: } private: - void _Assert_after_sorted_input() const { - _STL_ASSERT(_STD is_sorted(cbegin(), cend(), _Get_comp_v()), "Input was not sorted!"); - if constexpr (!_Multi) { - _STL_ASSERT(_Is_unique(), "Input was sorted but not unique!"); - } - } - - bool _Is_unique() const { - if (empty()) { - return true; - } - const const_iterator _End = cend(); - const_iterator _It = cbegin(); - while (++_It != _End) { - if (_Keys_equal(*(_It - 1), *_It)) { - return false; + bool _Check_sorted(const_iterator _It, const const_iterator _End) const { + if (_Multi) { + return _STD is_sorted(_It, _End, _Get_comp_v()); + } else { + // sorted-unique + if (_It == _End) { + return true; } + while (++_It != _End) { + if (!_Compare(*(_It - 1), *_It)) { + return false; + } + } + return true; } - return true; } + static constexpr const char* _Msg_not_sorted = _Multi ? "Input was not sorted!" : "Input was not sorted-unique!"; + bool _Check_where(const const_iterator _Where, const _Kty& _Val) const { // check that _Val can be inserted before _Where if constexpr (_Multi) { @@ -612,14 +610,12 @@ private: } } - void _Erase_dupes_if_needed() { + void _Erase_dupes_if_not_multi() { if constexpr (!_Multi) { const iterator _End = end(); const iterator _New_end = _STD unique(begin(), _End, [&](const _Kty& _Lhs, const _Kty& _Rhs) { return _Keys_equal(_Lhs, _Rhs); }); _Get_cont().erase(_New_end, _End); - - _STL_INTERNAL_CHECK(_Is_unique()); } } @@ -633,14 +629,13 @@ private: if constexpr (!_Presorted) { _STD sort(_Old_end, _New_end, _Comp); } else { - _STL_ASSERT(_STD is_sorted(_Old_end, _New_end, _Comp), "Input was not sorted!"); + _STL_ASSERT(_Check_sorted(_Old_end, _New_end), _Msg_not_sorted); } _STD inplace_merge(_Begin, _Old_end, _New_end, _Comp); + _Erase_dupes_if_not_multi(); - _STL_INTERNAL_CHECK(_STD is_sorted(_Begin, _New_end, _Comp)); - - _Erase_dupes_if_needed(); + _STL_INTERNAL_CHECK(_Check_sorted(cbegin(), cend())); } void _Make_invariants_fulfilled() { @@ -657,10 +652,9 @@ private: _STD sort(_Begin_unsorted, _End, _Comp); _STD inplace_merge(_Begin, _Begin_unsorted, _End, _Comp); + _Erase_dupes_if_not_multi(); - _STL_INTERNAL_CHECK(_STD is_sorted(_Begin, _End, _Comp)); - - _Erase_dupes_if_needed(); + _STL_INTERNAL_CHECK(_Check_sorted(cbegin(), cend())); } template From cfde52b989818bc426d741721ba0d095cb73db95 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Mon, 25 Sep 2023 15:27:45 +0800 Subject: [PATCH 05/11] 4.2. remove `_Keys_equal` method. --- stl/inc/flat_set | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/stl/inc/flat_set b/stl/inc/flat_set index 0047e7b6ccd..107a2b3480e 100644 --- a/stl/inc/flat_set +++ b/stl/inc/flat_set @@ -612,13 +612,16 @@ private: void _Erase_dupes_if_not_multi() { if constexpr (!_Multi) { - const iterator _End = end(); - const iterator _New_end = - _STD unique(begin(), _End, [&](const _Kty& _Lhs, const _Kty& _Rhs) { return _Keys_equal(_Lhs, _Rhs); }); + const auto _Equal_to = [this](const _Kty& _Lhs, const _Kty& _Rhs) { + return !_Compare(_Lhs, _Rhs) && !_Compare(_Rhs, _Lhs); + }; + const iterator _End = end(); + const iterator _New_end = _STD unique(begin(), _End, _Equal_to); _Get_cont().erase(_New_end, _End); } } + template void _Restore_invariants_after_insert(const size_type _Old_size) { auto _Comp = _Get_comp_v(); @@ -665,14 +668,6 @@ private: return _DEBUG_LT_PRED(_My_pair._Get_first(), _Lhs, _Rhs); } - template - _NODISCARD bool _Keys_equal(const _Lty& _Lhs, const _Rty& _Rhs) const - noexcept(noexcept(!_Compare(_Lhs, _Rhs) && !_Compare(_Rhs, _Lhs))) { - _STL_INTERNAL_STATIC_ASSERT(_Keylt_transparent || (is_same_v<_Kty, _Lty> && is_same_v<_Kty, _Rty>) ); - - return !_Compare(_Lhs, _Rhs) && !_Compare(_Rhs, _Lhs); - } - _NODISCARD const _Container& _Get_cont() const noexcept { return _My_pair._Myval2; } From d7e8768d16bc275d6e577c51bfadf987ea5369ad Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Mon, 25 Sep 2023 17:46:26 +0800 Subject: [PATCH 06/11] 5. add test coverages --- tests/std/tests/P1222R4_flat_set/test.cpp | 109 +++++++++++++++++++++- 1 file changed, 107 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/P1222R4_flat_set/test.cpp b/tests/std/tests/P1222R4_flat_set/test.cpp index 7422f959fca..ab31a7cbea1 100644 --- a/tests/std/tests/P1222R4_flat_set/test.cpp +++ b/tests/std/tests/P1222R4_flat_set/test.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include using namespace std; @@ -235,6 +236,107 @@ void test_insert_2() { } } +struct val_comparer { + const auto& extract_key(const auto& obj) const { + if constexpr (requires { void(obj.val); }) { + return obj.val; + } else { + return obj; + } + } + + bool operator()(const auto& lhs, const auto& rhs) const { + return extract_key(lhs) < extract_key(rhs); + } + + using is_transparent = int; +}; + +void test_comparer_application() { + struct int_holder { + int val; + bool operator<(const int_holder&) const = delete; + bool operator==(const int_holder&) const = delete; + }; + + flat_set fs{{0}, {3}, {1}, {0}, {5}}; + assert(fs.contains(0)); + assert(!fs.contains(2)); + fs.insert(fs.begin(), int_holder{4}); + fs.insert(2); + assert(fs.contains(4)); + assert(fs.contains(int_holder{2})); + + assert(fs.lower_bound(3) == fs.lower_bound(int_holder{3})); + fs.erase(2); + assert(!fs.contains(int_holder{2})); +} + +void test_insert_3() { + // test that flat_set::insert(K&&) doesn't modify input for failed insertion. + struct int_holder { + int val; + mutable bool converted = false; + + explicit operator int() const { + converted = true; + return val; + } + }; + + flat_set fs{0, 3, 5}; + + assert_all_requirements_and_equals(fs, {0, 3, 5}); + + int_holder holder{3}; + assert(!holder.converted); + + fs.insert(holder); + assert(!holder.converted); + assert_all_requirements_and_equals(fs, {0, 3, 5}); + + holder.val = 1; + fs.insert(holder); + assert(holder.converted); + assert_all_requirements_and_equals(fs, {0, 1, 3, 5}); +} + +void test_insert_4() { + // test that hinted insertion is robust against invalid hints. + mt19937 eng(42); + + uniform_int_distribution dist_seq(0, 20); + + vector seq(200); + for (int& val : seq) { + val = dist_seq(eng); + } + + { + flat_multiset with_hint, no_hint; + for (const int val : seq) { + uniform_int_distribution dist_idx(0, int(with_hint.size())); + auto random_hint = with_hint.begin() + dist_idx(eng); + with_hint.insert(random_hint, val); + no_hint.insert(val); + } + + assert(with_hint == no_hint); + } + + { + flat_set with_hint, no_hint; + for (const int val : seq) { + uniform_int_distribution dist_idx(0, int(with_hint.size())); + auto random_hint = with_hint.begin() + dist_idx(eng); + with_hint.insert(random_hint, val); + no_hint.insert(val); + } + + assert(with_hint == no_hint); + } +} + template void test_spaceship_operator() { static constexpr bool multi = _Is_specialization_v; @@ -342,8 +444,8 @@ void test_count() { flat_set fs{2}; assert(fs.count(1) == 0); - flat_multiset fs2{1, 2, 2, 3}; - assert(fs2.count(2) == 2); + flat_multiset fs2{10, 20, 20, 30}; + assert(fs2.count(20) == 2); } int main() { @@ -363,7 +465,10 @@ int main() { test_insert_1>(); test_insert_2>(); test_insert_2>(); + test_insert_3(); + test_insert_4(); + test_comparer_application(); test_non_static_comparer(); test_extract>(); From 4ca5b9b647dadff719ef27d1ab43d7c9f3a4a4ff Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Mon, 25 Sep 2023 17:47:18 +0800 Subject: [PATCH 07/11] 6. fix for `test_comparer_application` --- stl/inc/flat_set | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/flat_set b/stl/inc/flat_set index 107a2b3480e..38476220071 100644 --- a/stl/inc/flat_set +++ b/stl/inc/flat_set @@ -428,7 +428,7 @@ public: return _RANGES equal(_Lhs._Get_cont(), _Rhs._Get_cont()); } - _NODISCARD friend _Synth_three_way_result<_Kty> operator<=>(const _Deriv& _Lhs, const _Deriv& _Rhs) { + _NODISCARD friend auto operator<=>(const _Deriv& _Lhs, const _Deriv& _Rhs) { return _STD lexicographical_compare_three_way( _Lhs.cbegin(), _Lhs.cend(), _Rhs.cbegin(), _Rhs.cend(), _Synth_three_way{}); } From b9be81842c1cfe1c323d40f90784bdd9dcd8b773 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Tue, 26 Sep 2023 14:11:26 +0800 Subject: [PATCH 08/11] refine tests --- stl/inc/flat_set | 1 - tests/std/tests/P1222R4_flat_set/test.cpp | 75 ++++++++++++++--------- 2 files changed, 45 insertions(+), 31 deletions(-) diff --git a/stl/inc/flat_set b/stl/inc/flat_set index 38476220071..26c2d253617 100644 --- a/stl/inc/flat_set +++ b/stl/inc/flat_set @@ -621,7 +621,6 @@ private: } } - template void _Restore_invariants_after_insert(const size_type _Old_size) { auto _Comp = _Get_comp_v(); diff --git a/tests/std/tests/P1222R4_flat_set/test.cpp b/tests/std/tests/P1222R4_flat_set/test.cpp index ab31a7cbea1..e103248cac0 100644 --- a/tests/std/tests/P1222R4_flat_set/test.cpp +++ b/tests/std/tests/P1222R4_flat_set/test.cpp @@ -236,10 +236,10 @@ void test_insert_2() { } } -struct val_comparer { +struct key_comparer { const auto& extract_key(const auto& obj) const { - if constexpr (requires { void(obj.val); }) { - return obj.val; + if constexpr (requires { obj.key; }) { + return obj.key; } else { return obj; } @@ -253,56 +253,71 @@ struct val_comparer { }; void test_comparer_application() { - struct int_holder { - int val; - bool operator<(const int_holder&) const = delete; - bool operator==(const int_holder&) const = delete; + // The set must rely on its comparer to do the comparisons. + struct incomparable { + int key; + bool operator<(const incomparable&) const = delete; + bool operator==(const incomparable&) const = delete; }; - flat_set fs{{0}, {3}, {1}, {0}, {5}}; + flat_set fs{{0}, {3}, {1}, {0}, {5}}; assert(fs.contains(0)); assert(!fs.contains(2)); - fs.insert(fs.begin(), int_holder{4}); + fs.insert(fs.begin(), incomparable{4}); fs.insert(2); assert(fs.contains(4)); - assert(fs.contains(int_holder{2})); + assert(fs.contains(incomparable{2})); - assert(fs.lower_bound(3) == fs.lower_bound(int_holder{3})); + assert(fs.lower_bound(3) == fs.lower_bound(incomparable{3})); fs.erase(2); - assert(!fs.contains(int_holder{2})); + assert(!fs.contains(incomparable{2})); } -void test_insert_3() { - // test that flat_set::insert(K&&) doesn't modify input for failed insertion. - struct int_holder { - int val; +void test_insert_transparent() { + // For flat_set::insert([hint,]auto&&), the input should be unchanged if the set already + // contains an equivalent element. + struct detect_conversion { + int key; mutable bool converted = false; explicit operator int() const { converted = true; - return val; + return key; } }; - flat_set fs{0, 3, 5}; + flat_set fs{0, 3, 5}; + assert_all_requirements_and_equals(fs, {0, 3, 5}); + detect_conversion detector{3}; + assert(!detector.converted); + fs.insert(detector /*3*/); assert_all_requirements_and_equals(fs, {0, 3, 5}); + assert(!detector.converted); - int_holder holder{3}; - assert(!holder.converted); + detector.key = 1; - fs.insert(holder); - assert(!holder.converted); - assert_all_requirements_and_equals(fs, {0, 3, 5}); + assert(!detector.converted); + fs.insert(detector /*1*/); + assert_all_requirements_and_equals(fs, {0, 1, 3, 5}); + assert(detector.converted); - holder.val = 1; - fs.insert(holder); - assert(holder.converted); + detector.converted = false; + + assert(!detector.converted); + fs.insert(fs.end(), detector /*1*/); assert_all_requirements_and_equals(fs, {0, 1, 3, 5}); + assert(!detector.converted); + + detector.key = 2; + + assert(!detector.converted); + fs.insert(fs.begin(), detector /*2*/); + assert_all_requirements_and_equals(fs, {0, 1, 2, 3, 5}); + assert(detector.converted); } -void test_insert_4() { - // test that hinted insertion is robust against invalid hints. +void test_insert_using_invalid_hint() { mt19937 eng(42); uniform_int_distribution dist_seq(0, 20); @@ -465,8 +480,8 @@ int main() { test_insert_1>(); test_insert_2>(); test_insert_2>(); - test_insert_3(); - test_insert_4(); + test_insert_transparent(); + test_insert_using_invalid_hint(); test_comparer_application(); test_non_static_comparer(); From fa6c030ec00d69551145e7416ab14bfc6a7bfed0 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Tue, 26 Sep 2023 17:55:13 +0800 Subject: [PATCH 09/11] 7. update scope guard; works towards exception-conformance --- stl/inc/flat_set | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/stl/inc/flat_set b/stl/inc/flat_set index 26c2d253617..0aa386f4c7c 100644 --- a/stl/inc/flat_set +++ b/stl/inc/flat_set @@ -25,16 +25,21 @@ _STL_DISABLE_CLANG_WARNINGS _STD_BEGIN -template -struct _NODISCARD _Clear_scope_guard { - _Ty* _Clearable; - ~_Clear_scope_guard() { - if (_Clearable) { - _Clearable->clear(); +template +struct _NODISCARD _Clear_guard { + _Ty* _Target; + ~_Clear_guard() { + if (_Target) [[unlikely]] { + _Target->clear(); } } }; +template +struct [[maybe_unused]] _NODISCARD _Clear_guard<_Ty, true> { + _Ty* _Target; // no effect if the guarded operations don't throw. +}; + template concept _Allocator_for = uses_allocator_v<_Container, _Alloc>; @@ -152,8 +157,10 @@ public: : _Base_flat_set(_Tsort, container_type(_Ilist.begin(), _Ilist.end(), _Al)) {} _Deriv& operator=(initializer_list<_Kty> _Ilist) { + _Clear_guard<_Base_flat_set> _Guard{this}; _Get_cont().assign(_Ilist.begin(), _Ilist.end()); _Make_invariants_fulfilled(); + _Guard._Target = nullptr; return static_cast<_Deriv&>(*this); } @@ -282,14 +289,16 @@ public: _Insert_range(_Ilist.begin(), _Ilist.end()); } - _NODISCARD container_type extract() && { + _NODISCARD container_type extract() && noexcept(is_nothrow_move_constructible_v<_Container>) /* strengthened */ { // always clears the container (N4950 [flat.set.modifiers]/14 and [flat.multiset.modifiers]/10) - _Clear_scope_guard<_Base_flat_set> _Guard{this}; + _Clear_guard<_Base_flat_set, is_nothrow_move_constructible_v<_Container>> _Guard{this}; return _STD move(_Get_cont()); } void replace(container_type&& _Cont) { _STL_ASSERT(_Check_sorted(_Cont.cbegin(), _Cont.cend()), _Msg_not_sorted); - _Get_cont() = _STD move(_Cont); + _Clear_guard<_Base_flat_set, is_nothrow_move_assignable_v<_Container>> _Guard{this}; + _Get_cont() = _STD move(_Cont); + _Guard._Target = nullptr; } iterator erase(iterator _Where) { @@ -725,18 +734,18 @@ public: _EXPORT_STD template _Container::size_type erase_if(flat_set<_Kty, _Keylt, _Container>& _Val, _Pred _Predicate) { // clears the container to maintain the invariants when an exception is thrown (N4950 [flat.set.erasure]/5) - _Clear_scope_guard> _Guard{_STD addressof(_Val)}; + _Clear_guard> _Guard{_STD addressof(_Val)}; const auto _Erased_count = _STD _Erase_remove_if(_Val, _STD _Pass_fn(_Predicate)); - _Guard._Clearable = nullptr; + _Guard._Target = nullptr; return _Erased_count; } _EXPORT_STD template _Container::size_type erase_if(flat_multiset<_Kty, _Keylt, _Container>& _Val, _Pred _Predicate) { // clears the container to maintain the invariants when an exception is thrown (N4950 [flat.multiset.erasure]/5) - _Clear_scope_guard> _Guard{_STD addressof(_Val)}; + _Clear_guard> _Guard{_STD addressof(_Val)}; const auto _Erased_count = _STD _Erase_remove_if(_Val, _STD _Pass_fn(_Predicate)); - _Guard._Clearable = nullptr; + _Guard._Target = nullptr; return _Erased_count; } From 9f2e612882c890a8874813310ee709866364c00a Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Tue, 26 Sep 2023 22:40:53 +0800 Subject: [PATCH 10/11] nitpick --- stl/inc/flat_set | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/flat_set b/stl/inc/flat_set index 0aa386f4c7c..f320fc103d1 100644 --- a/stl/inc/flat_set +++ b/stl/inc/flat_set @@ -29,7 +29,7 @@ template struct _NODISCARD _Clear_guard { _Ty* _Target; ~_Clear_guard() { - if (_Target) [[unlikely]] { + if (_Target) { _Target->clear(); } } @@ -37,7 +37,7 @@ struct _NODISCARD _Clear_guard { template struct [[maybe_unused]] _NODISCARD _Clear_guard<_Ty, true> { - _Ty* _Target; // no effect if the guarded operations don't throw. + _Ty* _Target; // do nothing as the guarded operations don't throw. }; template From 6bcbbe60df4c4d047eb57b829e54c82e02cd423e Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 26 Sep 2023 18:41:01 -0700 Subject: [PATCH 11/11] Code review feedback. --- stl/inc/flat_set | 6 +++--- tests/std/tests/P1222R4_flat_set/test.cpp | 10 ++++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/stl/inc/flat_set b/stl/inc/flat_set index f320fc103d1..1efe93be36c 100644 --- a/stl/inc/flat_set +++ b/stl/inc/flat_set @@ -447,8 +447,8 @@ public: } private: - bool _Check_sorted(const_iterator _It, const const_iterator _End) const { - if (_Multi) { + _NODISCARD bool _Check_sorted(const_iterator _It, const const_iterator _End) const { + if constexpr (_Multi) { return _STD is_sorted(_It, _End, _Get_comp_v()); } else { // sorted-unique @@ -466,7 +466,7 @@ private: static constexpr const char* _Msg_not_sorted = _Multi ? "Input was not sorted!" : "Input was not sorted-unique!"; - bool _Check_where(const const_iterator _Where, const _Kty& _Val) const { + _NODISCARD bool _Check_where(const const_iterator _Where, const _Kty& _Val) const { // check that _Val can be inserted before _Where if constexpr (_Multi) { // check that _Where is the upper_bound for _Val diff --git a/tests/std/tests/P1222R4_flat_set/test.cpp b/tests/std/tests/P1222R4_flat_set/test.cpp index e103248cac0..9bb2adcd0df 100644 --- a/tests/std/tests/P1222R4_flat_set/test.cpp +++ b/tests/std/tests/P1222R4_flat_set/test.cpp @@ -328,9 +328,10 @@ void test_insert_using_invalid_hint() { } { - flat_multiset with_hint, no_hint; + flat_multiset with_hint; + flat_multiset no_hint; for (const int val : seq) { - uniform_int_distribution dist_idx(0, int(with_hint.size())); + uniform_int_distribution dist_idx(0, static_cast(with_hint.size())); auto random_hint = with_hint.begin() + dist_idx(eng); with_hint.insert(random_hint, val); no_hint.insert(val); @@ -340,9 +341,10 @@ void test_insert_using_invalid_hint() { } { - flat_set with_hint, no_hint; + flat_set with_hint; + flat_set no_hint; for (const int val : seq) { - uniform_int_distribution dist_idx(0, int(with_hint.size())); + uniform_int_distribution dist_idx(0, static_cast(with_hint.size())); auto random_hint = with_hint.begin() + dist_idx(eng); with_hint.insert(random_hint, val); no_hint.insert(val);