From e758cb295267ae98f3fbb43f0102215d93989f1e Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Sun, 10 Mar 2024 21:39:56 +0800 Subject: [PATCH 1/8] Style cleanup - Eliminate redundant parentheses. - Use `it[-1]` instead of `*(it - 1)`. - Add some `const`. Not yet to iterators due to moving. - Eliminate most `if {/*...*/ return; } else {/*...*/}`, except when returning `end()`/`cend()` in `else`. --- stl/inc/flat_map | 118 +++++++++++++++++++++++------------------------ 1 file changed, 59 insertions(+), 59 deletions(-) diff --git a/stl/inc/flat_map b/stl/inc/flat_map index 5391ac79285..8d4d9acf214 100644 --- a/stl/inc/flat_map +++ b/stl/inc/flat_map @@ -126,7 +126,7 @@ struct _Pairing_iterator_provider { } pointer operator->() const { - return pointer{*(*this)}; + return pointer{**this}; } _Iterator& operator++() { @@ -767,50 +767,50 @@ protected: bool _Insert_before_position = false; bool _Insert_after_position_minus_1 = false; if constexpr (_IsUnique) { - _Insert_before_position = (_Position == _End) || _Key_compare(_Key_val, *(_Position._Key_it)); - _Insert_after_position_minus_1 = (_Position == _Begin) || _Key_compare(*(_Position._Key_it - 1), _Key_val); + _Insert_before_position = _Position == _End || _Key_compare(_Key_val, *_Position._Key_it); + _Insert_after_position_minus_1 = _Position == _Begin || _Key_compare(_Position._Key_it[-1], _Key_val); } else { - _Insert_before_position = (_Position == _End) || !_Key_compare(*(_Position._Key_it), _Key_val); - _Insert_after_position_minus_1 = (_Position == _Begin) || !_Key_compare(_Key_val, *(_Position._Key_it - 1)); + _Insert_before_position = _Position == _End || !_Key_compare(*_Position._Key_it, _Key_val); + _Insert_after_position_minus_1 = _Position == _Begin || !_Key_compare(_Key_val, _Position._Key_it[-1]); } - bool _Hint_is_accurate = _Insert_before_position && _Insert_after_position_minus_1; + const bool _Hint_is_accurate = _Insert_before_position && _Insert_after_position_minus_1; if (_Hint_is_accurate) { - auto _Dist = _STD distance(_Begin._Key_it, _Position._Key_it); + const auto _Dist = _Position._Key_it - _Begin._Key_it; { key_type _Key_to_insert(_STD forward<_OtherKey>(_Key_val)); mapped_type _Mapped_to_insert(_STD forward<_MappedArgTypes>(_Args)...); _Insert_exact(_Position, _STD move(_Key_to_insert), _STD move(_Mapped_to_insert)); } return begin() + _Dist; - } else { - if constexpr (_OverwriteIfExists) { - if (_Key_equal(_Key_val, *(_Position._Key_it))) { - auto _Dist = _STD distance(_Begin._Key_it, _Position._Key_it); - auto _It = begin() + _Dist; - *(_It._Mapped_it) = mapped_type{_STD forward<_MappedArgTypes>(_Args)...}; - return _It; - } - - _Position = lower_bound(_Key_val); - if (_Position != _End && _Key_equal(_Key_val, *(_Position._Key_it))) { - auto _Dist = _STD distance(_Begin._Key_it, _Position._Key_it); - auto _It = begin() + _Dist; - *(_It._Mapped_it) = mapped_type{_STD forward<_MappedArgTypes>(_Args)...}; - return _It; - } - } else { - _Position = lower_bound(_Key_val); + } + + if constexpr (_OverwriteIfExists) { + if (_Key_equal(_Key_val, *_Position._Key_it)) { + const auto _Dist = _Position._Key_it - _Begin._Key_it; + const auto _It = begin() + _Dist; + *_It._Mapped_it = mapped_type{_STD forward<_MappedArgTypes>(_Args)...}; + return _It; } - const auto _Dist = _STD distance(_Begin._Key_it, _Position._Key_it); - { - key_type _Key_to_insert(_STD forward<_OtherKey>(_Key_val)); - mapped_type _Mapped_to_insert(_STD forward<_MappedArgTypes>(_Args)...); - _Insert_exact(_Position, _STD move(_Key_to_insert), _STD move(_Mapped_to_insert)); + _Position = lower_bound(_Key_val); + if (_Position != _End && _Key_equal(_Key_val, *_Position._Key_it)) { + const auto _Dist = _Position._Key_it - _Begin._Key_it; + auto _It = begin() + _Dist; + *_It._Mapped_it = mapped_type{_STD forward<_MappedArgTypes>(_Args)...}; + return _It; } - return begin() + _Dist; + } else { + _Position = lower_bound(_Key_val); } + + const auto _Dist = _Position._Key_it - _Begin._Key_it; + { + key_type _Key_to_insert(_STD forward<_OtherKey>(_Key_val)); + mapped_type _Mapped_to_insert(_STD forward<_MappedArgTypes>(_Args)...); + _Insert_exact(_Position, _STD move(_Key_to_insert), _STD move(_Mapped_to_insert)); + } + return begin() + _Dist; } void _Insert_exact(const_iterator _Position, key_type&& _Key_val, mapped_type&& _Mapped_val) { @@ -850,7 +850,7 @@ private: auto _Subrange = _RANGES unique(_Sorted_view, [this](const_reference _Left, const_reference _Right) { return this->_Key_equal(_Left.first, _Right.first); }); - auto _Remaining_count = _STD distance(_Sorted_view.begin(), _Subrange.begin()); + auto _Remaining_count = _Subrange.begin() - _Sorted_view.begin(); _Data.keys.erase(_Data.keys.begin() + _Remaining_count, _Data.keys.end()); _Data.values.erase(_Data.values.begin() + _Remaining_count, _Data.values.end()); _Guard._Clearable = nullptr; @@ -936,36 +936,36 @@ private: template _NODISCARD iterator _Lower_bound(const _KeyTy& _Key_val) { _STL_INTERNAL_STATIC_ASSERT(is_same_v<_KeyTy, key_type> || _Is_transparent_v); - auto _Key_it = _STD lower_bound(_Data.keys.cbegin(), _Data.keys.cend(), _Key_val, _Key_compare); - auto _Dist = _STD distance(_Data.keys.cbegin(), _Key_it); - auto _Val_it = _Data.values.begin() + _Dist; + auto _Key_it = _STD lower_bound(_Data.keys.cbegin(), _Data.keys.cend(), _Key_val, _Key_compare); + const auto _Dist = _Key_it - _Data.keys.cbegin(); + auto _Val_it = _Data.values.begin() + _Dist; return iterator{_STD move(_Key_it), _STD move(_Val_it)}; } template _NODISCARD const_iterator _Lower_bound(const _KeyTy& _Key_val) const { _STL_INTERNAL_STATIC_ASSERT(is_same_v<_KeyTy, key_type> || _Is_transparent_v); - auto _Key_it = _STD lower_bound(_Data.keys.cbegin(), _Data.keys.cend(), _Key_val, _Key_compare); - auto _Dist = _STD distance(_Data.keys.cbegin(), _Key_it); - auto _Val_it = _Data.values.cbegin() + _Dist; + auto _Key_it = _STD lower_bound(_Data.keys.cbegin(), _Data.keys.cend(), _Key_val, _Key_compare); + const auto _Dist = _Key_it - _Data.keys.cbegin(); + auto _Val_it = _Data.values.cbegin() + _Dist; return const_iterator{_STD move(_Key_it), _STD move(_Val_it)}; } template _NODISCARD iterator _Upper_bound(const _KeyTy& _Key_val) { _STL_INTERNAL_STATIC_ASSERT(is_same_v<_KeyTy, key_type> || _Is_transparent_v); - auto _Key_it = _STD upper_bound(_Data.keys.cbegin(), _Data.keys.cend(), _Key_val, _Key_compare); - auto _Dist = _STD distance(_Data.keys.cbegin(), _Key_it); - auto _Val_it = _Data.values.begin() + _Dist; + auto _Key_it = _STD upper_bound(_Data.keys.cbegin(), _Data.keys.cend(), _Key_val, _Key_compare); + const auto _Dist = _Key_it - _Data.keys.cbegin(); + auto _Val_it = _Data.values.begin() + _Dist; return iterator{_STD move(_Key_it), _STD move(_Val_it)}; } template _NODISCARD const_iterator _Upper_bound(const _KeyTy& _Key_val) const { _STL_INTERNAL_STATIC_ASSERT(is_same_v<_KeyTy, key_type> || _Is_transparent_v); - auto _Key_it = _STD upper_bound(_Data.keys.cbegin(), _Data.keys.cend(), _Key_val, _Key_compare); - auto _Dist = _STD distance(_Data.keys.cbegin(), _Key_it); - auto _Val_it = _Data.values.cbegin() + _Dist; + auto _Key_it = _STD upper_bound(_Data.keys.cbegin(), _Data.keys.cend(), _Key_val, _Key_compare); + const auto _Dist = _Key_it - _Data.keys.cbegin(); + auto _Val_it = _Data.values.cbegin() + _Dist; return const_iterator{_STD move(_Key_it), _STD move(_Val_it)}; } @@ -1167,9 +1167,9 @@ private: const auto _Position = this->find(_Key_val); if (_Position == this->end()) { _Xout_of_range("std::flat_map::at: the specified key does not exist."); - } else { - return _Position->second; } + + return _Position->second; } template @@ -1178,9 +1178,9 @@ private: const auto _Position = this->find(_Key_val); if (_Position == this->end()) { _Xout_of_range("std::flat_map::at: the specified key does not exist."); - } else { - return _Position->second; } + + return _Position->second; } template @@ -1188,17 +1188,17 @@ private: auto _Key_it = _STD lower_bound(_Data.keys.begin(), _Data.keys.end(), _Key_val, _Key_compare); if (_Key_it != _Data.keys.end() && _Key_equal(*_Key_it, _STD forward<_KeyTy>(_Key_val))) { // Already exists - return {this->begin() + _STD distance(_Data.keys.begin(), _Key_it), false}; - } else { - // Need to insert - auto _Index = _STD distance(_Data.keys.begin(), _Key_it); - { - key_type _Key_to_insert(_STD forward<_KeyTy>(_Key_val)); - mapped_type _Mapped_to_insert(_STD forward<_MappedArgTypes>(_Mapped_args)...); - this->_Insert_exact(this->cbegin() + _Index, _STD move(_Key_to_insert), _STD move(_Mapped_to_insert)); - } - return {this->begin() + _Index, true}; + return {this->begin() + (_Key_it - _Data.keys.begin()), false}; + } + + // Need to insert + const auto _Index = _Key_it - _Data.keys.begin(); + { + key_type _Key_to_insert(_STD forward<_KeyTy>(_Key_val)); + mapped_type _Mapped_to_insert(_STD forward<_MappedArgTypes>(_Mapped_args)...); + this->_Insert_exact(this->cbegin() + _Index, _STD move(_Key_to_insert), _STD move(_Mapped_to_insert)); } + return {this->begin() + _Index, true}; } template @@ -1333,7 +1333,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 _Index = _STD distance(_Data.keys.begin(), _Key_it); + const auto _Index = _Key_it - _Data.keys.begin(); { _Key _Key_to_insert(_STD forward<_KeyTy>(_Key_val)); From cd7688b306bd64d723daba750b5399cfb17270b9 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Sun, 10 Mar 2024 22:44:55 +0800 Subject: [PATCH 2/8] Use direct-non-list-initialization --- stl/inc/flat_map | 4 ++-- tests/std/tests/P0429R9_flat_map/test.cpp | 29 +++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/stl/inc/flat_map b/stl/inc/flat_map index 8d4d9acf214..77aeff60c80 100644 --- a/stl/inc/flat_map +++ b/stl/inc/flat_map @@ -789,7 +789,7 @@ protected: if (_Key_equal(_Key_val, *_Position._Key_it)) { const auto _Dist = _Position._Key_it - _Begin._Key_it; const auto _It = begin() + _Dist; - *_It._Mapped_it = mapped_type{_STD forward<_MappedArgTypes>(_Args)...}; + *_It._Mapped_it = mapped_type(_STD forward<_MappedArgTypes>(_Args)...); return _It; } @@ -797,7 +797,7 @@ protected: if (_Position != _End && _Key_equal(_Key_val, *_Position._Key_it)) { const auto _Dist = _Position._Key_it - _Begin._Key_it; auto _It = begin() + _Dist; - *_It._Mapped_it = mapped_type{_STD forward<_MappedArgTypes>(_Args)...}; + *_It._Mapped_it = mapped_type(_STD forward<_MappedArgTypes>(_Args)...); return _It; } } else { diff --git a/tests/std/tests/P0429R9_flat_map/test.cpp b/tests/std/tests/P0429R9_flat_map/test.cpp index ce73a32703f..084495e63d4 100644 --- a/tests/std/tests/P0429R9_flat_map/test.cpp +++ b/tests/std/tests/P0429R9_flat_map/test.cpp @@ -336,6 +336,28 @@ void test_gh_4344() { assert(check_value_content(fm, {'m', 'o', 'e', 'w'})); } +class direct_init_only { +private: + unsigned int n_ = 0u; + +public: + struct src_type { + unsigned int n_ = 0u; + + friend bool operator==(const src_type&, const src_type&) = default; + }; + + explicit direct_init_only(const src_type& s) noexcept : n_(s.n_) {} + direct_init_only(initializer_list) = delete; + + direct_init_only& operator=(const src_type& s) noexcept { + n_ = s.n_; + return *this; + } + + friend bool operator==(const direct_init_only&, const direct_init_only&) = default; +}; + void test_insert_or_assign() { flat_map fm; @@ -378,6 +400,13 @@ void test_insert_or_assign() { assert(check_key_content(fm, {10, 20, 70, 90})); assert(check_value_content(fm, {'b', 'a', 'X', 'w'})); + + // ensure direct-initialization + flat_map direct_fm; + direct_fm.insert_or_assign(direct_fm.end(), 0, direct_init_only::src_type{42u}); + + assert(check_key_content(direct_fm, {0})); + assert(check_value_content(direct_fm, {direct_init_only(direct_init_only::src_type{42u})})); } // Test MSVC STL-specific SCARY-ness From 9fff373116a98276a4ee1bb4cddec207e54a21fb Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Sun, 10 Mar 2024 22:56:23 +0800 Subject: [PATCH 3/8] Avoid calling `distance` on non-forward input iterators Also fix iterator dereferencing in insertion --- stl/inc/flat_map | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/stl/inc/flat_map b/stl/inc/flat_map index 77aeff60c80..605e5ce838d 100644 --- a/stl/inc/flat_map +++ b/stl/inc/flat_map @@ -862,27 +862,30 @@ private: _Clear_flat_map_scope_guard _Guard{this}; // Insert the new elements at the end - size_type _OldSize = size(); - size_type _NewSize = _STD distance(_First, _Last); + const size_type _Old_size = size(); + if constexpr (_Is_cpp17_fwd_iter_v<_InputIterator>) { + const auto _New_size = static_cast(_STD distance(_First, _Last)); - _Data.keys.reserve(_Data.keys.size() + _NewSize); - _Data.values.reserve(_Data.values.size() + _NewSize); + _Data.keys.reserve(_Data.keys.size() + _New_size); + _Data.values.reserve(_Data.values.size() + _New_size); + } for (; _First != _Last; ++_First) { - _Data.keys.emplace_back(_STD move(_First->first)); - _Data.values.emplace_back(_STD move(_First->second)); + value_type _Val = *_First; + _Data.keys.emplace_back(_STD move(_Val.first)); + _Data.values.emplace_back(_STD move(_Val.second)); } // Sort the newly inserted elements auto _Sorted_view = _View_to_mutate(); if constexpr (_NeedSorting) { auto _Sorted_new_elements = _Sorted_view; - _Sorted_new_elements.advance(_OldSize); + _Sorted_new_elements.advance(_Old_size); _RANGES sort(_Sorted_new_elements, value_compare(_Key_compare)); } // Merge the newly inserted elements with the existing elements - _RANGES inplace_merge(_Sorted_view, _Sorted_view.begin() + _OldSize, value_compare(_Key_compare)); + _RANGES inplace_merge(_Sorted_view, _Sorted_view.begin() + _Old_size, value_compare(_Key_compare)); if constexpr (_NeedDeduping) { _Dedup(); From e04d30db33bf9c29be4e65a98681f0d3de00f471 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Sun, 10 Mar 2024 23:08:43 +0800 Subject: [PATCH 4/8] Short-circuit evaluation of `_Hint_is_accurate` --- stl/inc/flat_map | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/stl/inc/flat_map b/stl/inc/flat_map index 605e5ce838d..c37489e43da 100644 --- a/stl/inc/flat_map +++ b/stl/inc/flat_map @@ -764,16 +764,18 @@ protected: const const_iterator _Begin = cbegin(); const const_iterator _End = cend(); - bool _Insert_before_position = false; - bool _Insert_after_position_minus_1 = false; - if constexpr (_IsUnique) { - _Insert_before_position = _Position == _End || _Key_compare(_Key_val, *_Position._Key_it); - _Insert_after_position_minus_1 = _Position == _Begin || _Key_compare(_Position._Key_it[-1], _Key_val); - } else { - _Insert_before_position = _Position == _End || !_Key_compare(*_Position._Key_it, _Key_val); - _Insert_after_position_minus_1 = _Position == _Begin || !_Key_compare(_Key_val, _Position._Key_it[-1]); - } - const bool _Hint_is_accurate = _Insert_before_position && _Insert_after_position_minus_1; + const bool _Hint_is_accurate = [&] { + if constexpr (_IsUnique) { + if (_Position == _End || _Key_compare(_Key_val, *_Position._Key_it)) { + return _Position == _Begin || _Key_compare(_Position._Key_it[-1], _Key_val); + } + } else { + if (_Position == _End || !_Key_compare(*_Position._Key_it, _Key_val)) { + return _Position == _Begin || !_Key_compare(_Key_val, _Position._Key_it[-1]); + } + } + return false; + }(); if (_Hint_is_accurate) { const auto _Dist = _Position._Key_it - _Begin._Key_it; @@ -783,7 +785,7 @@ protected: _Insert_exact(_Position, _STD move(_Key_to_insert), _STD move(_Mapped_to_insert)); } return begin() + _Dist; - } + } if constexpr (_OverwriteIfExists) { if (_Key_equal(_Key_val, *_Position._Key_it)) { From 896d66fde281c8cfa39e748ddb61df9652546ba3 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Sun, 10 Mar 2024 23:33:26 +0800 Subject: [PATCH 5/8] Eliminate `_Key_equal` after `lower_bound` --- stl/inc/flat_map | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/stl/inc/flat_map b/stl/inc/flat_map index c37489e43da..c9a52ea7555 100644 --- a/stl/inc/flat_map +++ b/stl/inc/flat_map @@ -796,7 +796,7 @@ protected: } _Position = lower_bound(_Key_val); - if (_Position != _End && _Key_equal(_Key_val, *_Position._Key_it)) { + if (_Position != _End && !_Key_compare(_Key_val, *_Position._Key_it)) { const auto _Dist = _Position._Key_it - _Begin._Key_it; auto _It = begin() + _Dist; *_It._Mapped_it = mapped_type(_STD forward<_MappedArgTypes>(_Args)...); @@ -908,7 +908,7 @@ private: _NODISCARD iterator _Find(const _KeyTy& _Key_val) { _STL_INTERNAL_STATIC_ASSERT(is_same_v<_KeyTy, key_type> || _Is_transparent_v); iterator _Position = lower_bound(_Key_val); - if (_Position != end() && _Key_equal(_Position->first, _Key_val)) { + if (_Position != end() && !_Key_compare(_Key_val, _Position->first)) { return _Position; } else { return end(); @@ -919,7 +919,7 @@ private: _NODISCARD const_iterator _Find(const _KeyTy& _Key_val) const { _STL_INTERNAL_STATIC_ASSERT(is_same_v<_KeyTy, key_type> || _Is_transparent_v); const_iterator _Position = lower_bound(_Key_val); - if (_Position != cend() && _Key_equal(_Position->first, _Key_val)) { + if (_Position != cend() && !_Key_compare(_Key_val, _Position->first)) { return _Position; } else { return cend(); @@ -1191,7 +1191,7 @@ private: template pair _Try_emplace(_KeyTy&& _Key_val, _MappedArgTypes&&... _Mapped_args) { auto _Key_it = _STD lower_bound(_Data.keys.begin(), _Data.keys.end(), _Key_val, _Key_compare); - if (_Key_it != _Data.keys.end() && _Key_equal(*_Key_it, _STD forward<_KeyTy>(_Key_val))) { + if (_Key_it != _Data.keys.end() && !_Key_compare(_STD forward<_KeyTy>(_Key_val), *_Key_it)) { // Already exists return {this->begin() + (_Key_it - _Data.keys.begin()), false}; } From 6d4085aa3e68def7b1142ebbb59e7560571126ea Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 11 Mar 2024 01:20:57 +0800 Subject: [PATCH 6/8] Unwrap iterators before calling algorithms - `lower_bound` - `upper_bound` - `lexicographical_compare_three_way` --- stl/inc/flat_map | 56 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/stl/inc/flat_map b/stl/inc/flat_map index c9a52ea7555..ec035877147 100644 --- a/stl/inc/flat_map +++ b/stl/inc/flat_map @@ -722,8 +722,9 @@ public: } _NODISCARD_FRIEND auto operator<=>(const _Derived& _Left, const _Derived& _Right) { - return _STD lexicographical_compare_three_way( - _Left.cbegin(), _Left.cend(), _Right.cbegin(), _Right.cend(), _Synth_three_way{}); + 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{}); } friend void swap(_Derived& _Left, _Derived& _Right) noexcept { @@ -737,7 +738,6 @@ protected: template size_type _Erase_if(_Predicate _Pred) { - auto _View = _View_to_mutate(); auto _Mut_first = _View.begin(); auto _Mut_last = _View.end(); @@ -848,11 +848,11 @@ private: void _Dedup() { _Clear_flat_map_scope_guard _Guard{this}; - auto _Sorted_view = _View_to_mutate(); - auto _Subrange = _RANGES unique(_Sorted_view, [this](const_reference _Left, const_reference _Right) { + auto _Sorted_view = _View_to_mutate(); + auto _Subrange = _RANGES unique(_Sorted_view, [this](const_reference _Left, const_reference _Right) { return this->_Key_equal(_Left.first, _Right.first); }); - auto _Remaining_count = _Subrange.begin() - _Sorted_view.begin(); + const auto _Remaining_count = _Subrange.begin() - _Sorted_view.begin(); _Data.keys.erase(_Data.keys.begin() + _Remaining_count, _Data.keys.end()); _Data.values.erase(_Data.values.begin() + _Remaining_count, _Data.values.end()); _Guard._Clearable = nullptr; @@ -941,36 +941,56 @@ private: template _NODISCARD iterator _Lower_bound(const _KeyTy& _Key_val) { _STL_INTERNAL_STATIC_ASSERT(is_same_v<_KeyTy, key_type> || _Is_transparent_v); - auto _Key_it = _STD lower_bound(_Data.keys.cbegin(), _Data.keys.cend(), _Key_val, _Key_compare); - const auto _Dist = _Key_it - _Data.keys.cbegin(); - auto _Val_it = _Data.values.begin() + _Dist; + const auto _Key_unchecked_begin = _STD _Get_unwrapped(_Data.keys.cbegin()); + const auto _Key_unchecked_it = + _STD lower_bound(_Key_unchecked_begin, _STD _Get_unwrapped(_Data.keys.cend()), _Key_val, _Key_compare); + const auto _Dist = _Key_unchecked_it - _Key_unchecked_begin; + + auto _Key_it = _Data.keys.cbegin(); + _STD _Seek_wrapped(_Key_it, _Key_unchecked_it); + auto _Val_it = _Data.values.begin() + _Dist; return iterator{_STD move(_Key_it), _STD move(_Val_it)}; } template _NODISCARD const_iterator _Lower_bound(const _KeyTy& _Key_val) const { _STL_INTERNAL_STATIC_ASSERT(is_same_v<_KeyTy, key_type> || _Is_transparent_v); - auto _Key_it = _STD lower_bound(_Data.keys.cbegin(), _Data.keys.cend(), _Key_val, _Key_compare); - const auto _Dist = _Key_it - _Data.keys.cbegin(); - auto _Val_it = _Data.values.cbegin() + _Dist; + const auto _Key_unchecked_begin = _STD _Get_unwrapped(_Data.keys.cbegin()); + const auto _Key_unchecked_it = + _STD lower_bound(_Key_unchecked_begin, _STD _Get_unwrapped(_Data.keys.cend()), _Key_val, _Key_compare); + const auto _Dist = _Key_unchecked_it - _Key_unchecked_begin; + + auto _Key_it = _Data.keys.cbegin(); + _STD _Seek_wrapped(_Key_it, _Key_unchecked_it); + auto _Val_it = _Data.values.cbegin() + _Dist; return const_iterator{_STD move(_Key_it), _STD move(_Val_it)}; } template _NODISCARD iterator _Upper_bound(const _KeyTy& _Key_val) { _STL_INTERNAL_STATIC_ASSERT(is_same_v<_KeyTy, key_type> || _Is_transparent_v); - auto _Key_it = _STD upper_bound(_Data.keys.cbegin(), _Data.keys.cend(), _Key_val, _Key_compare); - const auto _Dist = _Key_it - _Data.keys.cbegin(); - auto _Val_it = _Data.values.begin() + _Dist; + const auto _Key_unchecked_begin = _STD _Get_unwrapped(_Data.keys.cbegin()); + const auto _Key_unchecked_it = + _STD upper_bound(_Key_unchecked_begin, _STD _Get_unwrapped(_Data.keys.cend()), _Key_val, _Key_compare); + const auto _Dist = _Key_unchecked_it - _Key_unchecked_begin; + + auto _Key_it = _Data.keys.cbegin(); + _STD _Seek_wrapped(_Key_it, _Key_unchecked_it); + auto _Val_it = _Data.values.begin() + _Dist; return iterator{_STD move(_Key_it), _STD move(_Val_it)}; } template _NODISCARD const_iterator _Upper_bound(const _KeyTy& _Key_val) const { _STL_INTERNAL_STATIC_ASSERT(is_same_v<_KeyTy, key_type> || _Is_transparent_v); - auto _Key_it = _STD upper_bound(_Data.keys.cbegin(), _Data.keys.cend(), _Key_val, _Key_compare); - const auto _Dist = _Key_it - _Data.keys.cbegin(); - auto _Val_it = _Data.values.cbegin() + _Dist; + const auto _Key_unchecked_begin = _STD _Get_unwrapped(_Data.keys.cbegin()); + const auto _Key_unchecked_it = + _STD upper_bound(_Key_unchecked_begin, _STD _Get_unwrapped(_Data.keys.cend()), _Key_val, _Key_compare); + const auto _Dist = _Key_unchecked_it - _Key_unchecked_begin; + + auto _Key_it = _Data.keys.cbegin(); + _STD _Seek_wrapped(_Key_it, _Key_unchecked_it); + auto _Val_it = _Data.values.cbegin() + _Dist; return const_iterator{_STD move(_Key_it), _STD move(_Val_it)}; } From 406cefd300f8605a0c8e9557cd7edf2362bfc0c2 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 11 Mar 2024 01:49:17 +0800 Subject: [PATCH 7/8] Improve tests - Inspect the return value of `erase_if` - Show different effects of insertion for different containers - Extract `conditional_t` - Test SCARY-ness for various containers (including `deque`) --- .../include/test_header_units_and_modules.hpp | 12 +-- tests/std/tests/P0429R9_flat_map/test.cpp | 92 ++++++++++++------- 2 files changed, 67 insertions(+), 37 deletions(-) diff --git a/tests/std/include/test_header_units_and_modules.hpp b/tests/std/include/test_header_units_and_modules.hpp index 11d234a116e..667def47819 100644 --- a/tests/std/include/test_header_units_and_modules.hpp +++ b/tests/std/include/test_header_units_and_modules.hpp @@ -251,17 +251,17 @@ void test_flat_map() { constexpr auto simple_truth = [](const auto&) { return true; }; flat_map fm; - fm.emplace(1, 1); - fm.emplace(1, 2); + fm.emplace(42, 172); + fm.emplace(42, 729); assert(fm.size() == 1); - erase_if(fm, simple_truth); + assert(erase_if(fm, simple_truth) == 1); assert(fm.empty()); flat_multimap fmm; - fmm.emplace(1, 1); - fmm.emplace(1, 2); + fmm.emplace(42, 172); + fmm.emplace(42, 729); assert(fmm.size() == 2); - erase_if(fmm, simple_truth); + assert(erase_if(fmm, simple_truth) == 2); assert(fmm.empty()); } #endif // TEST_STANDARD >= 23 diff --git a/tests/std/tests/P0429R9_flat_map/test.cpp b/tests/std/tests/P0429R9_flat_map/test.cpp index 084495e63d4..67d54f99d92 100644 --- a/tests/std/tests/P0429R9_flat_map/test.cpp +++ b/tests/std/tests/P0429R9_flat_map/test.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -269,38 +270,45 @@ void test_pointer_to_incomplete_type() { flat_map, shared_ptr>> fmap; } -void test_insert_unique() { +void test_insert() { flat_map fm; - const auto p1 = fm.insert({10, 'm'}); - assert(p1.first->first == 10); - assert(p1.first->second == 'm'); - assert(p1.second); + const auto res1 = fm.insert({10, 'm'}); + assert(res1.first->first == 10); + assert(res1.first->second == 'm'); + assert(res1.second); + + const auto res2 = fm.insert({10, 'n'}); + assert(res2.first->first == 10); + assert(res2.first->second == 'm'); + assert(!res2.second); const flat_map::value_type val_pair{90, 'w'}; - const auto p2 = fm.insert(val_pair); - assert(p2.first->first == 90); - assert(p2.first->second == 'w'); - assert(p2.second); + const auto res3 = fm.insert(val_pair); + assert(res3.first->first == 90); + assert(res3.first->second == 'w'); + assert(res3.second); assert(check_key_content(fm, {10, 90})); assert(check_value_content(fm, {'m', 'w'})); -} -void test_insert_multi() { flat_multimap fmm; const auto it1 = fmm.insert({10, 'm'}); assert(it1->first == 10); assert(it1->second == 'm'); - const flat_multimap::value_type val_pair{90, 'w'}; - const auto it2 = fmm.insert(val_pair); - assert(it2->first == 90); - assert(it2->second == 'w'); + const auto it2 = fmm.insert({10, 'n'}); + assert(it2->first == 10); + assert(it2->second == 'n'); - assert(check_key_content(fmm, {10, 90})); - assert(check_value_content(fmm, {'m', 'w'})); + const flat_multimap::value_type val_pair2{90, 'w'}; + const auto it3 = fmm.insert(val_pair2); + assert(it3->first == 90); + assert(it3->second == 'w'); + + assert(check_key_content(fmm, {10, 10, 90})); + assert(check_value_content(fmm, {'n', 'm', 'w'})); } // GH-4344 Fix compile errors @@ -410,29 +418,52 @@ void test_insert_or_assign() { } // Test MSVC STL-specific SCARY-ness + +template +struct flat_map_unique_if_impl; +template <> +struct flat_map_unique_if_impl { + template + using type = flat_map; +}; +template <> +struct flat_map_unique_if_impl { + template + using type = flat_multimap; +}; + +template +using flat_map_unique_if = flat_map_unique_if_impl::template type; + template void test_scary_ness_one() { // COMPILE-ONLY - using Iter = flat_map::iterator; - using OtherIter = conditional_t, vector>, - flat_multimap, vector>>::iterator; + using Iter = flat_map::iterator; + using OtherIter = + flat_map_unique_if, vector>::iterator; static_assert(is_same_v); using ConstIter = flat_map::const_iterator; using OtherConstIter = - conditional_t, vector>, - flat_multimap, vector>>::const_iterator; + flat_map_unique_if, vector>::const_iterator; static_assert(is_same_v); - using Cont = flat_map, vector, vector>::containers; - using OtherCont = conditional_t, vector>, - flat_multimap, vector>>::containers; + using Cont = flat_map, vector, vector>::containers; + using OtherCont = + flat_map_unique_if, vector>::containers; static_assert(is_same_v); using ValueComp = flat_map, vector>::value_compare; - using OtherValueComp = - conditional_t, vector>, - flat_multimap, vector>>::value_compare; - static_assert(is_same_v); + using OtherValueComp1 = + flat_map_unique_if, vector>::value_compare; + using OtherValueComp2 = flat_map_unique_if, vector>::value_compare; + using OtherValueComp3 = + flat_map_unique_if, deque>::value_compare; + using OtherValueComp4 = + flat_map_unique_if, vector>::value_compare; + static_assert(is_same_v); + static_assert(is_same_v); + static_assert(is_same_v); + static_assert(is_same_v); } void test_scary_ness() { // COMPILE-ONLY @@ -465,8 +496,7 @@ int main() { test_construction(); test_pointer_to_incomplete_type(); test_erase_if(); - test_insert_unique(); - test_insert_multi(); + test_insert(); test_gh_4344(); test_insert_or_assign(); } From 1ab4922230616f40b08799d4ad9d71c403f273e9 Mon Sep 17 00:00:00 2001 From: "A. Jiang" Date: Mon, 11 Mar 2024 02:13:39 +0800 Subject: [PATCH 8/8] Consistency change on `__cpp_lib_byte` --- stl/inc/flat_map | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/stl/inc/flat_map b/stl/inc/flat_map index ec035877147..bc14e268b33 100644 --- a/stl/inc/flat_map +++ b/stl/inc/flat_map @@ -1282,7 +1282,17 @@ flat_map(from_range_t, _Rng&&, _Compare = _Compare(), _Allocator = _Allocator()) -> flat_map<_Range_key_type<_Rng>, _Range_mapped_type<_Rng>, _Compare, vector<_Range_key_type<_Rng>, _Rebind_alloc_t<_Allocator, _Range_key_type<_Rng>>>, vector<_Range_mapped_type<_Rng>, _Rebind_alloc_t<_Allocator, _Range_mapped_type<_Rng>>>>; -#endif // defined(__cpp_lib_byte) +#else // ^^^ defined(__cpp_lib_byte) / !defined(__cpp_lib_byte) vvv +template <_RANGES input_range _Rng, class _Compare = less<_Range_key_type<_Rng>>> +flat_map(from_range_t, _Rng&&, _Compare = _Compare()) -> flat_map<_Range_key_type<_Rng>, _Range_mapped_type<_Rng>, + _Compare, vector<_Range_key_type<_Rng>, allocator<_Range_key_type<_Rng>>>, + vector<_Range_mapped_type<_Rng>, allocator<_Range_mapped_type<_Rng>>>>; + +template <_RANGES input_range _Rng, class _Compare, class _Allocator> +flat_map(from_range_t, _Rng&&, _Compare, _Allocator) -> flat_map<_Range_key_type<_Rng>, _Range_mapped_type<_Rng>, + _Compare, vector<_Range_key_type<_Rng>, _Rebind_alloc_t<_Allocator, _Range_key_type<_Rng>>>, + vector<_Range_mapped_type<_Rng>, _Rebind_alloc_t<_Allocator, _Range_mapped_type<_Rng>>>>; +#endif // ^^^ !defined(__cpp_lib_byte) ^^^ template <_RANGES input_range _Rng, class _Allocator> flat_map(from_range_t, _Rng&&, _Allocator) -> flat_map<_Range_key_type<_Rng>, _Range_mapped_type<_Rng>, @@ -1410,7 +1420,18 @@ flat_multimap(from_range_t, _Rng&&, _Compare = _Compare(), _Allocator = _Allocat -> flat_multimap<_Range_key_type<_Rng>, _Range_mapped_type<_Rng>, _Compare, vector<_Range_key_type<_Rng>, _Rebind_alloc_t<_Allocator, _Range_key_type<_Rng>>>, vector<_Range_mapped_type<_Rng>, _Rebind_alloc_t<_Allocator, _Range_mapped_type<_Rng>>>>; -#endif // defined(__cpp_lib_byte) +#else // ^^^ defined(__cpp_lib_byte) / !defined(__cpp_lib_byte) vvv +template <_RANGES input_range _Rng, class _Compare = less<_Range_key_type<_Rng>>> +flat_multimap(from_range_t, _Rng&&, _Compare = _Compare()) -> flat_multimap<_Range_key_type<_Rng>, + _Range_mapped_type<_Rng>, _Compare, vector<_Range_key_type<_Rng>, allocator<_Range_key_type<_Rng>>>, + vector<_Range_mapped_type<_Rng>, allocator<_Range_mapped_type<_Rng>>>>; + +template <_RANGES input_range _Rng, class _Compare, class _Allocator> +flat_multimap(from_range_t, _Rng&&, _Compare, _Allocator) + -> flat_multimap<_Range_key_type<_Rng>, _Range_mapped_type<_Rng>, _Compare, + vector<_Range_key_type<_Rng>, _Rebind_alloc_t<_Allocator, _Range_key_type<_Rng>>>, + vector<_Range_mapped_type<_Rng>, _Rebind_alloc_t<_Allocator, _Range_mapped_type<_Rng>>>>; +#endif // ^^^ !defined(__cpp_lib_byte) ^^^ template <_RANGES input_range _Rng, class _Allocator> flat_multimap(from_range_t, _Rng&&, _Allocator) -> flat_multimap<_Range_key_type<_Rng>, _Range_mapped_type<_Rng>,