Skip to content
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.

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

P1206R7 Conversions From Ranges To Containers #2806

Merged
merged 38 commits into from
Aug 9, 2022

Conversation

CaseyCarter
Copy link
Member

<ranges>: Implement ranges::to, guarded by __cpp_lib_ranges_to_container.

The following are all guarded by __cpp_lib_containers_ranges:
<xmemory>: Generalize _Uninitialized_copy to iterator+sentinel ranges; add _Uninitialized_copy_n for counted ranges. Implement from_range_t, from_range, and exposition-only helper concepts / type aliases.
<xutility>: Promote _Get_final_iterator_unwrapped here from <algorithm>. Add _Copy_memmove_n (_Copy_n_unchecked), similar to _Copy_memmove (resp. _Copy_unchecked) but for counted ranges. Generalize _Iter_copy_cat to _Sent_copy_cat.
<memory>: Use _Copy_memmove_n (see <xutility>) as appropriate.

<yvals_core.h>: _HAS_CXX23 controls P1206R7, define __cpp_lib_containers_ranges and __cpp_lib_ranges_to_container.

<deque>: Implement from_range_t constructor(s) and corresponding deduction guide(s), prepend_range, append_range, assign_range, and insert_range.
<forward_list>: Implement from_range_t constructor(s) and corresponding deduction guide(s), prepend_range, assign_range, and insert_range_after.
<list>: Implement from_range_t constructor(s) and corresponding deduction guide(s), prepend_range, append_range, assign_range, and insert_range.
<map> and <set>: For both map, multimap, set, and multiset, implement from_range_t constructor(s) and corresponding deduction guide(s). (Inherit insert_range from _Tree.)
<queue>: For both queue and priority_queue, implement from_range_t constructor(s) and corresponding deduction guide(s), and push_range. (I've speculatively implemented priority_queue::push_range by calling append_range on the container per the resolution I've proposed for an LWG issue I submitted specifically to allow implementation via append_range.)
<stack>: Implement from_range_t constructor(s) and corresponding deduction guide(s), and push_range.
<unordered_map> and <unordered_set>: For both unordered_map, unordered_multimap, unordered_set, and unordered_multiset, implement from_range_t constructor(s) and corresponding deduction guide(s). (Inherit insert_range from _Hash.)
<vector>: For both vector and vector<bool>, implement from_range_t constructor(s) and corresponding deduction guide(s), append_range, assign_range, and insert_range.
<xstring>: For both basic_string, implement from_range_t constructor(s) and corresponding deduction guide(s), append_range, assign_range, insert_range, and replace_with_range.

Test both new feature-test macros in tests/std/tests/VSO_0157762_feature_test_macros.
Test all new deduction guides in tests/std/tests/P0433R2_deduction_guides.
Test _Copy_n_unchecked in tests/std/tests/P0784R7_library_machinery.
Add new tP1206R7_{container}_{operation} "range algorithm"-style tests for each new container (or container adapter) member function.
Add new P1206R7_from_range test for from_range and from_range_t.
Add new P1206R7_ranges_to test for ranges::to.

Fixes #2532.

`<ranges>`: Implement `ranges::to`, guarded by `__cpp_lib_ranges_to_container`.

The following are all guarded  by `__cpp_lib_containers_ranges`:
`<xmemory>`: Generalize `_Uninitialized_copy` to iterator+sentinel ranges; add `_Uninitialized_copy_n` for counted ranges. Implement `from_range_t`, `from_range`, and exposition-only helper concepts / type aliases.
`<xutility>`: Promote `_Get_final_iterator_unwrapped` here from `<algorithm>`. Add `_Copy_memmove_n` (`_Copy_n_unchecked`), similar to `_Copy_memmove` (resp. `_Copy_unchecked`) but for counted ranges. Generalize `_Iter_copy_cat` to `_Sent_copy_cat`.
`<memory>`: Use `_Copy_memmove_n` (see `<xutility>`) as appropriate.

`<yvals_core.h>`: `_HAS_CXX23` controls P1206R7, define `__cpp_lib_containers_ranges` and `__cpp_lib_ranges_to_container`.

`<deque>`: Implement `from_range_t` constructor(s) and corresponding deduction guide(s), `prepend_range`, `append_range`, `assign_range`, and `insert_range`.
`<forward_list>`: Implement `from_range_t` constructor(s) and corresponding deduction guide(s), `prepend_range`, `assign_range`, and `insert_range_after`.
`<list>`: Implement `from_range_t` constructor(s) and corresponding deduction guide(s), `prepend_range`, `append_range`, `assign_range`, and `insert_range`.
`<map>` and `<set>`: For both `map`, `multimap`, `set`, and `multiset`, implement `from_range_t` constructor(s) and corresponding deduction guide(s). (Inherit `insert_range` from `_Tree`.)
`<queue>`: For both `queue` and `priority_queue`, implement `from_range_t` constructor(s) and corresponding deduction guide(s), and `push_range`. (I've speculatively implemented `priority_queue::push_range` by calling `append_range` on the container per the resolution I've proposed for an LWG issue I submitted specifically to allow implementation via `append_range`.)
`<stack>`: Implement `from_range_t` constructor(s) and corresponding deduction guide(s), and `push_range`.
`<unordered_map>` and `<unordered_set>`: For both `unordered_map`, `unordered_multimap`, `unordered_set`, and `unordered_multiset`, implement `from_range_t` constructor(s) and corresponding deduction guide(s). (Inherit `insert_range` from `_Hash`.)
`<vector>`: For both `vector` and `vector<bool>`, implement `from_range_t` constructor(s) and corresponding deduction guide(s), `append_range`, `assign_range`, and `insert_range`.
`<xstring>`: For both `basic_string`, implement `from_range_t` constructor(s) and corresponding deduction guide(s), `append_range`, `assign_range`, `insert_range`, and `replace_with_range`.

