diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 860d21b8a..7bf37c7bd 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -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 | @@ -162,7 +166,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 | @@ -180,9 +184,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 | @@ -207,16 +211,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 ``` diff --git a/include/podio/UserDataCollection.h b/include/podio/UserDataCollection.h index d49a07011..dae5c4c40 100644 --- a/include/podio/UserDataCollection.h +++ b/include/podio/UserDataCollection.h @@ -81,6 +81,8 @@ class UserDataCollection : public CollectionBase { using iterator = typename std::vector::iterator; using difference_type = typename std::vector::difference_type; using size_type = typename std::vector::size_type; + using const_reverse_iterator = typename std::vector::const_reverse_iterator; + using reverse_iterator = typename std::vector::reverse_iterator; UserDataCollection() = default; /// Constructor from an existing vector (which will be moved from!) @@ -222,6 +224,25 @@ class UserDataCollection : public CollectionBase { const_iterator cend() const { return _vec.cend(); } + // reverse iterators + reverse_iterator rbegin() { + return _vec.rbegin(); + } + const_reverse_iterator rbegin() const { + return _vec.rbegin(); + } + const_reverse_iterator crbegin() const { + return _vec.crbegin(); + } + reverse_iterator rend() { + return _vec.rend(); + } + const_reverse_iterator rend() const { + return _vec.rend(); + } + const_reverse_iterator crend() const { + return _vec.crend(); + } typename std::vector::reference operator[](size_t idx) { return _vec[idx]; diff --git a/include/podio/detail/LinkCollectionImpl.h b/include/podio/detail/LinkCollectionImpl.h index c4b9c20ce..947f7d68f 100644 --- a/include/podio/detail/LinkCollectionImpl.h +++ b/include/podio/detail/LinkCollectionImpl.h @@ -16,6 +16,8 @@ #include "podio/utilities/MaybeSharedPtr.h" #include "podio/utilities/TypeHelpers.h" +#include + #ifdef PODIO_JSON_OUTPUT #include "nlohmann/json.hpp" #endif @@ -48,6 +50,8 @@ class LinkCollection : public podio::CollectionBase { using iterator = LinkMutableCollectionIterator; using difference_type = ptrdiff_t; using size_type = size_t; + using const_reverse_iterator = std::reverse_iterator; + using reverse_iterator = std::reverse_iterator; LinkCollection() = default; @@ -172,6 +176,25 @@ class LinkCollection : public podio::CollectionBase { iterator end() { return iterator(m_storage.entries.size(), &m_storage.entries); } + // 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(); + } bool isValid() const override { return m_isValid; diff --git a/include/podio/detail/LinkCollectionIterator.h b/include/podio/detail/LinkCollectionIterator.h index b51c2fb40..b8bb89c6e 100644 --- a/include/podio/detail/LinkCollectionIterator.h +++ b/include/podio/detail/LinkCollectionIterator.h @@ -17,7 +17,9 @@ class LinkCollectionIteratorT { using reference = LinkType; using pointer = LinkType*; 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; LinkCollectionIteratorT(size_t index, const LinkObjPointerContainer* coll) : m_index(index), m_object(podio::utils::MaybeSharedPtr{nullptr}), m_collection(coll) { @@ -29,8 +31,8 @@ class LinkCollectionIteratorT { LinkCollectionIteratorT& operator=(LinkCollectionIteratorT&&) = default; ~LinkCollectionIteratorT() = default; - bool operator!=(const LinkCollectionIteratorT& other) const { - return m_index != other.m_index; + auto operator<=>(const LinkCollectionIteratorT& other) const { + return m_index <=> other.m_index; } bool operator==(const LinkCollectionIteratorT& other) const { @@ -57,6 +59,51 @@ class LinkCollectionIteratorT { return copy; } + LinkCollectionIteratorT& operator--() { + --m_index; + return *this; + } + + LinkCollectionIteratorT operator--(int) { + auto copy = *this; + --m_index; + return copy; + } + + LinkCollectionIteratorT& operator+=(difference_type n) { + m_index += n; + return *this; + } + + LinkCollectionIteratorT operator+(difference_type n) const { + auto copy = *this; + copy += n; + return copy; + } + + friend LinkCollectionIteratorT operator+(difference_type n, const LinkCollectionIteratorT& it) { + return it + n; + } + + LinkCollectionIteratorT& operator-=(difference_type n) { + m_index -= n; + return *this; + } + + LinkCollectionIteratorT operator-(difference_type n) const { + auto copy = *this; + copy -= n; + return copy; + } + + LinkType operator[](difference_type n) const { + return LinkType{podio::utils::MaybeSharedPtr((*m_collection)[m_index + n])}; + } + + difference_type operator-(const LinkCollectionIteratorT& other) const { + return m_index - other.m_index; + } + private: size_t m_index{0}; LinkType m_object{podio::utils::MaybeSharedPtr{nullptr}}; diff --git a/python/templates/Collection.h.jinja2 b/python/templates/Collection.h.jinja2 index 53be2ddb3..1ee2364c7 100644 --- a/python/templates/Collection.h.jinja2 +++ b/python/templates/Collection.h.jinja2 @@ -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; + using reverse_iterator = std::reverse_iterator; {{ class.bare_type }}Collection(); {{ class.bare_type }}Collection({{ class.bare_type }}CollectionData&& data, bool isSubsetColl); @@ -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; diff --git a/python/templates/macros/iterator.jinja2 b/python/templates/macros/iterator.jinja2 index d3b512f94..f56b7287c 100644 --- a/python/templates/macros/iterator.jinja2 +++ b/python/templates/macros/iterator.jinja2 @@ -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; @@ -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 { @@ -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}; @@ -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 %} diff --git a/tests/unittests/links.cpp b/tests/unittests/links.cpp index eb80b076f..405141eb3 100644 --- a/tests/unittests/links.cpp +++ b/tests/unittests/links.cpp @@ -453,6 +453,19 @@ TEST_CASE("Links with interfaces", "[links][interface-types]") { REQUIRE(link.get() == cluster); } +TEST_CASE("Links reverse iterators", "[links][iterator]") { + const auto [linkColl, hitColl, clusterColl] = createLinkCollections(); + REQUIRE(linkColl.size() > 1); + auto it = --linkColl.rend(); + REQUIRE(*it == *linkColl.begin()); + it = linkColl.rbegin(); + REQUIRE(*it == *--linkColl.end()); + auto cit = --linkColl.rend(); + REQUIRE(*cit == *linkColl.cbegin()); + cit = linkColl.crbegin(); + REQUIRE(*cit == *--linkColl.cend()); +} + #ifdef PODIO_JSON_OUTPUT TEST_CASE("Link JSON conversion", "[links][json]") { const auto& [links, hits, clusters] = createLinkCollections(); diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index 6f4acc890..bb51cd8b3 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -495,6 +495,239 @@ TEST_CASE("Collection and iterator concepts", "[collection][container][iterator] // const_iterator STATIC_REQUIRE(std::input_iterator); } + + SECTION("forward_iterator") { + // iterator + STATIC_REQUIRE(std::forward_iterator); + { + REQUIRE(iterator{} == iterator{}); + auto coll = CollectionType(); + coll.create(); + auto i = coll.begin(); + auto j = coll.begin(); + REQUIRE(i == j); + REQUIRE(++i == ++j); + i = coll.begin(); + REQUIRE(((void)[](auto x) { ++x; }(i), *i) == *i); + } + // const_iterator + STATIC_REQUIRE(std::forward_iterator); + { + REQUIRE(const_iterator{} == const_iterator{}); + auto coll = CollectionType(); + coll.create(); + auto i = coll.cbegin(); + auto j = coll.cbegin(); + REQUIRE(i == j); + REQUIRE(++i == ++j); + i = coll.cbegin(); + REQUIRE(((void)[](auto x) { ++x; }(i), *i) == *i); + } + } + + SECTION("bidirectional_iterator") { + // iterator + STATIC_REQUIRE(std::bidirectional_iterator); + { + auto coll = CollectionType(); + coll.create(); + auto a = ++coll.begin(); + REQUIRE(std::addressof(--a) == std::addressof(a)); + a = ++coll.begin(); + auto b = ++coll.begin(); + REQUIRE(a == b); + REQUIRE(a-- == b); + a = ++coll.begin(); + REQUIRE(a == b); + a--; + --b; + REQUIRE(a == b); + a = ++coll.begin(); + b = ++coll.begin(); + REQUIRE(a == b); + REQUIRE(--(++a) == b); + REQUIRE(++(--a) == b); + } + // const_iterator + STATIC_REQUIRE(std::bidirectional_iterator); + auto coll = CollectionType(); + coll.create(); + auto a = ++coll.cbegin(); + REQUIRE(std::addressof(--a) == std::addressof(a)); + a = ++coll.cbegin(); + auto b = ++coll.cbegin(); + REQUIRE(a == b); + REQUIRE(a-- == b); + a = ++coll.cbegin(); + REQUIRE(a == b); + a--; + --b; + REQUIRE(a == b); + a = ++coll.cbegin(); + b = ++coll.cbegin(); + REQUIRE(a == b); + REQUIRE(--(++a) == b); + REQUIRE(++(--a) == b); + } + + SECTION("random_access_iterator") { + // iterator + STATIC_REQUIRE(std::totally_ordered); + { + auto coll = CollectionType(); + coll.create(); + coll.create(); + coll.create(); + auto a = coll.begin(); + auto b = coll.begin(); + REQUIRE(!(a < b)); + REQUIRE(!(a > b)); + REQUIRE((a == b)); + b = ++coll.begin(); + REQUIRE((a < b)); + REQUIRE(!(a > b)); + REQUIRE(!(a == b)); + a = ++coll.begin(); + b = coll.begin(); + REQUIRE(!(a < b)); + REQUIRE(a > b); + REQUIRE(!(a == b)); + auto c = coll.begin(); + a = c++; + b = c++; + REQUIRE(a < b); + REQUIRE(b < c); + REQUIRE(a < c); + REQUIRE((a > b) == (b < a)); + REQUIRE((a >= b) == !(a < b)); + REQUIRE((a <= b) == !(a > b)); + } + STATIC_REQUIRE(std::sized_sentinel_for); + { + auto coll = CollectionType(); + coll.create(); + auto i = coll.begin(); + auto s = coll.end(); + auto n = 1; + REQUIRE(s - i == n); + REQUIRE(i - s == -n); + } + STATIC_REQUIRE(std::random_access_iterator); + { + auto coll = CollectionType(); + coll.create().cellID(42); + coll.create().cellID(43); + coll.create().cellID(44); + coll.create().cellID(45); + auto a = coll.begin(); + auto n = 2; + auto b = a + n; + REQUIRE((a += n) == b); + a = coll.begin(); + REQUIRE(std::addressof(a += n) == std::addressof(a)); + a = coll.begin(); + auto k = a + n; + REQUIRE(k == (a += n)); + a = coll.begin(); + REQUIRE((a + n) == (n + a)); + auto x = 1; + auto y = 2; + REQUIRE((a + (x + y)) == ((a + x) + y)); + REQUIRE((a + 0) == a); + b = a + n; + REQUIRE((--b) == (a + n - 1)); + b = a + n; + REQUIRE((b += -n) == a); + b = a + n; + REQUIRE((b -= +n) == a); + b = a + n; + REQUIRE(std::addressof(b -= n) == std::addressof(b)); + b = a + n; + k = b - n; + REQUIRE(k == (b -= n)); + b = a + n; + REQUIRE(a[n] == *b); + REQUIRE(a <= b); + } + // const_iterator + STATIC_REQUIRE(std::totally_ordered); + { + auto coll = CollectionType(); + coll.create(); + coll.create(); + coll.create(); + auto a = coll.cbegin(); + auto b = coll.cbegin(); + REQUIRE(!(a < b)); + REQUIRE(!(a > b)); + REQUIRE((a == b)); + b = ++coll.cbegin(); + REQUIRE((a < b)); + REQUIRE(!(a > b)); + REQUIRE(!(a == b)); + a = ++coll.cbegin(); + b = coll.cbegin(); + REQUIRE(!(a < b)); + REQUIRE(a > b); + REQUIRE(!(a == b)); + auto c = coll.cbegin(); + a = c++; + b = c++; + REQUIRE(a < b); + REQUIRE(b < c); + REQUIRE(a < c); + REQUIRE((a > b) == (b < a)); + REQUIRE((a >= b) == !(a < b)); + REQUIRE((a <= b) == !(a > b)); + } + STATIC_REQUIRE(std::sized_sentinel_for); + { + auto coll = CollectionType(); + coll.create(); + auto i = coll.cbegin(); + auto s = coll.cend(); + auto n = 1; + REQUIRE(s - i == n); + REQUIRE(i - s == -n); + } + STATIC_REQUIRE(std::random_access_iterator); + { + auto coll = CollectionType(); + coll.create().cellID(42); + coll.create().cellID(43); + coll.create().cellID(44); + coll.create().cellID(45); + auto a = coll.cbegin(); + auto n = 2; + auto b = a + n; + REQUIRE((a += n) == b); + a = coll.cbegin(); + REQUIRE(std::addressof(a += n) == std::addressof(a)); + a = coll.cbegin(); + auto k = a + n; + REQUIRE(k == (a += n)); + a = coll.cbegin(); + REQUIRE((a + n) == (n + a)); + auto x = 1; + auto y = 2; + REQUIRE((a + (x + y)) == ((a + x) + y)); + REQUIRE((a + 0) == a); + b = a + n; + REQUIRE((--b) == (a + n - 1)); + b = a + n; + REQUIRE((b += -n) == a); + b = a + n; + REQUIRE((b -= +n) == a); + b = a + n; + REQUIRE(std::addressof(b -= n) == std::addressof(b)); + b = a + n; + k = b - n; + REQUIRE(k == (b -= n)); + b = a + n; + REQUIRE(a[n] == *b); + REQUIRE(a <= b); + } + } } TEST_CASE("Collection and unsupported iterator concepts", "[collection][container][iterator][std]") { @@ -508,17 +741,12 @@ TEST_CASE("Collection and unsupported iterator concepts", "[collection][containe DOCUMENTED_STATIC_FAILURE(std::output_iterator); DOCUMENTED_STATIC_FAILURE(std::output_iterator); DOCUMENTED_STATIC_FAILURE(std::output_iterator); - // std::forward_iterator - DOCUMENTED_STATIC_FAILURE(std::forward_iterator); - DOCUMENTED_STATIC_FAILURE(std::forward_iterator); - // std::bidirectional_iterator - DOCUMENTED_STATIC_FAILURE(std::bidirectional_iterator); - DOCUMENTED_STATIC_FAILURE(std::bidirectional_iterator); - // std::random_access_iterator - DOCUMENTED_STATIC_FAILURE(std::random_access_iterator); - DOCUMENTED_STATIC_FAILURE(std::random_access_iterator); // std::contiguous_iterator + DOCUMENTED_STATIC_FAILURE(std::is_lvalue_reference_v); + DOCUMENTED_STATIC_FAILURE(std::same_as>); DOCUMENTED_STATIC_FAILURE(std::contiguous_iterator); + DOCUMENTED_STATIC_FAILURE(std::is_lvalue_reference_v); + STATIC_REQUIRE(std::same_as>); DOCUMENTED_STATIC_FAILURE(std::contiguous_iterator); } @@ -922,15 +1150,63 @@ TEST_CASE("Collection and std iterator adaptors", "[collection][container][adapt STATIC_REQUIRE(traits::has_iterator_category_v>); DOCUMENTED_STATIC_FAILURE( std::is_base_of_v::iterator_category>); - DOCUMENTED_STATIC_FAILURE(std::bidirectional_iterator); - // TODO add runtime checks here + STATIC_REQUIRE(std::bidirectional_iterator); + { + auto coll = CollectionType(); + coll.create().cellID(42); + coll.create().cellID(43); + auto rit = std::reverse_iterator(std::end(coll)); + DOCUMENTED_STATIC_FAILURE( + traits::has_member_of_pointer_v); // can't -> because + // std::reverse_iterator::operator->() + // is const + // REQUIRE(rit->cellID() == 43); + REQUIRE((*rit).cellID() == 43); + ++rit; + // REQUIRE(rit->cellID() == 42); + REQUIRE((*rit).cellID() == 42); + REQUIRE(rit.base()->cellID() == 43); + rit = std::reverse_iterator(std::begin(coll)); + REQUIRE(rit.base()->cellID() == 42); + --rit; + // REQUIRE(rit->cellID() == 42); + REQUIRE((*rit).cellID() == 42); + REQUIRE(rit.base()->cellID() == 43); + --rit; + // REQUIRE(rit->cellID() == 43); + REQUIRE((*rit).cellID() == 43); + } // const_iterator STATIC_REQUIRE(traits::has_const_iterator_v); STATIC_REQUIRE(traits::has_iterator_category_v>); DOCUMENTED_STATIC_FAILURE( std::is_base_of_v::iterator_category>); - DOCUMENTED_STATIC_FAILURE(std::bidirectional_iterator); - // TODO add runtime checks here + STATIC_REQUIRE(std::bidirectional_iterator); + { + auto coll = CollectionType(); + coll.create().cellID(42); + coll.create().cellID(43); + auto rit = std::reverse_iterator(std::cend(coll)); + DOCUMENTED_STATIC_FAILURE( + traits::has_member_of_pointer_v); // can't -> because + // std::reverse_iterator::operator->() + // is const + // REQUIRE(rit->cellID() == 43); + REQUIRE((*rit).cellID() == 43); + ++rit; + // REQUIRE(rit->cellID() == 42); + REQUIRE((*rit).cellID() == 42); + REQUIRE(rit.base()->cellID() == 43); + rit = std::reverse_iterator(std::cbegin(coll)); + REQUIRE(rit.base()->cellID() == 42); + --rit; + // REQUIRE(rit->cellID() == 42); + REQUIRE((*rit).cellID() == 42); + REQUIRE(rit.base()->cellID() == 43); + --rit; + // REQUIRE(rit->cellID() == 43); + REQUIRE((*rit).cellID() == 43); + } } SECTION("Back inserter") { DOCUMENTED_STATIC_FAILURE(traits::has_const_reference_v); @@ -1085,11 +1361,11 @@ TEST_CASE("Collection as range", "[collection][ranges][std]") { DOCUMENTED_STATIC_FAILURE(std::ranges::output_range); DOCUMENTED_STATIC_FAILURE(std::ranges::output_range); // std::range::forward_range - DOCUMENTED_STATIC_FAILURE(std::ranges::forward_range); + STATIC_REQUIRE(std::ranges::forward_range); // std::range::bidirectional_range - DOCUMENTED_STATIC_FAILURE(std::ranges::bidirectional_range); + STATIC_REQUIRE(std::ranges::bidirectional_range); // std::range::random_access_range - DOCUMENTED_STATIC_FAILURE(std::ranges::random_access_range); + STATIC_REQUIRE(std::ranges::random_access_range); // std::range::contiguous_range DOCUMENTED_STATIC_FAILURE(std::ranges::contiguous_range); // std::range::common_range @@ -1154,13 +1430,17 @@ TEST_CASE("Collection and std ranges algorithms", "[collection][ranges][std]") { REQUIRE(subcoll.size() == 2); REQUIRE(subcoll[0].cellID() == 5); REQUIRE(subcoll[1].cellID() == 3); -} -// helper concept for unsupported algorithm compilation test -template -concept is_range_adjacent_findable = requires(T coll) { - std::ranges::adjacent_find(coll, [](const auto& a, const auto& b) { return a.cellID() == b.cellID(); }); -}; + auto adjacent_it = + std::ranges::adjacent_find(coll, [](const auto& a, const auto& b) { return a.cellID() == b.cellID(); }); + REQUIRE(adjacent_it != std::end(coll)); + auto target = std::begin(coll); + std::ranges::advance(target, 2); + REQUIRE(adjacent_it == target); + + auto rev_view = std::ranges::views::reverse(coll); + REQUIRE(rev_view.front().cellID() == 3); +} // helper concept for unsupported algorithm compilation test template @@ -1173,23 +1453,148 @@ concept is_range_fillable = requires(T coll) { std::ranges::fill(coll, typename TEST_CASE("Collection and unsupported std ranges algorithms", "[collection][ranges][std]") { // check that algorithms requiring unsupported iterator concepts won't compile - DOCUMENTED_STATIC_FAILURE(is_range_adjacent_findable); DOCUMENTED_STATIC_FAILURE(is_range_sortable); DOCUMENTED_STATIC_FAILURE(is_range_fillable); } -TEST_CASE("LinkCollectionIterator and iterator concepts", "[links][iterator][std]") { +TEST_CASE("LinkCollectionIterator and iterator concepts", "[links][iterators][std]") { using link_iterator = podio::LinkCollectionIteratorT; using link_const_iterator = podio::LinkCollectionIteratorT; + using link_collection = podio::LinkCollection; STATIC_REQUIRE(std::input_iterator); STATIC_REQUIRE(std::input_iterator); + STATIC_REQUIRE(std::forward_iterator); + STATIC_REQUIRE(std::forward_iterator); + SECTION("bidirectional_iterator") { + STATIC_REQUIRE(std::bidirectional_iterator); + { + auto coll = link_collection(); + coll.create(); + auto a = ++coll.begin(); + REQUIRE(std::addressof(--a) == std::addressof(a)); + a = ++coll.begin(); + auto b = ++coll.begin(); + REQUIRE(a == b); + REQUIRE(a-- == b); + a = ++coll.begin(); + REQUIRE(a == b); + a--; + --b; + REQUIRE(a == b); + a = ++coll.begin(); + b = ++coll.begin(); + REQUIRE(a == b); + REQUIRE(--(++a) == b); + REQUIRE(++(--a) == b); + } + STATIC_REQUIRE(std::bidirectional_iterator); + { + auto coll = link_collection(); + coll.create(); + auto a = ++coll.cbegin(); + REQUIRE(std::addressof(--a) == std::addressof(a)); + a = ++coll.cbegin(); + auto b = ++coll.cbegin(); + REQUIRE(a == b); + REQUIRE(a-- == b); + a = ++coll.cbegin(); + REQUIRE(a == b); + a--; + --b; + REQUIRE(a == b); + a = ++coll.cbegin(); + b = ++coll.cbegin(); + REQUIRE(a == b); + REQUIRE(--(++a) == b); + REQUIRE(++(--a) == b); + } + } + SECTION("random_access_iterator") { + STATIC_REQUIRE(std::random_access_iterator); + { + auto coll = link_collection(); + coll.create(); + coll.create(); + coll.create(); + coll.create(); + auto a = coll.begin(); + auto n = 2; + auto b = a + n; + REQUIRE((a += n) == b); + a = coll.begin(); + REQUIRE(std::addressof(a += n) == std::addressof(a)); + a = coll.begin(); + auto k = a + n; + REQUIRE(k == (a += n)); + a = coll.begin(); + REQUIRE((a + n) == (n + a)); + auto x = 1; + auto y = 2; + REQUIRE((a + (x + y)) == ((a + x) + y)); + REQUIRE((a + 0) == a); + b = a + n; + REQUIRE((--b) == (a + n - 1)); + b = a + n; + REQUIRE((b += -n) == a); + b = a + n; + REQUIRE((b -= +n) == a); + b = a + n; + REQUIRE(std::addressof(b -= n) == std::addressof(b)); + b = a + n; + k = b - n; + REQUIRE(k == (b -= n)); + b = a + n; + REQUIRE(a[n] == *b); + REQUIRE(a <= b); + } + STATIC_REQUIRE(std::random_access_iterator); + { + auto coll = link_collection(); + coll.create(); + coll.create(); + coll.create(); + coll.create(); + auto a = coll.cbegin(); + auto n = 2; + auto b = a + n; + REQUIRE((a += n) == b); + a = coll.cbegin(); + REQUIRE(std::addressof(a += n) == std::addressof(a)); + a = coll.cbegin(); + auto k = a + n; + REQUIRE(k == (a += n)); + a = coll.cbegin(); + REQUIRE((a + n) == (n + a)); + auto x = 1; + auto y = 2; + REQUIRE((a + (x + y)) == ((a + x) + y)); + REQUIRE((a + 0) == a); + b = a + n; + REQUIRE((--b) == (a + n - 1)); + b = a + n; + REQUIRE((b += -n) == a); + b = a + n; + REQUIRE((b -= +n) == a); + b = a + n; + REQUIRE(std::addressof(b -= n) == std::addressof(b)); + b = a + n; + k = b - n; + REQUIRE(k == (b -= n)); + b = a + n; + REQUIRE(a[n] == *b); + REQUIRE(a <= b); + } + } } -TEST_CASE("LinkCollection and range concepts", "[links][iterator][std]") { +TEST_CASE("LinkCollection and range concepts", "[links][ranges][std]") { using link_collection = podio::LinkCollection; STATIC_REQUIRE(std::ranges::input_range); + STATIC_REQUIRE(std::ranges::forward_range); + STATIC_REQUIRE(std::ranges::bidirectional_range); + STATIC_REQUIRE(std::ranges::random_access_range); STATIC_REQUIRE(std::ranges::sized_range); STATIC_REQUIRE(std::ranges::common_range); STATIC_REQUIRE(std::ranges::viewable_range); diff --git a/tests/unittests/unittest.cpp b/tests/unittests/unittest.cpp index 0e0210915..851287bf5 100644 --- a/tests/unittests/unittest.cpp +++ b/tests/unittests/unittest.cpp @@ -206,6 +206,45 @@ TEST_CASE("Looping", "[basics]") { } } +TEST_CASE("Reverse iterators", "[basics]") { + auto coll = ExampleHitCollection(); + coll.create(); + coll.create(); + auto it = std::rbegin(coll); + (*it).energy(43); + (*++it).energy(42); + REQUIRE((*it).energy() == 42); + REQUIRE((*--it).energy() == 43); + it = std::rend(coll); + REQUIRE((*--it).energy() == 42); + REQUIRE((*--it).energy() == 43); + auto cit = std::crbegin(coll); + REQUIRE((*cit).energy() == 43); + REQUIRE((*++cit).energy() == 42); + cit = std::crend(coll); + REQUIRE((*--cit).energy() == 42); + REQUIRE((*--cit).energy() == 43); +} + +TEST_CASE("UserDataCollection reverse iterators", "[basics]") { + auto coll = podio::UserDataCollection(); + coll.push_back(42); + coll.push_back(43); + auto it = std::rbegin(coll); + REQUIRE(*it == 43); + REQUIRE(*++it == 42); + REQUIRE(*--it == 43); + it = std::rend(coll); + REQUIRE(*--it == 42); + REQUIRE(*--it == 43); + auto cit = std::crbegin(coll); + REQUIRE(*cit == 43); + REQUIRE(*++cit == 42); + cit = std::crend(coll); + REQUIRE(*--cit == 42); + REQUIRE(*--cit == 43); +} + TEST_CASE("Notebook", "[basics]") { auto hits = ExampleHitCollection(); for (unsigned i = 0; i < 12; ++i) {