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

Make collection iterators forward_iterator* and beyond #720

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 15 additions & 10 deletions doc/collections_as_container.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,15 @@ In the following tables a convention from `Collection` is used: `iterator` stand
| `std::input_or_output_iterator` | ✔️ yes | ✔️ yes |
| `std::input_iterator` | ✔️ yes | ✔️ yes |
| `std::output_iterator` | ❌ no | ❌ no |
| `std::forward_iterator` | ❌ no | ❌ no |
| `std::bidirectional_iterator` | ❌ no | ❌ no |
| `std::random_access_iterator` | ❌ no | ❌ no |
| `std::forward_iterator` | ✔️ yes (see note below) | ✔️ yes (see note below) |
| `std::bidirectional_iterator` | ✔️ yes | ✔️ yes |
| `std::random_access_iterator` | ✔️ yes | ✔️ yes |
| `std::contiguous_iterator` | ❌ no | ❌ no |

> [!NOTE]
>The collections' iterators fulfil the `std::forward_iterator` except that the pointers obtained with `->` remain valid only as long as the iterator is valid instead of as long as the range remain valid. In practice this means a `ptr` obtained with `auto* ptr = it.operator->();` is valid only as long as `it` is valid.
>The values obtained immediately through `->` (for instance `auto& e = it->energy();`) behaves as expected for `std::forward_iterator` as their validity is tied to the validity of a collection.

### LegacyIterator

| Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment |
Expand Down Expand Up @@ -163,7 +167,7 @@ In addition to the *LegacyForwardIterator* the C++ standard specifies also the *

| Adaptor | Compatible with Collection? | Comment |
|---------|-----------------------------|---------|
| `std::reverse_iterator` | ❌ no | `iterator` and `const_iterator` not *LegacyBidirectionalIterator* or `std::bidirectional_iterator` |
| `std::reverse_iterator` | ❗ attention | `operator->` not defined as `iterator`'s and `const_iterator`'s `operator->` are non-`const` |
| `std::back_insert_iterator` | ❗ attention | Compatible only with SubsetCollections, otherwise throws `std::invalid_argument` |
| `std::front_insert_iterator` | ❌ no | `push_front` not defined |
| `std::insert_iterator` | ❌ no | `insert` not defined |
Expand All @@ -181,9 +185,9 @@ In addition to the *LegacyForwardIterator* the C++ standard specifies also the *
| `std::ranges::sized_range` | ✔️ yes |
| `std::ranges::input_range` | ✔️ yes |
| `std::ranges::output_range` | ❌ no |
| `std::ranges::forward_range` | ❌ no |
| `std::ranges::bidirectional_range` | ❌ no |
| `std::ranges::random_access_range` | ❌ no |
| `std::ranges::forward_range` | ✔️ yes |
| `std::ranges::bidirectional_range` | ✔️ yes |
| `std::ranges::random_access_range` | ✔️ yes |
| `std::ranges::contiguous_range` | ❌ no |
| `std::ranges::common_range` | ✔️ yes |
| `std::ranges::viewable_range` | ✔️ yes |
Expand All @@ -208,16 +212,17 @@ std::sort(std::begin(collection), std::end(collection)); // requires RandomAcces

The arguments of standard range algorithms are checked at compile time and must fulfil certain iterator concepts, such as `std::input_iterator` or `std::ranges::input_range`.

The iterators of PODIO collections model the `std::input_iterator` concept, so range algorithms that require this iterator type will work correctly with PODIO iterators. If an algorithm compiles, it is guaranteed to work as expected.
The iterators of PODIO collections model the `std::random_access_iterator` concept, so range algorithms that require this iterator type will work correctly with PODIO iterators. If an algorithm compiles, it is guaranteed to work as expected.

In particular, the PODIO collections' iterators do not fulfil the `std::output_iterator` concept, and as a result, mutating algorithms relying on this iterator type will not compile.

Similarly the collections themselves model the `std::input_range` concept and can be used in the range algorithms that require that concept. The algorithms requiring unsupported range concept, such as `std::output_range`, won't compile.
Similarly the collections themselves model the `std::random_access_range` concept and can be used in the range algorithms that require that concept. The algorithms requiring unsupported range concept, such as `std::output_range`, won't compile.