Test both new feature-test macros in `tests/std/tests/VSO_0157762_feature_test_macros`.
Test all new deduction guides in `tests/std/tests/P0433R2_deduction_guides`.
Test `_Copy_n_unchecked` in `tests/std/tests/P0784R7_library_machinery`.
Add new t`P1206R7_{container}_{operation}` "range algorithm"-style tests for each new container (or container adapter) member function.
Add new `P1206R7_from_range` test for `from_range` and `from_range_t`.
Add new `P1206R7_ranges_to` test for `ranges::to`.

Fixes microsoft#2532.
@CaseyCarter CaseyCarter added ranges C++20/23 ranges cxx23 C++23 feature labels Jun 20, 2022
@CaseyCarter CaseyCarter requested a review from a team as a code owner June 20, 2022 22:03
Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

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

There are numerous real test failures - please investigate, fix, and verify before we begin reviewing this small PR. 😹

stl/inc/xtree Outdated Show resolved Hide resolved
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja left a comment

Choose a reason for hiding this comment

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

A strange value 202200L is used to test feature-test macros.

I have not idea why the test for deque failed now...

Copy link
Contributor

@frederick-vs-ja frederick-vs-ja left a comment

Choose a reason for hiding this comment

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

We shouldn't use _RERAISE again (see #2308).

stl/inc/deque Outdated Show resolved Hide resolved
stl/inc/deque Outdated Show resolved Hide resolved
stl/inc/forward_list Outdated Show resolved Hide resolved
Copy link
Contributor

@miscco miscco left a comment

Choose a reason for hiding this comment

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

Partial review until ranges header

stl/inc/algorithm Show resolved Hide resolved
stl/inc/deque Show resolved Hide resolved
stl/inc/deque Outdated Show resolved Hide resolved
stl/inc/deque Outdated Show resolved Hide resolved
stl/inc/deque Outdated Show resolved Hide resolved
stl/inc/deque Outdated Show resolved Hide resolved
stl/inc/deque Outdated Show resolved Hide resolved
stl/inc/deque Outdated Show resolved Hide resolved
stl/inc/deque Show resolved Hide resolved
stl/inc/forward_list Outdated Show resolved Hide resolved
Copy link
Contributor

@miscco miscco left a comment

Choose a reason for hiding this comment

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

Partial review until xstring

stl/inc/ranges Outdated Show resolved Hide resolved
stl/inc/vector Show resolved Hide resolved
stl/inc/vector Outdated Show resolved Hide resolved
stl/inc/vector Outdated Show resolved Hide resolved
stl/inc/xstring Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej self-assigned this Jun 22, 2022
Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

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

Extremely partial video code review 😹

stl/inc/xutility Show resolved Hide resolved
stl/inc/xutility Outdated Show resolved Hide resolved
stl/inc/xutility Outdated Show resolved Hide resolved
stl/inc/xutility Outdated Show resolved Hide resolved
stl/inc/xutility Show resolved Hide resolved
stl/inc/list Outdated Show resolved Hide resolved
stl/inc/xutility Outdated Show resolved Hide resolved
stl/inc/xutility Show resolved Hide resolved
stl/inc/xmemory Outdated Show resolved Hide resolved
stl/inc/ranges Show resolved Hide resolved
tests/std/tests/P1206R7_string_from_range/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1206R7_vector_insert_range/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1206R7_ranges_to/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1206R7_ranges_to/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P1206R7_ranges_to/test.cpp Outdated Show resolved Hide resolved
@StephanTLavavej
Copy link
Member

@CaseyCarter @strega-nil-ms I've pushed changes nicely structured as a series of commits for double-checking. Please meow if you have meows!

@StephanTLavavej StephanTLavavej removed their assignment Aug 8, 2022
Copy link
Contributor

@strega-nil-ms strega-nil-ms left a comment

Choose a reason for hiding this comment

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

I am so excited to have ranges::to for tests!

excited totoro with friends

@CaseyCarter
Copy link
Member Author

@CaseyCarter @strega-nil-ms I've pushed changes nicely structured as a series of commits for double-checking. Please meow if you have meows!

Thanks for keeping these short and simple - very easy to review. You know, compared to, say, a hypothetical 7,000 line change.

... per resolution of just-submitted LWG issue.
@StephanTLavavej StephanTLavavej self-assigned this Aug 9, 2022
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 56446a5 into microsoft:main Aug 9, 2022
@StephanTLavavej
Copy link
Member

Thanks for implementing this tiny feature - I believe that to is the shortest function name in the STL! 🐈 😻 😹

@CaseyCarter CaseyCarter deleted the p1206 branch August 9, 2022 16:28
fsb4000 pushed a commit to fsb4000/STL that referenced this pull request Aug 13, 2022
&& constructible_from<_Container, from_range_t, _Rng, _Types...>;
concept _Converts_tag_constructible = _Ref_converts<_Rng, _Container>
// per LWG issue unnumbered as of 2022-08-08
&& constructible_from<_Container, const from_range_t&, _Rng, _Types...>;
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed this issue recently too.
And when I used MSVC-STL for resolution validation I found that you already dealt with it. Are you planning to file this LWG and stride_view::iterator::operator+= recently?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cxx23 C++23 feature ranges C++20/23 ranges
Projects
None yet
Development

Successfully merging this pull request may close these issues.

P1206R7 Conversions From Ranges To Containers
7 participants