Skip to content

Conversation

@miscco
Copy link
Contributor

@miscco miscco commented Feb 15, 2021

This implements spaceship for the container iterators

@miscco miscco requested a review from a team as a code owner February 15, 2021 21:15
@StephanTLavavej StephanTLavavej added cxx20 C++20 feature spaceship C++20 operator <=> labels Feb 16, 2021
Copy link
Contributor

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

What about basic_string and basic_string_view? (span is also notably covered by the Standard requirement, but we certainly have <=> for its iterators with test coverage elsewhere.)

@CaseyCarter CaseyCarter self-assigned this Feb 16, 2021
@miscco
Copy link
Contributor Author

miscco commented Feb 16, 2021

basic_string_view

Urgh char_traits and string literal overloads...

@CaseyCarter
Copy link
Contributor

basic_string_view

Urgh char_traits and string literal overloads...

The iterator <=> doesn't care about these things. (And the range <=> is in another PR so not your problem.)

@miscco
Copy link
Contributor Author

miscco commented Feb 16, 2021

basic_string_view

Urgh char_traits and string literal overloads...

The iterator <=> doesn't care about these things. (And the range <=> is in another PR so not your problem.)

I just pushed a commit for that ;) I like it when you agree that others should do the hard work

Copy link
Contributor

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

I've made a small change to add TRANSITION, GH-1635 to the two commented-out lines that would/could/should cover that work.

`TRANSITION` the commented-out code.
@CaseyCarter CaseyCarter removed their assignment Feb 16, 2021
Copy link
Contributor

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

Apply warning cleanup for tests.

Cleanup unused variable warnings.
@StephanTLavavej StephanTLavavej changed the title Implement spaceship container iterators feature/spaceship: Clause 22: Container Iterators Feb 16, 2021
@StephanTLavavej
Copy link
Member

I've pushed the following changes:

  • Fix _HAS_CXX comment typo.
  • The _Deque_const_iterator spaceship shouldn't be constexpr.
  • Mark vector and vector<bool> iterator spaceships as _CONSTEXPR20_CONTAINER.
  • Fix _STL_VERIFY message for _String_view_iterator spaceship, which isn't equality.
  • Skip _Tree_const_iterator operator!= for C++20.
  • Implement spaceships for stdext::checked_array_iterator and stdext::unchecked_array_iterator.
  • Skip _Reinterpret_move_iter operator!= for C++20. (This is an internal iterator used to make unordered_meow moves more efficient.)
  • When testing queue, there's no need to test deque iterators again, as they were tested above.
  • Activate, reorder, and comment "string iterators", "string_view iterators" tests.
  • Test checked_array_iterator and unchecked_array_iterator.

@StephanTLavavej
Copy link
Member

Thanks for implementing this Secret Bonus Round of spaceships! 🥇 🛸 😹

@StephanTLavavej StephanTLavavej removed their assignment Feb 19, 2021
@miscco miscco deleted the spaceship_container_iterators branch February 25, 2021 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cxx20 C++20 feature spaceship C++20 operator <=>

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants