Skip to content

<flat_map>: Unwrap flat_(multi)map's iterators #4650

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Stephan T. Lavavej (StephanTLavavej) merged 3 commits into from
May 11, 2024
Merged

<flat_map>: Unwrap flat_(multi)map's iterators #4650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b22f516
Unwrap `flat_(multi)map`'s iterators
frederick-vs-ja May 3, 2024
e692464
Add newlines.
StephanTLavavej May 11, 2024
6c1e3a1
No, discard void!
StephanTLavavej May 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 75 additions & 17 deletions stl/inc/flat_map
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,17 @@ struct _NODISCARD _Clear_flat_map_scope_guard {
template <bool _IsUnique, class _Key, class _Mapped, class _Compare, class _KeyContainer, class _MappedContainer>
class _Flat_map_base;

template <class _KeyIter, class _MappedIter, class _MappedConvIter>
struct _Pairing_iterator_provider;

template <class _KeyIter, class _MappedIter, class _MappedConvIter>
concept _Can_unwrap_pairing_iterator =
!conjunction_v<is_same<_Unwrapped_t<_KeyIter>, _KeyIter>, is_same<_Unwrapped_t<_MappedIter>, _MappedIter>>;

template <class _KeyIter, class _MappedIter, class _MappedConvIter>
using _Unwrapped_pairing_iterator = _Pairing_iterator_provider<_Unwrapped_t<_KeyIter>, _Unwrapped_t<_MappedIter>,
_Unwrapped_t<_MappedConvIter>>::_Iterator;

template <class _KeyIter, class _MappedIter, class _MappedConvIter>
struct _Pairing_iterator_provider {
class _Iterator {
Expand All @@ -95,7 +106,9 @@ struct _Pairing_iterator_provider {
friend class _Flat_map_base;

_Iterator() = default;
_Iterator(_KeyIter _Key_iter, _MappedIter _Mapped_iter) : _Key_it(_Key_iter), _Mapped_it(_Mapped_iter) {}
_Iterator(_KeyIter _Key_iter, _MappedIter _Mapped_iter) noexcept(
is_nothrow_move_constructible_v<_KeyIter> && is_nothrow_move_constructible_v<_MappedIter>)
: _Key_it(_STD move(_Key_iter)), _Mapped_it(_STD move(_Mapped_iter)) {}

using iterator_category = input_iterator_tag;
using iterator_concept = random_access_iterator_tag;
Expand Down Expand Up @@ -203,6 +216,57 @@ struct _Pairing_iterator_provider {
return _Const_iterator{_Key_it, _Mapped_it};
}

const _KeyIter& _Key_iterator() const noexcept {
return _Key_it;
}

const _MappedIter& _Mapped_iterator() const noexcept {
return _Mapped_it;
}

using _Prevent_inheriting_unwrap = _Iterator;

friend void _Verify_range(const _Iterator& _First, const _Iterator& _Last) noexcept {
if constexpr (_Range_verifiable_v<_KeyIter>) {
_Verify_range(_First._Key_it, _Last._Key_it); // intentional ADL
}

if constexpr (_Range_verifiable_v<_MappedIter>) {
_Verify_range(_First._Mapped_it, _Last._Mapped_it); // intentional ADL
}

_STL_VERIFY(_Last._Key_it - _First._Key_it == _Last._Mapped_it - _First._Mapped_it,
"iterators from inconsistent ranges");
}

void _Verify_offset(const difference_type _Off) const noexcept {
if constexpr (_Offset_verifiable_v<_KeyIter>) {
_Key_it._Verify_offset(_Off);
} else {
_STL_VERIFY(_Off == 0 || _Key_it != _KeyIter(), "cannot seek value-initialized iterator");
}

if constexpr (_Offset_verifiable_v<_MappedIter>) {
_Mapped_it._Verify_offset(_Off);
} else {
_STL_VERIFY(_Off == 0 || _Mapped_it != _MappedIter(), "cannot seek value-initialized iterator");
}
}

_NODISCARD auto _Unwrapped() const
requires _Can_unwrap_pairing_iterator<_KeyIter, _MappedIter, _MappedConvIter>
{
using _Unwrapped_iterator = _Unwrapped_pairing_iterator<_KeyIter, _MappedIter, _MappedConvIter>;
return _Unwrapped_iterator{_STD _Get_unwrapped(_Key_it), _STD _Get_unwrapped(_Mapped_it)};
}

void _Seek_to(const _Unwrapped_pairing_iterator<_KeyIter, _MappedIter, _MappedConvIter>& _Dst)
requires _Can_unwrap_pairing_iterator<_KeyIter, _MappedIter, _MappedConvIter>
{
_STD _Seek_wrapped(_Key_it, _Dst._Key_iterator());
_STD _Seek_wrapped(_Mapped_it, _Dst._Mapped_iterator());
}

private:
_KeyIter _Key_it;
_MappedIter _Mapped_it;
Expand Down Expand Up @@ -717,8 +781,10 @@ public:
}

_NODISCARD_FRIEND bool operator==(const _Derived& _Left, const _Derived& _Right) {
return _RANGES equal(_Left._Data.keys, _Right._Data.keys)
&& _RANGES equal(_Left._Data.values, _Right._Data.values);
auto& _Left_base = static_cast<const _Flat_map_base&>(_Left);
auto& _Right_base = static_cast<const _Flat_map_base&>(_Right);
return _RANGES equal(_Left_base._Data.keys, _Right_base._Data.keys)
&& _RANGES equal(_Left_base._Data.values, _Right_base._Data.values);
Comment on lines +786 to +787

@frederick-vs-ja A. Jiang (frederick-vs-ja) May 3, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we sometimes call _RANGES equal(_Left_base, _Right_base), especially when vectorization is unavailable?

@AlexGuteniev Alex Guteniev (AlexGuteniev) May 3, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that even if the comparison is not vectorized, it may be still advantageous to avoid jumping between arrays, and have dedicated cache/prefetch use just for one array. Though this advantage looks not very significant though.

@AlexGuteniev Alex Guteniev (AlexGuteniev) May 3, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm more wondering how this can work for multimap with equivalent keys with distinct values.

@frederick-vs-ja A. Jiang (frederick-vs-ja) May 3, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm more wondering how this can work for multimap with equivalent keys with distinct values.

In flat_multimap, multiple equivalent keys are stored multiple times and each instance is grouped with it's corresponding value, so ranges::equal just works.

@AlexGuteniev Alex Guteniev (AlexGuteniev) May 3, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But is order of values defined?
Like {4, 10}, {4, 20}, {4, 30} map can be stored like {4, 4, 4}, {10, 20, 30} or {4, 4, 4}, {30, 20, 10}

@frederick-vs-ja A. Jiang (frederick-vs-ja) May 3, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order of values depends on the order of insertion, as specified in [associative.reqmts.general]/68.

@AlexGuteniev Alex Guteniev (AlexGuteniev) May 4, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. To me it looks not useful if multimap with the same pairs might happen inequal

}

_NODISCARD_FRIEND auto operator<=>(const _Derived& _Left, const _Derived& _Right) {
Expand Down Expand Up @@ -863,15 +929,9 @@ private:
void _Insert_range(_InputIterator _First, _InputIterator _Last) {
_Clear_flat_map_scope_guard _Guard{this};

// Insert the new elements at the end
const size_type _Old_size = size();
if constexpr (_Is_cpp17_fwd_iter_v<_InputIterator>) {
const auto _New_size = static_cast<size_type>(_STD distance(_First, _Last));

_Data.keys.reserve(_Data.keys.size() + _New_size);
_Data.values.reserve(_Data.values.size() + _New_size);
}
const auto _Old_distance = static_cast<difference_type>(size());

// Insert the new elements at the end
for (; _First != _Last; ++_First) {
value_type _Val = *_First;
_Data.keys.emplace_back(_STD move(_Val.first));
Expand All @@ -882,12 +942,12 @@ private:
auto _Sorted_view = _View_to_mutate();
if constexpr (_NeedSorting) {
auto _Sorted_new_elements = _Sorted_view;
_Sorted_new_elements.advance(_Old_size);
_Sorted_new_elements.advance(_Old_distance);
_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() + _Old_size, value_compare(_Key_compare));
_RANGES inplace_merge(_Sorted_view, _Sorted_view.begin() + _Old_distance, value_compare(_Key_compare));

if constexpr (_NeedDeduping) {
_Dedup();
Expand Down Expand Up @@ -1285,8 +1345,7 @@ flat_map(from_range_t, _Rng&&, _Compare = _Compare(), _Allocator = _Allocator())
#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>>>>;
_Compare, vector<_Range_key_type<_Rng>>, vector<_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>,
Expand Down Expand Up @@ -1423,8 +1482,7 @@ flat_multimap(from_range_t, _Rng&&, _Compare = _Compare(), _Allocator = _Allocat
#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>>>>;
_Range_mapped_type<_Rng>, _Compare, vector<_Range_key_type<_Rng>>, vector<_Range_mapped_type<_Rng>>>;

template <_RANGES input_range _Rng, class _Compare, class _Allocator>
flat_multimap(from_range_t, _Rng&&, _Compare, _Allocator)
Expand Down
75 changes: 75 additions & 0 deletions tests/std/tests/P0429R9_flat_map/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,80 @@ void test_insert_or_assign() {
assert(check_value_content(direct_fm, {direct_init_only(direct_init_only::src_type{42u})}));
}

void test_comparison() {
{
flat_map<int, char> fm1{{1, '1'}, {2, '7'}, {3, '2'}, {4, '9'}};

assert(fm1 == fm1);
assert(!(fm1 != fm1));
assert(!(fm1 < fm1));
assert(!(fm1 > fm1));
assert(fm1 <= fm1);
assert(fm1 >= fm1);
assert(fm1 <=> fm1 == strong_ordering::equal);

flat_map<int, char> fm2{{1, '1'}, {7, '2'}, {2, '3'}, {9, '4'}};

assert(!(fm1 == fm2));
assert(fm1 != fm2);
assert(!(fm1 < fm2));
assert(fm1 > fm2);
assert(!(fm1 <= fm2));
assert(fm1 >= fm2);
assert(fm1 <=> fm2 == strong_ordering::greater);
}

{
flat_map<int, char, greater<int>> fm3{{1, '1'}, {2, '7'}, {3, '2'}, {4, '9'}};
flat_map<int, char, greater<int>> fm4{{1, '1'}, {7, '2'}, {2, '3'}, {9, '4'}};

assert(!(fm3 == fm4));
assert(fm3 != fm4);
assert(fm3 < fm4);
assert(!(fm3 > fm4));
assert(fm3 <= fm4);
assert(!(fm3 >= fm4));
assert(fm3 <=> fm4 == strong_ordering::less);
}
{
flat_multimap<int, char> fmm1{
{3, '2'}, {1, '7'}, {4, '1'}, {1, '8'}, {5, '2'}, {9, '8'}, {2, '1'}, {6, '8'}, {5, '2'}};

assert(fmm1 == fmm1);
assert(!(fmm1 != fmm1));
assert(!(fmm1 < fmm1));
assert(!(fmm1 > fmm1));
assert(fmm1 <= fmm1);
assert(fmm1 >= fmm1);
assert(fmm1 <=> fmm1 == strong_ordering::equal);

flat_multimap<int, char> fmm2{
{2, '3'}, {7, '1'}, {1, '4'}, {8, '1'}, {2, '5'}, {8, '9'}, {1, '2'}, {8, '6'}, {2, '5'}};

assert(!(fmm1 == fmm2));
assert(fmm1 != fmm2);
assert(!(fmm1 < fmm2));
assert(fmm1 > fmm2);
assert(!(fmm1 <= fmm2));
assert(fmm1 >= fmm2);
assert(fmm1 <=> fmm2 == strong_ordering::greater);
}
{
flat_multimap<int, char, greater<int>> fmm3{
{3, '2'}, {1, '7'}, {4, '1'}, {1, '8'}, {5, '2'}, {9, '8'}, {2, '1'}, {6, '8'}, {5, '2'}};
flat_multimap<int, char, greater<int>> fmm4{
{2, '3'}, {7, '1'}, {1, '4'}, {8, '1'}, {2, '5'}, {8, '9'}, {1, '2'}, {8, '6'}, {2, '5'}};

assert(!(fmm3 == fmm4));
assert(fmm3 != fmm4);
assert(!(fmm3 < fmm4));
assert(fmm3 > fmm4);
assert(!(fmm3 <= fmm4));
assert(fmm3 >= fmm4);
assert(fmm3 <=> fmm4 == strong_ordering::greater);
}
}

// Test MSVC STL-specific SCARY-ness

template <bool>
Expand Down Expand Up @@ -628,4 +702,5 @@ int main() {
test_insert();
test_gh_4344();
test_insert_or_assign();
test_comparison();
}