For example:
```c++
std::ranges::find_if(collection, predicate ); // requires input_range -> OK
std::ranges::adjacent_find(collection, predicate ); // requires forward_range -> won't compile
std::ranges::adjacent_find(collection, predicate ); // requires forward_range -> OK
std::ranges::views::reverse(collection); //requires bidirectional_range -> OK
std::ranges::fill(collection, value ); // requires output_range -> won't compile
std::ranges::sort(collection); // requires random_access_range and sortable -> won't compile
```
22 changes: 22 additions & 0 deletions python/templates/Collection.h.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ public:
using iterator = {{ class.bare_type }}MutableCollectionIterator;
using difference_type = ptrdiff_t;
using size_type = size_t;
using const_reverse_iterator = std::reverse_iterator<const_iterator>;
using reverse_iterator = std::reverse_iterator<iterator>;

{{ class.bare_type }}Collection();
{{ class.bare_type }}Collection({{ class.bare_type }}CollectionData&& data, bool isSubsetColl);
Expand Down Expand Up @@ -167,6 +169,26 @@ public:
const_iterator cend() const {
return end();
}
// reverse iterators
reverse_iterator rbegin() {
return reverse_iterator(end());
}
const_reverse_iterator rbegin() const {
return const_reverse_iterator(end());
}
const_reverse_iterator crbegin() const {
return rbegin();
}
reverse_iterator rend() {
return reverse_iterator(begin());
}
const_reverse_iterator rend() const {
return const_reverse_iterator(begin());
}
const_reverse_iterator crend() const {
return rend();
}


{% for member in Members %}
std::vector<{{ member.full_type }}> {{ member.name }}(const size_t nElem = 0) const;
Expand Down
62 changes: 59 additions & 3 deletions python/templates/macros/iterator.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ public:
using reference = {{ prefix }}{{ class.bare_type }};
using pointer = {{ prefix }}{{ class.bare_type }}*;
using iterator_category = std::input_iterator_tag;
using iterator_concept = std::input_iterator_tag;
// `std::forward_iterator` is supported except that the pointers obtained with `operator->()`
// remain valid as long as the iterator is valid, not as long as the range is valid.
using iterator_concept = std::random_access_iterator_tag;

{{ iterator_type }}(size_t index, const {{ class.bare_type }}ObjPointerContainer* collection) : m_index(index), m_object({{ ptr_init }}), m_collection(collection) {}
{{ iterator_type }}() = default;
Expand All @@ -19,8 +21,8 @@ public:
{{ iterator_type }}& operator=({{iterator_type}}&&) = default;
~{{ iterator_type }}() = default;

bool operator!=(const {{ iterator_type}}& x) const {
return m_index != x.m_index;
auto operator<=>(const {{ iterator_type}}& other) const {
return m_index <=> other.m_index;
}

bool operator==(const {{ iterator_type }}& x) const {
Expand All @@ -31,6 +33,15 @@ public:
pointer operator->();
{{ iterator_type }}& operator++();
{{ iterator_type }} operator++(int);
{{ iterator_type }}& operator--();
{{ iterator_type }} operator--(int);
{{ iterator_type }}& operator+=(difference_type n);
{{ iterator_type }} operator+(difference_type n) const;
friend {{ iterator_type }} operator+(difference_type n, const {{ iterator_type }}& it);
{{ iterator_type }}& operator-=(difference_type n);
{{ iterator_type }} operator-(difference_type n) const;
reference operator[](difference_type n) const;
difference_type operator-(const {{ iterator_type }}& other) const;

private:
size_t m_index{0};
Expand Down Expand Up @@ -64,5 +75,50 @@ private:
return copy;
}

{{ iterator_type }}& {{ iterator_type }}::operator--() {
--m_index;
return *this;
}

{{ iterator_type }} {{ iterator_type }}::operator--(int) {
auto copy = *this;
--m_index;
return copy;
}

{{ iterator_type }}& {{ iterator_type }}::operator+=(difference_type n) {
m_index += n;
return *this;
}

{{ iterator_type }} {{ iterator_type }}::operator+(difference_type n) const {
auto copy = *this;
copy += n;
return copy;
}

{{ iterator_type }} operator+({{ iterator_type }}::difference_type n, const {{ iterator_type }}& it) {
return it + n;
}

{{ iterator_type }}& {{ iterator_type }}::operator-=(difference_type n) {
m_index -= n;
return *this;
}

{{ iterator_type }} {{ iterator_type }}::operator-(difference_type n) const {
auto copy = *this;
copy -= n;
return copy;
}

{{ iterator_type }}::reference {{ iterator_type }}::operator[](difference_type n) const {
return reference{ {{ ptr_type }}((*m_collection)[m_index + n]) };
}

{{ iterator_type }}::difference_type {{ iterator_type }}::operator-(const {{ iterator_type }}& other) const {
return m_index - other.m_index;
}

{% endwith %}
{% endmacro %}
Loading
Loading