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 fulfill LegacyInputIterator and input_iterator concept #626

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
91 changes: 45 additions & 46 deletions doc/collections_as_container.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ The PODIO `Collection`s interface was designed to mimic the standard *Container*
On the implementation level most of the differences with respect to the *Container* comes from the fact that in order to satisfy the additional semantics a `Collection` doesn't directly store [user layer objects](design.md#the-user-layer). Instead, [data layer objects](design.md#the-internal-data-layer) are stored and user layer objects are constructed and returned when needed. Similarly, the `Collection` iterators operate on the user layer objects but don't expose `Collection`'s storage directly to the users. Instead, they construct and return user layer objects when needed.
In other words, a `Collection` utilizes the user layer type as a reference type instead of using plain references (`&` or `&&`) to stored data layer types.

As a consequence some of the **standard algorithms may not work** with PODIO `Collection` iterators. See [standard algorithm documentation](#collection-and-standard-algorithms) below.
As a consequence some of the **standard algorithms may not work** with PODIO `Collection` iterators. See [standard algorithm documentation](#collection-and-standard-algorithms) below.

The following tables list the compliance of a PODIO generated collection with the *Container* named requirement, stating which member types, interfaces, or concepts are fulfilled and which are not. Additionally, there are some comments explaining missing parts or pointing out differences in behaviour.

Expand All @@ -16,12 +16,12 @@ The following tables list the compliance of a PODIO generated collection with th
| Name | Type | Requirements | Fulfilled by Collection? | Comment |
|------|------|--------------|--------------------------|---------|
| `value_type` | `T` | *[Erasable](https://en.cppreference.com/w/cpp/named_req/Erasable)* | ✔️ yes | Defined as an immutable user layer object type |
| `reference` | `T&` | | ❌ no | Not defined |
| `reference` | `T&` | | ❌ no | Not defined |
| `const_reference` | `const T&` | | ❌ no | Not defined |
| `iterator` | Iterator whose `value_type` is `T` | [*LegacyForwardIterator*](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) convertible to `const_iterator` | ❌ no | Defined as podio `MutableCollectionIterator`. `iterator::value_type` not defined, not [*LegacyForwardIterator*](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) ([see below](#legacyforwarditerator)), not convertible to `const_iterator`|
| `const_iterator` | Constant iterator whose `value_type` is `T` | [*LegacyForwardIterator*](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) | ❌ no | Defined as podio `CollectionIterator`. `const_iterator::value_type` not defined, not [*LegacyForwardIterator*](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) ([see below](#legacyforwarditerator))
| `difference_type`| Signed integer | Must be the same as `std::iterator_traits::difference_type` for `iterator` and `const_iterator` | ❌ no | `std::iterator_traits::difference_type` not defined |
| `size_type` | Unsigned integer | Large enough to represent all positive values of `difference_type` | ✔️ yes | |
| `iterator` | Iterator whose `value_type` is `T` | [*LegacyForwardIterator*](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) convertible to `const_iterator` | ❌ no | Defined as podio `MutableCollectionIterator`. Not [*LegacyForwardIterator*](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) ([see below](#legacyforwarditerator)), not convertible to `const_iterator`|
| `const_iterator` | Constant iterator whose `value_type` is `T` | [*LegacyForwardIterator*](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) | ❌ no | Defined as podio `CollectionIterator`. Not [*LegacyForwardIterator*](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) ([see below](#legacyforwarditerator))
| `difference_type`| Signed integer | Must be the same as `std::iterator_traits::difference_type` for `iterator` and `const_iterator` | ✔️ yes | |
| `size_type` | Unsigned integer | Large enough to represent all positive values of `difference_type` | ✔️ yes | |

### Container member functions and operators

Expand All @@ -31,18 +31,18 @@ The following tables list the compliance of a PODIO generated collection with th
| `C(a)` | `C` | Creates a copy of `a` | ❌ no | Not defined, non-copyable by design |
| `C(rv)` | `C` | Moves `rv` | ✔️ yes | |
| `a = b` | `C&` | Destroys or copy-assigns all elements of `a` from elements of `b` | ❌ no | Not defined, non-copyable by design |
| `a = rv` | `C&` | Destroys or move-assigns all elements of `a` from elements of `rv` | ✔️ yes | |
| `a.~C()` | `void` | Destroys all elements of `a` and frees all memory| ✔️ yes | Invalidates all handles retrieved from this collection |
| `a.begin()` | `(const_)iterator` | Iterator to the first element of `a` | ✔️ yes | |
| `a = rv` | `C&` | Destroys or move-assigns all elements of `a` from elements of `rv` | ✔️ yes | |
| `a.~C()` | `void` | Destroys all elements of `a` and frees all memory| ✔️ yes | Invalidates all handles retrieved from this collection |
| `a.begin()` | `(const_)iterator` | Iterator to the first element of `a` | ✔️ yes | |
| `a.end()` | `(const_)iterator` | Iterator to one past the last element of `a` | ✔️ yes | |
| `a.cbegin()` | `const_iterator` | Same as `const_cast<const C&>(a).begin()` | ✔️ yes | |
| `a.cend()` | `const_iterator` | Same as `const_cast<const C&>(a).end()`| ✔️ yes | |
| `a.cend()` | `const_iterator` | Same as `const_cast<const C&>(a).end()`| ✔️ yes | |
| `a == b` | Convertible to `bool` | Same as `std::equal(a.begin(), a.end(), b.begin(), b.end())`| ❌ no | Not defined |
| `a != b` | Convertible to `bool` | Same as `!(a == b)` | ❌ no | Not defined |
| `a.swap(b)` | `void` | Exchanges the values of `a` and `b` | ❌ no | Not defined |
| `swap(a,b)` | `void` | Same as `a.swap(b)` | ❌ no | `a.swap(b)` not defined |
| `a.size()` | `size_type` | Same as `std::distance(a.begin(), a.end())` | ✔️ yes | |
| `a.max_size()` | `size_type` | `b.size()` where b is the largest possible container | ✔️ yes | |
| `a.max_size()` | `size_type` | `b.size()` where `b` is the largest possible container | ✔️ yes | |
| `a.empty()` | Convertible to `bool` | Same as `a.begin() == a.end()` | ✔️ yes | |

## Collection as an *AllocatorAwareContainer*
Expand All @@ -53,9 +53,9 @@ PODIO collections don't provide a customization point for allocators and use onl

### AllocatorAwareContainer types

| Name | Requirements | Fulfilled by Collection? | Comment |
| Name | Requirements | Fulfilled by Collection? | Comment |
|------|--------------|--------------------------|---------|
| `allocator_type` | `allocator_type::value_type` same as `value_type` | ❌ no | `allocator_type` not defined |
| `allocator_type` | `allocator_type::value_type` same as `value_type` | ❌ no | `allocator_type` not defined |

### *AllocatorAwareContainer* expression and statements

Expand All @@ -70,8 +70,8 @@ In the following tables a convention from `Collection` is used: `iterator` stand

| Named requirement | `iterator` | `const_iterator` |
|-------------------|-----------------------|-----------------------------|
| [LegacyIterator](https://en.cppreference.com/w/cpp/named_req/Iterator) | ❌ no ([see below](#legacyiterator)) | ❌ no ([see below](#legacyiterator)) |
| [LegacyInputIterator](https://en.cppreference.com/w/cpp/named_req/InputIterator) | ❌ no ([see below](#legacyinputiterator)) | ❌ no ([see below](#legacyinputiterator)) |
| [LegacyIterator](https://en.cppreference.com/w/cpp/named_req/Iterator) | ✔️ yes ([see below](#legacyiterator)) | ✔️ yes ([see below](#legacyiterator)) |
| [LegacyInputIterator](https://en.cppreference.com/w/cpp/named_req/InputIterator) | ✔️ yes ([see below](#legacyinputiterator)) | ✔️ yes ([see below](#legacyinputiterator)) |
| [LegacyForwardIterator](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) | ❌ no ([see below](#legacyforwarditerator)) | ❌ no ([see below](#legacyforwarditerator)) |
| [LegacyOutputIterator](https://en.cppreference.com/w/cpp/named_req/OutputIterator) | ❌ no ([see below](#legacyoutputiterator)) | ❌ no ([see below](#legacyoutputiterator)) |
| [LegacyBidirectionalIterator](https://en.cppreference.com/w/cpp/named_req/BidirectionalIterator) | ❌ no | ❌ no |
Expand All @@ -82,9 +82,9 @@ In the following tables a convention from `Collection` is used: `iterator` stand
|---------|------------------------|------------------------------|
| `std::indirectly_readable` | ❌ no | ❌ no |
| `std::indirectly_writable` | ❌ no | ❌ no |
| `std::weakly_incrementable` | ❌ no | ❌ no |
| `std::incrementable` | ❌ no | ❌ no |
| `std::input_or_output_iterator` | ❌ no | ❌ no |
| `std::weakly_incrementable` | ✔️ yes | ✔️ yes |
| `std::incrementable` | ✔️ yes | ✔️ yes |
| `std::input_or_output_iterator` | ✔️ yes | ✔️ yes |
| `std::input_iterator` | ❌ no | ❌ no |
| `std::output_iterator` | ❌ no | ❌ no |
| `std::forward_iterator` | ❌ no | ❌ no |
Expand All @@ -96,15 +96,15 @@ In the following tables a convention from `Collection` is used: `iterator` stand

| Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment |
|-------------|-------------------------------------------|---------|
| [*CopyConstructible*](https://en.cppreference.com/w/cpp/named_req/CopyConstructible) | ❌ no / ❌ no | Move constructor and copy constructor not defined |
| [*CopyAssignable*](https://en.cppreference.com/w/cpp/named_req/CopyAssignable) | ❌ no / ❌ no | Move assignment and copy assignment not defined |
| [*CopyConstructible*](https://en.cppreference.com/w/cpp/named_req/CopyConstructible) | ✔️ yes / ✔️ yes | |
| [*CopyAssignable*](https://en.cppreference.com/w/cpp/named_req/CopyAssignable) | ✔️ yes / ✔️ yes | |
| [*Destructible*](https://en.cppreference.com/w/cpp/named_req/Destructible) | ✔️ yes / ✔️ yes | |
| [*Swappable*](https://en.cppreference.com/w/cpp/named_req/Swappable) | ❌ no / ❌ no | |
| `std::iterator_traits::value_type` (Until C++20 ) | ❌ no / ❌ no | Not defined |
| `std::iterator_traits::difference_type` | ❌ no / ❌ no | Not defined |
| `std::iterator_traits::reference` | ❌ no / ❌ no | Not defined |
| `std::iterator_traits::pointer` | ❌ no / ❌ no | Not defined |
| `std::iterator_traits::iterator_category` | ❌ no / ❌ no | Not defined |
| [*Swappable*](https://en.cppreference.com/w/cpp/named_req/Swappable) | ✔️ yes / ✔️ yes | |
| `std::iterator_traits::value_type` (Until C++20 ) | ✔️ yes / ✔️ yes | |
| `std::iterator_traits::difference_type` | ✔️ yes / ✔️ yes | |
| `std::iterator_traits::reference` | ✔️ yes / ✔️ yes | |
| `std::iterator_traits::pointer` | ✔️ yes / ✔️ yes | |
| `std::iterator_traits::iterator_category` | ✔️ yes / ✔️ yes | |

| Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment |
|------------|-------------|-----------|-------------------------------------------|---------|
Expand All @@ -115,49 +115,48 @@ In the following tables a convention from `Collection` is used: `iterator` stand

| Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment |
|-------------|-------------------------------------------|---------|
| [*LegacyIterator*](https://en.cppreference.com/w/cpp/named_req/Iterator) | ❌ no / ❌ no | [See above](#legacyiterator) |
| [*LegacyIterator*](https://en.cppreference.com/w/cpp/named_req/Iterator) | ✔️ yes / ✔️ yes | [See above](#legacyiterator) |
| [*EqualityComparable*](https://en.cppreference.com/w/cpp/named_req/EqualityComparable) | ✔️ yes / ✔️ yes | |

| Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment |
|------------|-------------|-----------|-------------------------------------------|---------|
| `i != j` | Contextually convertible to `bool` | Same as `!(i==j)` | ✔️ yes / ✔️ yes | |
| `*i` | `reference`, convertible to `value_type` | | ❌ no / ❌ no | `reference` and `value_type` not defined |
| `i != j` | Contextually convertible to `bool` | Same as `!(i==j)` | ✔️ yes / ✔️ yes | |
| `*i` | `reference`, convertible to `value_type` | | ✔️ yes / ✔️ yes | |
| `i->m` | | Same as `(*i).m` | ✔️ yes / ✔️ yes | |
| `++r` | `It&` | | ✔️ yes / ✔️ yes | |
| `(void)r++` | | Same as `(void)++r` | ❌ no / ❌ no | Post-increment not defined |
| `*r++` | Convertible to `value_type` | Same as `value_type x = *r; ++r; return x;` | ❌ no / ❌ no | Post-increment and `value_type` not defined |
| `(void)r++` | | Same as `(void)++r` | ✔️ yes / ✔️ yes | |
| `*r++` | Convertible to `value_type` | Same as `value_type x = *r; ++r; return x;` | ✔️ yes / ✔️ yes | |

### LegacyForwardIterator

In addition to the *LegacyForwardIterator* the C++ standard specifies also the *mutable LegacyForwardIterator*, which is both *LegacyForwardIterator* and *LegacyOutputIterator*. The term **mutable** used in this context doesn't imply mutability in the sense used in the PODIO.


| Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment |
|-------------|-------------------------------------------|---------|
| [*LegacyInputIterator*](https://en.cppreference.com/w/cpp/named_req/InputIterator) | ❌ no / ❌ no | [See above](#legacyinputiterator)|
| [*DefaultConstructible*](https://en.cppreference.com/w/cpp/named_req/DefaultConstructible) | ❌ no / ❌ no | Value initialization not defined |
| If *mutable* iterator then `reference` same as `value_type&` or `value_type&&`, otherwise same as `const value_type&` or `const value_type&&` | ❌ no / ❌ no | `reference` and `value_type` not defined |
| [Multipass guarantee](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) | ❌ no / ❌ no | Copy constructor not defined |
| [Singular iterators](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) | ❌ no / ❌ no | Value initialization not defined |
| [*LegacyInputIterator*](https://en.cppreference.com/w/cpp/named_req/InputIterator) | ✔️ yes / ✔️ yes | [See above](#legacyinputiterator)|
| [*DefaultConstructible*](https://en.cppreference.com/w/cpp/named_req/DefaultConstructible) | ✔️ yes / ✔️ yes | |
| If *mutable* iterator then `reference` same as `value_type&` or `value_type&&`, otherwise same as `const value_type&` or `const value_type&&` | ❌ no / ❌ no | `reference` type is not a reference (`&` or `&&`) |
| [Multipass guarantee](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) | ❌ no / ❌ no | References from dereferencing equal iterators aren't bound to the same object |
| [Singular iterators](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) | ✔️ yes / ✔️ yes | |

| Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment |
|------------|-------------|-----------|-------------------------------------------|---------|
| `i++` | `It` | Same as `It ip = i; ++i; return ip;` | ❌ no / ❌ no | Post-increment not defined |
| `*i++` | `reference` | | ❌ no / ❌ no | Post-increment and `reference` not defined |
| `i++` | `It` | Same as `It ip = i; ++i; return ip;` | ✔️ yes / ✔️ yes | |
| `*i++` | `reference` | | ✔️ yes / ✔️ yes | |

### LegacyOutputIterator

| Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment |
|-------------|-------------------------------------------|---------|
| [*LegacyIterator*](https://en.cppreference.com/w/cpp/named_req/Iterator) | ❌ no / ❌ no | [See above](#legacyiterator) |
| [*LegacyIterator*](https://en.cppreference.com/w/cpp/named_req/Iterator) | ✔️ yes / ✔️ yes | [See above](#legacyiterator) |
| Is pointer type or class type | ✔️ yes / ✔️ yes | |

| Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment |
|------------|-------------|-----------|-------------------------------------------|---------|
| `*r = o` | | | ❗ attention / ❗ attention | Defined but an assignment doesn't modify objects inside collection |
| `++r` | `It&` | | ✔️ yes / ✔️ yes | |
| `r++` | Convertible to `const It&` | Same as `It temp = r; ++r; return temp;` | ❌ no / ❌ no | Post-increment not defined |
| `*r++ = o` | | Same as `*r = o; ++r;`| ❌ no / ❌ no | Post-increment not defined |
| `++r` | `It&` | | ✔️ yes / ✔️ yes | |
| `r++` | Convertible to `const It&` | Same as `It temp = r; ++r; return temp;` | ✔️ yes / ✔️ yes | |
| `*r++ = o` | | Same as `*r = o; ++r;`| ❗ attention / ❗ attention | Defined but an assignment doesn't modify objects inside collection |

## Collection iterators and standard iterator adaptors

Expand All @@ -167,9 +166,9 @@ In addition to the *LegacyForwardIterator* the C++ standard specifies also the *
| `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 |
| `std::const_iterator` | ❌ no | `iterator` and `const_iterator` not *LegacyInputIterator* or `std::input_iterator` |
| `std::move_iterator` | ❌ no | `iterator` and `const_iterator` not *LegacyInputIterator* or `std::input_iterator` |
| `std::counted_iterator` | ❌ no | `iterator` and `const_iterator` not `std::input_or_output_iterator` |
| `std::const_iterator` (C++23) | ❌ no | C++23 not supported yet |
| `std::move_iterator` | ✔️ yes | Limited usefulness since dereference returns `reference` not rvalue (`&&`) |
| `std::counted_iterator` | ✔️ yes | |


## Collection and standard algorithms
Expand Down
29 changes: 23 additions & 6 deletions python/templates/macros/iterator.jinja2
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,20 @@
{% set ptr_init = 'podio::utils::MaybeSharedPtr<' + class.bare_type +'Obj>{nullptr}' %}
class {{ iterator_type }} {
public:
using value_type = {{ class.bare_type }};
using difference_type = ptrdiff_t;
using reference = {{ prefix }}{{ class.bare_type }};
using pointer = {{ prefix }}{{ class.bare_type }}*;
using iterator_category = std::input_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 }}(): {{ iterator_type }}(0, nullptr) {}
m-fila marked this conversation as resolved.
Show resolved Hide resolved

{{ iterator_type }}(const {{ iterator_type }}&) = delete;
{{ iterator_type }}& operator=(const {{ iterator_type }}&) = delete;
{{ iterator_type }}(const {{ iterator_type }}&) = default;
{{ iterator_type }}({{ iterator_type }}&&) = default;
{{ iterator_type }}& operator=(const {{ iterator_type }}&) = default;
{{ iterator_type }}& operator=({{iterator_type}}&&) = default;
m-fila marked this conversation as resolved.
Show resolved Hide resolved
~{{ iterator_type }}() = default;

bool operator!=(const {{ iterator_type}}& x) const {
return m_index != x.m_index; // TODO: may not be complete
Expand All @@ -16,9 +26,10 @@ public:
return m_index == x.m_index; // TODO: may not be complete
}

{{ prefix }}{{ class.bare_type }} operator*();
{{ prefix }}{{ class.bare_type }}* operator->();
reference operator*();
pointer operator->();
{{ iterator_type }}& operator++();
{{ iterator_type }} operator++(int);

private:
size_t m_index;
Expand All @@ -32,12 +43,12 @@ private:
{% macro iterator_definitions(class, prefix='') %}
{% with iterator_type = class.bare_type + prefix + 'CollectionIterator' %}
{% set ptr_type = 'podio::utils::MaybeSharedPtr<' + class.bare_type +'Obj>' %}
{{ prefix }}{{ class.bare_type }} {{ iterator_type }}::operator*() {
{{ iterator_type }}::reference {{ iterator_type }}::operator*() {
m_object.m_obj = {{ ptr_type }}((*m_collection)[m_index]);
return m_object;
}

{{ prefix }}{{ class.bare_type }}* {{ iterator_type }}::operator->() {
{{ iterator_type }}::pointer {{ iterator_type }}::operator->() {
m_object.m_obj = {{ ptr_type }}((*m_collection)[m_index]);
return &m_object;
}
Expand All @@ -47,5 +58,11 @@ private:
return *this;
}

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

{% endwith %}
{% endmacro %}
Loading