Skip to content

<flat_set>: flat_(multi)set's iterator type should be constant iterator#4132

Merged
StephanTLavavej merged 7 commits intomicrosoft:feature/flat_setfrom
achabense:_Flat_set_iterator_type_fix
Nov 7, 2023
Merged

<flat_set>: flat_(multi)set's iterator type should be constant iterator#4132
StephanTLavavej merged 7 commits intomicrosoft:feature/flat_setfrom
achabense:_Flat_set_iterator_type_fix

Conversation

@achabense
Copy link
Contributor

@achabense achabense commented Oct 27, 2023

Citation:

For associative containers where the value type is the same as the key type, both iterator and const_iterator are constant iterators. It is unspecified whether or not iterator and const_iterator are the same type.

This pr makes flat_(multi)set's iterator an alias for const_iterator.

@achabense achabense requested a review from a team as a code owner October 27, 2023 21:10
@CaseyCarter
Copy link
Contributor

FWIW, I usually hear "flat_meow" used to collectively refer to flat_map, flat_multimap, flat_set, and flat_multiset.

achabense and others added 2 commits October 28, 2023 07:46
@StephanTLavavej StephanTLavavej added enhancement Something can be improved flat_meow C++23 container adaptors labels Oct 30, 2023
@achabense achabense changed the title <flat_set>: flat_meow's iterator type should be constant iterator <flat_set>: flat_(multi)set's iterator type should be constant iterator Oct 30, 2023
template <_Different_from<_Kty> _Other>
requires (
_Keylt_transparent && !is_convertible_v<_Other, iterator> && !is_convertible_v<_Other, const_iterator>)
requires (_Keylt_transparent && !is_convertible_v<_Other, const_iterator>)
Copy link
Contributor Author

@achabense achabense Oct 30, 2023

Choose a reason for hiding this comment

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

Is it ok not to add comment for this one? Or am I adding too many repetitive comments?

(Also, there are some remaining iterator in return types (erase etc). I find it trickey to decide whether to return const_iterator/iterator. Returning iterator is more obviously standard-conformant, but that also hides some informations...)

Copy link
Member

Choose a reason for hiding this comment

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

I probably would have commented this, but it's not really more confusing than the other locations which silently assume that they're the same type. I think this is fine to merge as-is.

@StephanTLavavej StephanTLavavej self-assigned this Oct 31, 2023
@CaseyCarter CaseyCarter self-assigned this Nov 1, 2023
template <_Different_from<_Kty> _Other>
requires (
_Keylt_transparent && !is_convertible_v<_Other, iterator> && !is_convertible_v<_Other, const_iterator>)
requires (_Keylt_transparent && !is_convertible_v<_Other, const_iterator>)
Copy link
Member

Choose a reason for hiding this comment

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

I probably would have commented this, but it's not really more confusing than the other locations which silently assume that they're the same type. I think this is fine to merge as-is.

@StephanTLavavej StephanTLavavej merged commit cf59a16 into microsoft:feature/flat_set Nov 7, 2023
@StephanTLavavej
Copy link
Member

Thanks for constantly improving <flat_set>! 😹 🎉 😻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Something can be improved flat_meow C++23 container adaptors

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants