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

Feature/ranges part1 #7

Merged
merged 12 commits into from
Nov 8, 2023
Merged

Feature/ranges part1 #7

merged 12 commits into from
Nov 8, 2023

Conversation

whaeck
Copy link
Member

@whaeck whaeck commented Nov 1, 2023

This PR introduces a number of sfinae structures and aliases related to iterators and ranges.

This PR also introduces a ViewBase CRTP base class for views using these sfinae structures and aliases. The ViewBase assumes that the Derived class has a Derived::iterator type, a begin() and end() iterator. All methods with the exception of begin() and end() are made available depending on the type of the Derived::iterator type:

  • if Derived::iterator satisfies concepts::IsInputIterator: operator== and operator!=
  • if Derived::iterator satisfies concepts::IsForwardIterator: front(), size(), empty() and operator bool()
  • if Derived::iterator satisfies concepts::IsBidirectionalIterator: back()
  • if Derived::iterator satisfies concepts::IsRandomAccessIterator: operator[] and at()

The ViewBase interface should satisfy the C++20 view_interface, with the exception of data() which requires contiguous iterators (a concept introduced in C++20).

The ViewBase is now applied as the base class for the IteratorView which previously could only be used for random access iterators. It can now be used for all iterator types and the interface will be made available based on the underlying iterator type.

timburculosis
timburculosis previously approved these changes Nov 6, 2023
Copy link

@timburculosis timburculosis left a comment

Choose a reason for hiding this comment

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

Looks good, just one code cleanup bit to consider.

Choose a reason for hiding this comment

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

I know we discussed this already, but after reading through this I have another idea. Consider combining the two SFINAE'd functions into one function with a static assert. Since there's only two functions, one of which that works and the other that fails to compile, you don't need the SFINAE in the template arguments anymore. The static_asserts should handle what you were originally using SFINAE for - the code won't compile if the static_assert isn't met.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done! No sfinae, I like it :-)

@whaeck whaeck merged commit 28db74e into develop Nov 8, 2023
8 checks passed
@whaeck whaeck deleted the feature/ranges-part1 branch November 8, 2023 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants