From 4daedd0de260ee0f0cdb96d4dea31322e6c8206e Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 12 Jul 2024 01:44:33 +0800 Subject: [PATCH 1/3] Fix insertion order error Per WG21-N4981 [associative.reqmts.general]/68 and /72. --- stl/inc/flat_map | 24 +++++++++++++++-------- tests/std/tests/P0429R9_flat_map/test.cpp | 6 ++---- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/stl/inc/flat_map b/stl/inc/flat_map index 16ca80fd831..c8acb0ea1d5 100644 --- a/stl/inc/flat_map +++ b/stl/inc/flat_map @@ -830,20 +830,28 @@ protected: const const_iterator _Begin = cbegin(); const const_iterator _End = cend(); - const bool _Hint_is_accurate = [&] { + const weak_ordering _Hint_order = [&] { if constexpr (_IsUnique) { if (_Position == _End || _Key_compare(_Key_val, *_Position._Key_it)) { - return _Position == _Begin || _Key_compare(_Position._Key_it[-1], _Key_val); + if (_Position == _Begin || _Key_compare(_Position._Key_it[-1], _Key_val)) { + return weak_ordering::equivalent; + } else { + return weak_ordering::greater; + } } } else { if (_Position == _End || !_Key_compare(*_Position._Key_it, _Key_val)) { - return _Position == _Begin || !_Key_compare(_Key_val, _Position._Key_it[-1]); + if (_Position == _Begin || !_Key_compare(_Key_val, _Position._Key_it[-1])) { + return weak_ordering::equivalent; + } else { + return weak_ordering::greater; + } } } - return false; + return weak_ordering::less; }(); - if (_Hint_is_accurate) { + if (_Hint_order == weak_ordering::equivalent) { const auto _Dist = _Position._Key_it - _Begin._Key_it; { key_type _Key_to_insert(_STD forward<_OtherKey>(_Key_val)); @@ -861,7 +869,7 @@ protected: return _It; } - _Position = lower_bound(_Key_val); + _Position = _Hint_order == weak_ordering::less ? lower_bound(_Key_val) : upper_bound(_Key_val); if (_Position != _End && !_Key_compare(_Key_val, *_Position._Key_it)) { const auto _Dist = _Position._Key_it - _Begin._Key_it; auto _It = begin() + _Dist; @@ -869,7 +877,7 @@ protected: return _It; } } else { - _Position = lower_bound(_Key_val); + _Position = _Hint_order == weak_ordering::less ? lower_bound(_Key_val) : upper_bound(_Key_val); } const auto _Dist = _Position._Key_it - _Begin._Key_it; @@ -1428,7 +1436,7 @@ private: template iterator _Emplace_key_mapped(_KeyTy&& _Key_val, _MappedTy&& _Mapped_val) { - const auto _Key_it = _STD lower_bound(_Data.keys.begin(), _Data.keys.end(), _Key_val, _Key_compare); + const auto _Key_it = _STD upper_bound(_Data.keys.begin(), _Data.keys.end(), _Key_val, _Key_compare); const auto _Index = _Key_it - _Data.keys.begin(); { diff --git a/tests/std/tests/P0429R9_flat_map/test.cpp b/tests/std/tests/P0429R9_flat_map/test.cpp index a2093294c4a..9bea8ee487e 100644 --- a/tests/std/tests/P0429R9_flat_map/test.cpp +++ b/tests/std/tests/P0429R9_flat_map/test.cpp @@ -453,10 +453,8 @@ void test_insert() { assert(it5->second == 'q'); assert(check_key_content(fmm, {10, 10, 70, 70, 90})); - // FIXME: The expected order of the last 3 value elements is 'p', 'q', 'w' (no permutation allowed), according to - // N4981 [associative.reqmts.general]/72. The current implementation gives 'q', 'p', 'w', which is wrong. - assert(check_value_content(fmm, {'n', 'm', 'p', 'q', 'w'}, - {{0, 1, subrange_type::permutation}, {2, 3, subrange_type::permutation}, {4, 4, subrange_type::equal}})); + // N4981 [associative.reqmts.general]/68 and /72 specify the values of the result to be {'m', 'n', 'p', 'q', 'w'}. + assert(check_value_content(fmm, {'m', 'n', 'p', 'q', 'w'})); } // GH-4344 Fix compile errors From b9e30d1f30eb6693c6ca76103c949a64a545a809 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 12 Jul 2024 01:45:29 +0800 Subject: [PATCH 2/3] Fix access control error --- stl/inc/flat_map | 23 +++++++++++++++-------- tests/std/tests/P0429R9_flat_map/test.cpp | 18 ++++++++++++++++++ 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/stl/inc/flat_map b/stl/inc/flat_map index c8acb0ea1d5..330eb2d638a 100644 --- a/stl/inc/flat_map +++ b/stl/inc/flat_map @@ -459,27 +459,34 @@ public: : _Flat_map_base(_Tag, _Ilist, key_compare(), _Alloc) {} // Copy constructors - _Flat_map_base(const _Derived& _Other) : _Key_compare(_Other._Key_compare), _Data(_Other._Data) {} + _Flat_map_base(const _Derived& _Other) + : _Key_compare(static_cast(_Other)._Key_compare), + _Data(static_cast(_Other)._Data) {} template <_Valid_allocator_for_flat_map _Allocator> _Flat_map_base(const _Derived& _Other, const _Allocator& _Alloc) - : _Key_compare(_Other._Key_compare), - _Data{.keys = _STD make_obj_using_allocator(_Alloc, _Other._Data.keys), - .values = _STD make_obj_using_allocator(_Alloc, _Other._Data.values)} {} + : _Key_compare(static_cast(_Other)._Key_compare), + _Data{.keys = _STD make_obj_using_allocator( + _Alloc, static_cast(_Other)._Data.keys), + .values = _STD make_obj_using_allocator( + _Alloc, static_cast(_Other)._Data.values)} {} // Move constructors _Flat_map_base(_Derived&& _Other) noexcept(is_nothrow_move_constructible_v && is_nothrow_move_constructible_v && is_nothrow_move_constructible_v) - : _Key_compare(move(_Other._Key_compare)), _Data(move(_Other).extract()) {} + : _Key_compare(_STD move(static_cast<_Flat_map_base&>(_Other)._Key_compare)), + _Data(_STD move(static_cast<_Flat_map_base&>(_Other).extract())) {} template <_Valid_allocator_for_flat_map _Allocator> _Flat_map_base(_Derived&& _Other, const _Allocator& _Alloc) noexcept( is_nothrow_move_constructible_v && is_nothrow_move_constructible_v && is_nothrow_move_constructible_v) - : _Key_compare(move(_Other._Key_compare)), - _Data{.keys = _STD make_obj_using_allocator(_Alloc, move(_Other._Data.keys)), - .values = _STD make_obj_using_allocator(_Alloc, move(_Other._Data.values))} {} + : _Key_compare(_STD move(static_cast<_Flat_map_base&>(_Other)._Key_compare)), + _Data{.keys = _STD make_obj_using_allocator( + _Alloc, _STD move(static_cast<_Flat_map_base&>(_Other)._Data.keys)), + .values = _STD make_obj_using_allocator( + _Alloc, _STD move(static_cast<_Flat_map_base&>(_Other)._Data.values))} {} _Derived& operator=(initializer_list _Ilist) { clear(); diff --git a/tests/std/tests/P0429R9_flat_map/test.cpp b/tests/std/tests/P0429R9_flat_map/test.cpp index 9bea8ee487e..977844600b5 100644 --- a/tests/std/tests/P0429R9_flat_map/test.cpp +++ b/tests/std/tests/P0429R9_flat_map/test.cpp @@ -357,6 +357,24 @@ void test_construction() { {4, 5, subrange_type::equal}, })); } + // Test GH-4779 (access from _Flat_map_base to derived flat_map/flat_multimap) + { + allocator ator; + + using fm = flat_map, vector, vector>; + fm m0; + fm m1(m0); + fm m2(m0, ator); + fm m3(move(m0)); + fm m4(move(m1), ator); + + using fmm = flat_multimap, vector, vector>; + fmm mm0; + fmm mm1(mm0); + fmm mm2(mm0, ator); + fmm mm3(move(mm0)); + fmm mm4(move(mm1), ator); + } } void test_erase_if() { From ad46423daa119beef2719eb4902a5ac5774f547d Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Fri, 12 Jul 2024 01:45:50 +0800 Subject: [PATCH 3/3] Remove the `_NODISCARD_FRIEND` workaround --- stl/inc/flat_map | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/flat_map b/stl/inc/flat_map index 330eb2d638a..c9dfe198f40 100644 --- a/stl/inc/flat_map +++ b/stl/inc/flat_map @@ -787,14 +787,14 @@ public: return _Equal_range(_Key_val); } - _NODISCARD_FRIEND bool operator==(const _Derived& _Left, const _Derived& _Right) { + _NODISCARD friend bool operator==(const _Derived& _Left, const _Derived& _Right) { auto& _Left_base = static_cast(_Left); auto& _Right_base = static_cast(_Right); return _RANGES equal(_Left_base._Data.keys, _Right_base._Data.keys) && _RANGES equal(_Left_base._Data.values, _Right_base._Data.values); } - _NODISCARD_FRIEND auto operator<=>(const _Derived& _Left, const _Derived& _Right) { + _NODISCARD friend auto operator<=>(const _Derived& _Left, const _Derived& _Right) { return _STD lexicographical_compare_three_way(_STD _Get_unwrapped(_Left.cbegin()), _STD _Get_unwrapped(_Left.cend()), _STD _Get_unwrapped(_Right.cbegin()), _STD _Get_unwrapped(_Right.cend()), _Synth_three_way{});