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

Update "old" range types #95

Merged
merged 1 commit into from
Sep 16, 2019
Merged

Update "old" range types #95

merged 1 commit into from
Sep 16, 2019

Conversation

CaseyCarter
Copy link
Member

@CaseyCarter CaseyCarter commented Sep 14, 2019

Description

  • array, basic_string, basic_string_view, valarray, and vector (span is not yet implemented) model contiguous_range by defining the nested type iterator_concept to contiguous_iterator_tag in iterator and const_iterator, and specializing pointer_traits for those types (to fulfill contiguous_iterator's std::to_address requirement)

  • basic_string_view (Ditto, no span yet) models the exposition-only forwarding-range concept (which indicates that the validity of iterators is not bound to the lifetime of the range) by defining hidden-friend overloads of begin and end that accept rvalues

  • Drive-by:

    • mark the _Unfancy internal helper function [[nodiscard]]
    • Remove redundant _Can_begin requirement from std::ranges::_Begin::_Cpo::_Choose
  • Add test coverage to devcrt/P0896R4_ranges_range_machinery:

    • tighten up test_std_container:
      • data and cdata reject rvalue arguments since the returned pointer could potentially dangle (contiguous_range codepaths were lacking coverage)
      • the size_type of a standard container can be other than std::size_t when using fancy pointers
      • we should enforce that each container's iterator type models the expected concept
    • Add test coverage to ensure that contiguous standard library containers model contiguous_range even when using an "old" fancy pointer type that does not model contiguous_iterator

Checklist:

  • I understand README.md.
  • If this is a feature addition, that feature has been voted into the C++
    Working Draft.
  • Any code files edited have been processed by clang-format.
  • Identifiers in any product code changes are properly _Ugly as per
    https://eel.is/c++draft/lex.name#3.1 .
  • Identifiers in test code changes are not _Ugly.
  • Test code includes the correct headers as per the Standard, not just
    what happens to compile.
  • The STL builds and test harnesses have passed (must be manually verified
    by an STL maintainer before CI is online, leave this unchecked for initial
    submission).
  • This change introduces no known ABI breaks (adding members, renaming
    members, adding virtual functions, changing whether a type is an aggregate or
    trivially copyable, etc.). If unsure, leave this box unchecked and ask a
    maintainer for help.

This is a replay of Microsoft-internal pull request 201813.

* `array`, `basic_string`, `basic_string_view`, `valarray`, and `vector` (`span` is not yet implemented) model `contiguous_range` by defining the nested type `iterator_concept` to `contiguous_iterator_tag` in `iterator` and `const_iterator`, and specializing `pointer_traits` for those types (to fulfill `contiguous_iterator`'s `std::to_address` requirement)

* `basic_string_view` (Ditto, no `span` yet) models the exposition-only *`forwarding-range`* concept (which indicates that the validity of iterators is not bound to the lifetime of the range) by defining hidden-friend overloads of `begin` and `end` that accept rvalues

* Drive-by:
  * mark the `_Unfancy` internal helper function `[[nodiscard]]`
  * Remove redundant `_Can_begin` requirement from `std::ranges::_Begin::_Cpo::_Choose`

* Add test coverage to `devcrt/P0896R4_ranges_range_machinery`:
  * tighten up `test_std_container`:
    * `data` and `cdata` reject rvalue arguments since the returned pointer could potentially dangle (`contiguous_range` codepaths were lacking coverage)
    * the `size_type` of a standard container can be other than `std::size_t` when using fancy pointers
    * we should enforce that each container's iterator type models the expected concept
  * Add test coverage to ensure that contiguous standard library containers model `contiguous_range` even when using an "old" fancy pointer type that does not model `contiguous_iterator`
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.

Your commit notes describe "add test coverage", but the tests aren't on GitHub yet. No need to remove, just wanted to mention it. Otherwise looks good.

@CaseyCarter CaseyCarter merged commit 92508be into microsoft:master Sep 16, 2019
@CaseyCarter CaseyCarter deleted the replay branch September 16, 2019 01:41
@CaseyCarter CaseyCarter mentioned this pull request Mar 16, 2020
@StephanTLavavej StephanTLavavej added the ranges C++20/23 ranges label Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ranges C++20/23 ranges
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants