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

[range.iter.ops], default_sentinel, and unreachable_sentinel #329

Merged
merged 1 commit into from
Dec 2, 2019

Conversation

CaseyCarter
Copy link
Member

Description

Implements iterator primitive operations std::ranges::advance, std::ranges::next, std::ranges::prev, and std::ranges::distance; as well as std::default_sentinel and std::unreachable_sentinel.

This change reworks the STL's iterator unwrapping machinery to enable unwrapping of C++20 move-only single-pass iterators (and if constepxrs all the things). Consequently, _Iter_ref_t, _Iter_value_t, and _Iter_diff_t resolve to iter_reference_t, iter_value_t, and iter_difference_t (respectively) in __cpp_lib_concepts (soon to be C++20) mode. This change necessitates some fixes to unique_copy and _Fill_memset_is_safe which both assume that _Iter_value_t<T> is well-formed for any iterator T. (iter_value_t<T> does not have that property: it is only well-formed when readable<T>.)

I notably haven't unified default_sentinel_t with _Default_sentinel out of an abundance of paranoia. Our move_iterator is comparable with _Default_sentinel, which is not the case for std::default_sentinel.

Drive-by:

  • This change if constexpr-izes unique_copy.

[This is a replay of Microsoft-internal PR MSVC-PR-214209.]

Checklist

Be sure you've read README.md and understand the scope of this repo.

If you're unsure about a box, leave it unchecked. A maintainer will help you.

  • Identifiers in product code changes are properly _Ugly as per
    https://eel.is/c++draft/lex.name#3.1 or there are no product code changes.
  • The STL builds successfully and all tests have passed (must be manually
    verified by an STL maintainer before automated testing is enabled on GitHub,
    leave this unchecked for initial submission).
  • These changes introduce no known ABI breaks (adding members, renaming
    members, adding virtual functions, changing whether a type is an aggregate
    or trivially copyable, etc.).
  • These changes were written from scratch using only this repository,
    the C++ Working Draft (including any cited standards), other WG21 papers
    (excluding reference implementations outside of proposed standard wording),
    and LWG issues as reference material. If they were derived from a project
    that's already listed in NOTICE.txt, that's fine, but please mention it.
    If they were derived from any other project (including Boost and libc++,
    which are not yet listed in NOTICE.txt), you must mention it here,
    so we can determine whether the license is compatible and what else needs
    to be done.

Implements iterator primitive operations `std::ranges::advance`, `std::ranges::next`, `std::ranges::prev`, and `std::ranges::distance`; as well as `std::default_sentinel` and `std::unreachable_sentinel`.

This change reworks the STL's iterator unwrapping machinery to enable unwrapping of C++20 move-only single-pass iterators (and `if constepxr`s all the things). Consequently, `_Iter_ref_t`, `_Iter_value_t`, and `_Iter_diff_t` resolve to `iter_reference_t`, `iter_value_t`, and `iter_difference_t` (respectively) in `__cpp_lib_concepts` (soon to be C++20) mode. This change necessitates some fixes to `unique_copy` and `_Fill_memset_is_safe` which both assume that `_Iter_value_t<T>` is well-formed for any iterator `T`. (`iter_value_t<T>` does not have that property: it is only well-formed when `readable<T>`.)

I notably haven't unified `default_sentinel_t` with `_Default_sentinel` out of an abundance of paranoia. Our `move_iterator` is comparable with `_Default_sentinel`, which is not the case for `std::default_sentinel`.

Drive-by:
* This change `if constexpr`-izes `unique_copy`.
@CaseyCarter CaseyCarter requested a review from a team as a code owner November 26, 2019 12:14
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.

I am just a dude so this feels like my two year old telling me how to speak properly but meh...

There is a lot of __foo to _Foo noise going on here. I would suggest to put that into a different commit, so that it doesnt drain too much mental capacity. Also there are quite some places where this was not applied.

The same goes for _STD to _RANGES and the _Unwrapped_t<Foo> to _Unwrapped_t<const Foo&> If they are separated into their own commit they are easily verifiable and do not drain so much energy form the real changes

stl/inc/xutility Show resolved Hide resolved
stl/inc/xutility Show resolved Hide resolved
stl/inc/xutility Show resolved Hide resolved
stl/inc/xutility Show resolved Hide resolved
stl/inc/xutility Show resolved Hide resolved
stl/inc/xutility Show resolved Hide resolved
@CaseyCarter
Copy link
Member Author

There is a lot of __foo to _Foo noise going on here.

The same goes for _STD to _RANGES and the _Unwrapped_t<Foo> to _Unwrapped_t<const Foo&>

Ironically enough, all three of these cases were expansions of the PR requested during internal review. __meow to _Meow was admittedly separable, but the others were considered necessary to ensure the correctness of the overall change.

Hopefully we'll have the test infrastructure online soon so this silly dual-review process can stop and we can properly work completely out of GitHub.

@miscco
Copy link
Contributor

miscco commented Nov 27, 2019

@CaseyCarter Sorry for the noise. The internal conventions are quite hard to get. Thanks for the explanations

stl/inc/xutility Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cxx20 C++20 feature ranges C++20/23 ranges
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants