From d340640ea104333405c5bb5b4262f82ebcfbf1be Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Tue, 7 May 2024 14:54:37 +0200 Subject: [PATCH 01/43] Add document comparing collection with standard 'container' named requirement --- doc/collections_as_container.md | 39 +++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 doc/collections_as_container.md diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md new file mode 100644 index 000000000..f6e1ec5d5 --- /dev/null +++ b/doc/collections_as_container.md @@ -0,0 +1,39 @@ +# PODIO Collection as a *Container* + +Comparison of the PODIO `Collection`s with a C++ named requirement [*Container*](https://en.cppreference.com/w/cpp/named_req/Container). + +The PODIO `Collection`s are move-only classes with emphasis on the distinction between mutable and non-mutable access to the elements. + +### Types + +| Name | Type | Requirements | Fulfilled by Collection? | Comment | +|------|------|--------------|--------------------------|---------| +| `value_type` | `T` | *Erasable* | ❌ no | defined as non-mutable component type| +| `reference` | `T&` | | ❌ no | not defined | +| `const_reference` | `const T&` | | ❌ no | not defined | +| `iterator` | Iterator whose value type is `T` | *LegacyForwardIterator* convertible to `const_iterator` | ❌ no | not *LegacyForwardIterator*, not convertible to `const_iterator`| +| `const_iterator` | Constant iterator whose value type is `T` | *LegacyForwardIterator* | ❌ no | value type is mutable component type, not *LegacyForwardIterator* +| `difference_type`| Signed integer | Must be the same as `iterator_traits::difference_type` for `iterator` and `const_iterator` | ❌ no | not defined | +| `size_type` | Unsigned integer | Large enough to represent all positive values of difference_type| ❌ no | not defined | + +### Member functions and operators + +| Expression | Return type | Semantics | Fulfilled by Collection? | Comment | +|------------|-------------|-----------|--------------------------|---------| +| `C()` | `C` | Creates an empty container | ✔️ yes | +| `C(a)` | `C` | Creates a copy of `a` | ❌ no | non-copyable +| `C(rv)` | `C` | Moves `rv` | ✔️ yes | +| `a = b` | `C&` | Destroys or copy-assigns all elements of `a` from elements of `b` | ❌ no | non-copyable +| `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 | +| `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` | `const_cast(a).begin()` | ❌ no | not defined | +| `a.cend()` | `const_iterator` | `const_cast(a).end()`| ❌ no | not defined | +| `a == b` | Convertible to `bool` | `std::equal(a.begin(), a.end(), b.begin(), b.end())`| ❌ no | not defined | +| `a != b` | Convertible to `bool` | `!(a == b)` | ❌ no | not defined | +| `a.swap()` | `void` | Exchanges the values of `a` and `b` | ❌ no | not defined | +| `swap(a,b)` | `void` | `a.swap(b)`| ❌ no | not defined | +| `a.size()` | `size_type` | `std::distance(a.begin(), a.end())` | ✔️ yes | +| `a.max_size()` | `size_type` | `b.size()` where b is the largest possible container | ❌ no | not defined | +| `a.empty()` | Convertible to `bool` | `a.begin() == a.end()` | ✔️ yes | From e1161e1975c65c7d77492e967150408d9890b9a6 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Tue, 7 May 2024 14:49:43 +0200 Subject: [PATCH 02/43] Add to collection: `cbegin`, `cend`, `max_size`, `size_type`, `difference_type` required by container --- python/templates/Collection.h.jinja2 | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/python/templates/Collection.h.jinja2 b/python/templates/Collection.h.jinja2 index 6e1e45ada..2f47d1628 100644 --- a/python/templates/Collection.h.jinja2 +++ b/python/templates/Collection.h.jinja2 @@ -51,6 +51,8 @@ public: using value_type = {{ class.bare_type }}; using const_iterator = {{ class.bare_type }}CollectionIterator; using iterator = {{ class.bare_type }}MutableCollectionIterator; + using difference_type = ptrdiff_t; + using size_type = size_t; {{ class.bare_type }}Collection(); {{ class.bare_type }}Collection({{ class.bare_type }}CollectionData&& data, bool isSubsetColl); @@ -86,6 +88,9 @@ public: /// number of elements in the collection std::size_t size() const final; + /// maximal number of elements in the collection + std::size_t max_size() const; + /// Is the collection empty bool empty() const final; @@ -153,12 +158,18 @@ public: const_iterator begin() const { return const_iterator(0, &m_storage.entries); } + const_iterator cbegin() const { + return begin(); + } iterator end() { return iterator(m_storage.entries.size(), &m_storage.entries); } const_iterator end() const { return const_iterator(m_storage.entries.size(), &m_storage.entries); } + const_iterator cend() const { + return end(); + } {% for member in Members %} std::vector<{{ member.full_type }}> {{ member.name }}(const size_t nElem = 0) const; From d1539d4d4563fea3e15193e602d86c52663d9600 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Tue, 7 May 2024 14:50:27 +0200 Subject: [PATCH 03/43] Add basic tests for collection compliance with container named requirement --- tests/unittests/CMakeLists.txt | 6 + tests/unittests/std_interoperability.cpp | 273 +++++++++++++++++++++++ 2 files changed, 279 insertions(+) create mode 100644 tests/unittests/std_interoperability.cpp diff --git a/tests/unittests/CMakeLists.txt b/tests/unittests/CMakeLists.txt index e57da8103..498fe63be 100644 --- a/tests/unittests/CMakeLists.txt +++ b/tests/unittests/CMakeLists.txt @@ -46,6 +46,11 @@ if (ENABLE_SIO) target_link_libraries(unittest_podio PRIVATE podio::podioSioIO) endif() +add_executable(interoperability_test std_interoperability.cpp) +target_link_libraries(interoperability_test PUBLIC TestDataModel PRIVATE Catch2::Catch2WithMain) +# target_compile_definitions(interoperability_test PUBLIC "CATCH_CONFIG_RUNTIME_STATIC_REQUIRE") + target_compile_features(interoperability_test PUBLIC cxx_std_20) + # The unittests can easily be filtered and they are labelled so we can put together a # list of labels that we want to ignore set(filter_tests "") @@ -84,4 +89,5 @@ else() ENVIRONMENT LD_LIBRARY_PATH=${CMAKE_CURRENT_BINARY_DIR}:${PROJECT_BINARY_DIR}/src:${PROJECT_BINARY_DIR}/tests:$:$<$:$>:$ENV{LD_LIBRARY_PATH} ) + catch_discover_tests(interoperability_test) endif() diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp new file mode 100644 index 000000000..0b850b214 --- /dev/null +++ b/tests/unittests/std_interoperability.cpp @@ -0,0 +1,273 @@ +#include "datamodel/ExampleHitCollection.h" +#include +#include +#include + +#if __cplusplus >= 202002L + #include +#endif + +using CollectionType = ExampleHitCollection; + +namespace traits { + +// typename T::value_type +template +struct has_value_type : std::false_type {}; +template +struct has_value_type> : std::true_type {}; +template +inline constexpr bool has_value_type_v = has_value_type::value; + +// typename T::reference +template +struct has_reference : std::false_type {}; +template +struct has_reference> : std::true_type {}; +template +inline constexpr bool has_reference_v = has_reference::value; + +// typename T::const_reference +template +struct has_const_reference : std::false_type {}; +template +struct has_const_reference> : std::true_type {}; +template +inline constexpr bool has_const_reference_v = has_const_reference::value; + +// typename T::iterator +template +struct has_iterator : std::false_type {}; +template +struct has_iterator> : std::true_type {}; +template +inline constexpr bool has_iterator_v = has_iterator::value; + +// typename T::const_iterator +template +struct has_const_iterator : std::false_type {}; +template +struct has_const_iterator> : std::true_type {}; +template +inline constexpr bool has_const_iterator_v = has_const_iterator::value; + +// typename T::difference_type +template +struct has_difference_type : std::false_type {}; +template +struct has_difference_type> : std::true_type {}; +template +inline constexpr bool has_difference_type_v = has_difference_type::value; + +// typename T::size_type +template +struct has_size_type : std::false_type {}; +template +struct has_size_type> : std::true_type {}; +template +inline constexpr bool has_size_type_v = has_size_type::value; + +// T::begin +template +struct has_begin : std::false_type {}; +template +struct has_begin().begin())>> : std::true_type {}; +template +inline constexpr bool has_begin_v = has_begin::value; + +// T::end +template +struct has_end : std::false_type {}; +template +struct has_end().end())>> : std::true_type {}; +template +inline constexpr bool has_end_v = has_end::value; + +// T::cbegin +template +struct has_cbegin : std::false_type {}; +template +struct has_cbegin().cbegin())>> : std::true_type {}; +template +inline constexpr bool has_cbegin_v = has_cbegin::value; + +// T::cend +template +struct has_cend : std::false_type {}; +template +struct has_cend().cend())>> : std::true_type {}; +template +inline constexpr bool has_cend_v = has_cend::value; + +// T::operator==(T) +template +struct has_equality_comparator : std::false_type {}; +template +struct has_equality_comparator() == std::declval())>> : std::true_type {}; +template +inline constexpr bool has_equality_comparator_v = has_equality_comparator::value; + +// T::operator!=(T) +template +struct has_inequality_comparator : std::false_type {}; +template +struct has_inequality_comparator() != std::declval())>> : std::true_type {}; +template +inline constexpr bool has_inequality_comparator_v = has_inequality_comparator::value; + +// T::swap +template +struct has_swap : std::false_type {}; +template +struct has_swap().swap(std::declval()))>> : std::true_type {}; +template +inline constexpr bool has_swap_v = has_swap::value; + +// T::size +template +struct has_size : std::false_type {}; +template +struct has_size().size())>> : std::true_type {}; +template +inline constexpr bool has_size_v = has_size::value; + +// T::max_size +template +struct has_max_size : std::false_type {}; +template +struct has_max_size().max_size())>> : std::true_type {}; +template +inline constexpr bool has_max_size_v = has_max_size::value; + +// T::empty +template +struct has_empty : std::false_type {}; +template +struct has_empty().empty())>> : std::true_type {}; +template +inline constexpr bool has_empty_v = has_empty::value; +} // namespace traits + +TEST_CASE("Collection types", "[collection][container][types][std]") { + + // value_type + STATIC_REQUIRE(traits::has_value_type_v); + + // reference + STATIC_REQUIRE_FALSE(traits::has_reference_v); + // STATIC_REQUIRE(std::is_same_v>); + + // const_reference + STATIC_REQUIRE_FALSE(traits::has_const_reference_v); + // STATIC_REQUIRE(std::is_same_v>>); + + // iterator + STATIC_REQUIRE(traits::has_iterator_v); + STATIC_REQUIRE_FALSE(std::is_convertible_v); + + // const_iterator + STATIC_REQUIRE(traits::has_const_iterator_v); + + // difference_type + STATIC_REQUIRE(traits::has_difference_type_v); + + STATIC_REQUIRE(std::is_signed_v); + STATIC_REQUIRE(std::is_integral_v); + // STATIC_REQUIRE( + // std::is_same_v::difference_type>); + // STATIC_REQUIRE(std::is_same_v::difference_type>); + + // size_type + STATIC_REQUIRE(traits::has_size_type_v); + STATIC_REQUIRE(std::is_unsigned_v); + STATIC_REQUIRE(std::is_integral_v); + STATIC_REQUIRE(std::numeric_limits::max() >= + std::numeric_limits::max()); +} + +TEST_CASE("Collection members", "[collection][container][types][std]") { + // C() + STATIC_REQUIRE(std::is_default_constructible_v); + REQUIRE(CollectionType().empty() == true); + + // C(a) + STATIC_REQUIRE_FALSE(std::is_copy_constructible_v); + + // C(rv) + STATIC_REQUIRE(std::is_move_constructible_v); + + // a = b + STATIC_REQUIRE_FALSE(std::is_copy_assignable_v); + + // a = rv + STATIC_REQUIRE(std::is_move_assignable_v); + + // a.~C() + STATIC_REQUIRE(std::is_destructible_v); + + // a.begin() + STATIC_REQUIRE(traits::has_begin_v); + STATIC_REQUIRE(std::is_same_v().begin()), CollectionType::iterator>); + STATIC_REQUIRE( + std::is_same_v().begin()), CollectionType::const_iterator>); + + // a.end() + STATIC_REQUIRE(traits::has_end_v); + STATIC_REQUIRE(std::is_same_v().end()), CollectionType::iterator>); + STATIC_REQUIRE(std::is_same_v().end()), CollectionType::const_iterator>); + + // a.cbegin() + STATIC_REQUIRE(traits::has_cbegin_v); + STATIC_REQUIRE(std::is_same_v().cbegin()), CollectionType::const_iterator>); + + // a.cend() + STATIC_REQUIRE(traits::has_cend_v); + STATIC_REQUIRE(std::is_same_v().cend()), CollectionType::const_iterator>); + + // a == b + STATIC_REQUIRE_FALSE(traits::has_equality_comparator_v); + // STATIC_REQUIRE(std::is_convertible_v()==std::declval()), + // bool>); + + // a != b + STATIC_REQUIRE_FALSE(traits::has_inequality_comparator_v); + // STATIC_REQUIRE(std::is_convertible_v()!=std::declval()), + // bool>); + + // a.swap(b) + STATIC_REQUIRE_FALSE(traits::has_swap_v); + // STATIC_REQUIRE( + // std::is_same_v().swap(std::declval())), void>); + + // swap(a,b) + STATIC_REQUIRE(std::is_swappable_v); + + // a.size() + STATIC_REQUIRE(traits::has_size_v); + STATIC_REQUIRE(std::is_same_v().size()), CollectionType::size_type>); + + // a.max_size()) + STATIC_REQUIRE(traits::has_max_size_v); + STATIC_REQUIRE(std::is_same_v().max_size()), CollectionType::size_type>); + + // a.empty() + STATIC_REQUIRE(traits::has_empty_v); + STATIC_REQUIRE(std::is_convertible_v().empty()), bool>); +} + +TEST_CASE("Collection iterators", "[collection][container][interator][std]") { + +#if (__cplusplus >= 202002L) + STATIC_REQUIRE_FALSE(std::forward_iterator); + STATIC_REQUIRE_FALSE(std::forward_iterator); +#endif + FAIL(); +} + +TEST_CASE("Collection and std::algorithms", "[collection][container][algorithm][std]") { + auto a = CollectionType(); + FAIL(); +} From 8e34c3ddfdbd38348b7988311e9496b849c3ae5f Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Tue, 7 May 2024 20:24:36 +0200 Subject: [PATCH 04/43] add missing `max_size` implementation, add collection methods and aliases to `UserDataCollection` --- include/podio/CollectionBase.h | 3 +++ include/podio/UserDataCollection.h | 17 +++++++++++++++++ python/templates/Collection.cc.jinja2 | 4 ++++ python/templates/Collection.h.jinja2 | 2 +- 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/include/podio/CollectionBase.h b/include/podio/CollectionBase.h index ae6fa1d4a..2c0224557 100644 --- a/include/podio/CollectionBase.h +++ b/include/podio/CollectionBase.h @@ -55,6 +55,9 @@ class CollectionBase { /// number of elements in the collection virtual size_t size() const = 0; + /// maximal number of elements in the collection + virtual std::size_t max_size() const = 0; + /// Is the collection empty virtual bool empty() const = 0; diff --git a/include/podio/UserDataCollection.h b/include/podio/UserDataCollection.h index 0b2c9a092..ce1f3bba3 100644 --- a/include/podio/UserDataCollection.h +++ b/include/podio/UserDataCollection.h @@ -77,6 +77,12 @@ class UserDataCollection : public CollectionBase { VectorMembersInfo m_vecmem_info{}; public: + using value_type = typename decltype(_vec)::value_type; + using const_iterator = typename decltype(_vec)::const_iterator; + using iterator = typename decltype(_vec)::iterator; + using difference_type = typename decltype(_vec)::difference_type; + using size_type = typename decltype(_vec)::size_type; + UserDataCollection() = default; /// Constructor from an existing vector (which will be moved from!) UserDataCollection(std::vector&& vec) : _vec(std::move(vec)) { @@ -133,6 +139,11 @@ class UserDataCollection : public CollectionBase { return _vec.size(); } + /// maximal number of elements in the collection + size_t max_size() const override { + return _vec.max_size(); + } + /// Is the collection empty bool empty() const override { return _vec.empty(); @@ -206,6 +217,12 @@ class UserDataCollection : public CollectionBase { typename std::vector::const_iterator end() const { return _vec.end(); } + typename std::vector::const_iterator cbegin() const { + return _vec.cbegin(); + } + typename std::vector::const_iterator cend() const { + return _vec.cend(); + } typename std::vector::reference operator[](size_t idx) { return _vec[idx]; diff --git a/python/templates/Collection.cc.jinja2 b/python/templates/Collection.cc.jinja2 index c212784d3..004219e31 100644 --- a/python/templates/Collection.cc.jinja2 +++ b/python/templates/Collection.cc.jinja2 @@ -59,6 +59,10 @@ std::size_t {{ collection_type }}::size() const { return m_storage.entries.size(); } +std::size_t {{ collection_type }}::max_size() const { + return m_storage.entries.max_size(); +} + bool {{ collection_type }}::empty() const { return m_storage.entries.empty(); } diff --git a/python/templates/Collection.h.jinja2 b/python/templates/Collection.h.jinja2 index 2f47d1628..b57864cc7 100644 --- a/python/templates/Collection.h.jinja2 +++ b/python/templates/Collection.h.jinja2 @@ -89,7 +89,7 @@ public: std::size_t size() const final; /// maximal number of elements in the collection - std::size_t max_size() const; + std::size_t max_size() const final; /// Is the collection empty bool empty() const final; From 3c74e1fe5720605a5332b83ce532de4f49c1648b Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Tue, 7 May 2024 20:33:48 +0200 Subject: [PATCH 05/43] update documentation with container like methods and aliases --- doc/collections_as_container.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index f6e1ec5d5..83cee7138 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -13,8 +13,8 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | `const_reference` | `const T&` | | ❌ no | not defined | | `iterator` | Iterator whose value type is `T` | *LegacyForwardIterator* convertible to `const_iterator` | ❌ no | not *LegacyForwardIterator*, not convertible to `const_iterator`| | `const_iterator` | Constant iterator whose value type is `T` | *LegacyForwardIterator* | ❌ no | value type is mutable component type, not *LegacyForwardIterator* -| `difference_type`| Signed integer | Must be the same as `iterator_traits::difference_type` for `iterator` and `const_iterator` | ❌ no | not defined | -| `size_type` | Unsigned integer | Large enough to represent all positive values of difference_type| ❌ no | not defined | +| `difference_type`| Signed integer | Must be the same as `iterator_traits::difference_type` for `iterator` and `const_iterator` | ❌ no | `iterator_traits::difference_type` is not valid | +| `size_type` | Unsigned integer | Large enough to represent all positive values of difference_type| ✔️ yes | | ### Member functions and operators @@ -28,12 +28,12 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | `a.~C()` | `void` | Destroys all elements of `a` and frees all memory| ✔️ yes | | `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` | `const_cast(a).begin()` | ❌ no | not defined | -| `a.cend()` | `const_iterator` | `const_cast(a).end()`| ❌ no | not defined | +| `a.cbegin()` | `const_iterator` | `const_cast(a).begin()` | ✔️ yes | +| `a.cend()` | `const_iterator` | `const_cast(a).end()`| ✔️ yes | | `a == b` | Convertible to `bool` | `std::equal(a.begin(), a.end(), b.begin(), b.end())`| ❌ no | not defined | | `a != b` | Convertible to `bool` | `!(a == b)` | ❌ no | not defined | -| `a.swap()` | `void` | Exchanges the values of `a` and `b` | ❌ no | not defined | +| `a.swap(b)` | `void` | Exchanges the values of `a` and `b` | ❌ no | not defined | | `swap(a,b)` | `void` | `a.swap(b)`| ❌ no | not defined | | `a.size()` | `size_type` | `std::distance(a.begin(), a.end())` | ✔️ yes | -| `a.max_size()` | `size_type` | `b.size()` where b is the largest possible container | ❌ no | not defined | +| `a.max_size()` | `size_type` | `b.size()` where b is the largest possible container | ✔️ yes | not defined | | `a.empty()` | Convertible to `bool` | `a.begin() == a.end()` | ✔️ yes | From d7b1acc2b88421532b55bd02a80bfdc1584ca2fc Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Tue, 7 May 2024 23:52:18 +0200 Subject: [PATCH 06/43] Add placeholders for iterator requirements and interaction with std algorithms --- doc/collections_as_container.md | 117 ++++++++++++++++++++--- tests/unittests/std_interoperability.cpp | 98 ++++++++++++++++++- 2 files changed, 201 insertions(+), 14 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 83cee7138..09fcd474b 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -2,19 +2,19 @@ Comparison of the PODIO `Collection`s with a C++ named requirement [*Container*](https://en.cppreference.com/w/cpp/named_req/Container). -The PODIO `Collection`s are move-only classes with emphasis on the distinction between mutable and non-mutable access to the elements. +The PODIO `Collection`s are move-only classes with emphasis on the distinction between mutable and immutable access to the elements. ### Types | Name | Type | Requirements | Fulfilled by Collection? | Comment | |------|------|--------------|--------------------------|---------| -| `value_type` | `T` | *Erasable* | ❌ no | defined as non-mutable component type| +| `value_type` | `T` | *Erasable* | ❌ no | defined as immutable component type| | `reference` | `T&` | | ❌ no | not defined | | `const_reference` | `const T&` | | ❌ no | not defined | | `iterator` | Iterator whose value type is `T` | *LegacyForwardIterator* convertible to `const_iterator` | ❌ no | not *LegacyForwardIterator*, not convertible to `const_iterator`| | `const_iterator` | Constant iterator whose value type is `T` | *LegacyForwardIterator* | ❌ no | value type is mutable component type, not *LegacyForwardIterator* -| `difference_type`| Signed integer | Must be the same as `iterator_traits::difference_type` for `iterator` and `const_iterator` | ❌ no | `iterator_traits::difference_type` is not valid | -| `size_type` | Unsigned integer | Large enough to represent all positive values of difference_type| ✔️ yes | | +| `difference_type`| Signed integer | Must be the same as `iterator_traits::difference_type` for `iterator` and `const_iterator` | ❌ no | `iterator_traits::difference_type` doesn't | +| `size_type` | Unsigned integer | Large enough to represent all positive values of `difference_type` | ✔️ yes | | ### Member functions and operators @@ -28,12 +28,107 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | `a.~C()` | `void` | Destroys all elements of `a` and frees all memory| ✔️ yes | | `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` | `const_cast(a).begin()` | ✔️ yes | -| `a.cend()` | `const_iterator` | `const_cast(a).end()`| ✔️ yes | -| `a == b` | Convertible to `bool` | `std::equal(a.begin(), a.end(), b.begin(), b.end())`| ❌ no | not defined | -| `a != b` | Convertible to `bool` | `!(a == b)` | ❌ no | not defined | +| `a.cbegin()` | `const_iterator` | Same as `const_cast(a).begin()` | ✔️ yes | +| `a.cend()` | `const_iterator` | Same as `const_cast(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` | `a.swap(b)`| ❌ no | not defined | -| `a.size()` | `size_type` | `std::distance(a.begin(), a.end())` | ✔️ yes | +| `swap(a,b)` | `void` | Same as `a.swap(b)`| ❌ no | 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 | not defined | -| `a.empty()` | Convertible to `bool` | `a.begin() == a.end()` | ✔️ yes | +| `a.empty()` | Convertible to `bool` | Same as `a.begin() == a.end()` | ✔️ yes | + +## Collection iterator as a *Iterator* + +### LegacyIterator + +| Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment | +|-------------|-------------------------------------------|---------| +| *CopyConstructible* | ❌ no / ❌ no | | +| *CopyAssignable* | ❌ no / ❌ no | | +| *Destructible* | ✔️ yes / ✔️ yes | | +| *Swappable* | ✔️ yes / ✔️ yes | | +| `std::iterator_traits::value_type` (Until C++20 ) | ❌ no / ❌ no | | +| `std::iterator_traits::difference_type` | ❌ no / ❌ no | | +| `std::iterator_traits::reference` | ❌ no / ❌ no | | +| `std::iterator_traits::pointer` | ❌ no / ❌ no | | +| `std::iterator_traits::iterator_category` | ❌ no / ❌ no | | + +| Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment | +|------------|-------------|-----------|-------------------------------------------|---------| +| `*r` | Unspecified | Dereferenceable | ❌ no / ❌ no | | +| `++r` | `It&` | Incrementable | ❌ no / ❌ no | | + +### LegacyInputIterator + +| Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment | +|-------------|-------------------------------------------|---------| +| *LegacyIterator* | ❌ no | ❌ no | | +| *EqualityComparable* | ✔️ yes | ✔️ yes | | + +| Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment | +|------------|-------------|-----------|-------------------------------------------|---------| +| `i != j` | Convertible to `bool` | Same as `!(i==j)` | ❌ no / ❌ no | | +| `*i` | Reference, convertible to `value_type` | | ❌ no / ❌ no | | +| `i->m` | | Same as `(*i).m` | ❌ no / ❌ no | | +| `++r` | `It&` | | ❌ no / ❌ no | | +| `(void)r++` | | Same as `(void)++r` | ❌ no / ❌ no | | +| `*r++` | Convertible to `value_type` | Same as `value_type x = *r; ++r; return x;` | ❌ no / ❌ no | | + + +### LegacyForwardIterator + +| Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment | +|-------------|-------------------------------------------|---------| +| *LegacyInputIterator* | ❌ no / ❌ no | | +| *DefaultConstructible* | ❌ no / ❌ no | | + +| Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment | +|------------|-------------|-----------|-------------------------------------------|---------| +| `i++` | `It` | Same as `It ip = i; ++i; return ip;` | ❌ no / ❌ no | | +| `*i++` | `reference` | | ❌ no / ❌ no | | + +## Collection and standard algorithms + +### Non-modifying sequence operations + +| Algorithm | Compatible with Collection? | Comment | +| ----------|-----------------------------|---------| +| `all_of` | | | +| `any_of` | | | +| `none_of` | | | +| `for_each` | | | +| `for_each_n` | | | +| `count` | | | +| `count_if` | | | +| `mismatch` | | | +| `find` | | | +| `find_if` | | | +| `find_if_not` | | | +| `find_end` | | | +| `find_first_of` | | | +| `adjacent_find` | | | +| `search` | | | +| `search_n` | | | + +### Modifying sequence operations + +### Partitioning operations + +### Sorting operations + +### Binary search operations (on sorted ranges) + +### Other operations on sorted ranges + +### Set operations (on sorted ranges) + +### Heap operations + +### Minimum/maximum operations + +### Comparison operations + +### Permutation operations + +## Standard ranges diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index 0b850b214..15c165f2a 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -1,5 +1,6 @@ #include "datamodel/ExampleHitCollection.h" #include +#include #include #include @@ -67,6 +68,22 @@ struct has_size_type> : std::true_type {}; template inline constexpr bool has_size_type_v = has_size_type::value; +// typename T::pointer +template +struct has_pointer : std::false_type {}; +template +struct has_pointer> : std::true_type {}; +template +inline constexpr bool has_pointer_v = has_pointer::value; + +// typename T::iterator_category +template +struct has_iterator_category : std::false_type {}; +template +struct has_iterator_category> : std::true_type {}; +template +inline constexpr bool has_iterator_category_v = has_iterator_category::value; + // T::begin template struct has_begin : std::false_type {}; @@ -174,9 +191,11 @@ TEST_CASE("Collection types", "[collection][container][types][std]") { STATIC_REQUIRE(std::is_signed_v); STATIC_REQUIRE(std::is_integral_v); + STATIC_REQUIRE_FALSE(traits::has_difference_type_v>); // STATIC_REQUIRE( // std::is_same_v::difference_type>); + STATIC_REQUIRE_FALSE(traits::has_difference_type_v>); // STATIC_REQUIRE(std::is_same_v::difference_type>); @@ -259,11 +278,84 @@ TEST_CASE("Collection members", "[collection][container][types][std]") { } TEST_CASE("Collection iterators", "[collection][container][interator][std]") { - + using iterator = CollectionType::iterator; + using const_iterator = CollectionType::const_iterator; #if (__cplusplus >= 202002L) - STATIC_REQUIRE_FALSE(std::forward_iterator); - STATIC_REQUIRE_FALSE(std::forward_iterator); + STATIC_REQUIRE_FALSE(std::forward_iterator); + STATIC_REQUIRE_FALSE(std::forward_iterator); +#endif + + SECTION("LegacyForwardIterator") { + + SECTION("LegacyInputIterator") { + SECTION("LegacyIterator") { + // CopyConstructible + STATIC_REQUIRE_FALSE(std::is_move_constructible_v && std::is_copy_constructible_v); + + // CopyAssignable + STATIC_REQUIRE_FALSE(std::is_move_assignable_v && std::is_copy_assignable_v); + + // Destructible + STATIC_REQUIRE(std::is_destructible_v); + + // Swappable + STATIC_REQUIRE_FALSE(std::is_swappable_v); + +#if (__cplusplus < 202002L) + // std::iterator_traits::value_type + STATIC_REQUIRE_FALSE(traits::has_value_type_v>); #endif + // std::iterator_traits::difference_type + STATIC_REQUIRE_FALSE(traits::has_difference_type_v>); + // std::iterator_traits::reference + STATIC_REQUIRE_FALSE(traits::has_reference_v>); + // std::iterator_traits::pointer + STATIC_REQUIRE_FALSE(traits::has_pointer_v>); + // std::iterator_traits::iterator_category + STATIC_REQUIRE_FALSE(traits::has_iterator_category_v>); + + // *r + FAIL(); + + // ++r + FAIL(); + } + + // EqualityComparable + STATIC_REQUIRE(traits::has_equality_comparator_v); + + // i != j + STATIC_REQUIRE(traits::has_inequality_comparator_v); + + // *i + FAIL(); + + // i->m + FAIL(); + + // ++r + FAIL(); + + // (void)r++ + FAIL(); + + //*r++ + STATIC_REQUIRE_FALSE(traits::has_value_type_v>); + // STATIC_REQUIRE(std::is_convertible_v < decltype(*std::declval()++), + // std::iterator_traits::value_type >>); + } + + // DefaultConstructible + STATIC_REQUIRE_FALSE(std::is_default_constructible_v); + } + + // multipass guarantee + FAIL(); + + // i++ + FAIL(); + + // *i++ FAIL(); } From e117c94a3589a380e0431e41f7a1ed4173b2f4a4 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Wed, 8 May 2024 00:02:23 +0200 Subject: [PATCH 07/43] fix fail message --- tests/unittests/std_interoperability.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index 15c165f2a..a90a57375 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -315,10 +315,10 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { STATIC_REQUIRE_FALSE(traits::has_iterator_category_v>); // *r - FAIL(); + FAIL("Not yet implemented"); // ++r - FAIL(); + FAIL("Not yet implemented"); } // EqualityComparable @@ -328,16 +328,16 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { STATIC_REQUIRE(traits::has_inequality_comparator_v); // *i - FAIL(); + FAIL("Not yet implemented"); // i->m - FAIL(); + FAIL("Not yet implemented"); // ++r - FAIL(); + FAIL("Not yet implemented"); // (void)r++ - FAIL(); + FAIL("Not yet implemented"); //*r++ STATIC_REQUIRE_FALSE(traits::has_value_type_v>); @@ -350,16 +350,16 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { } // multipass guarantee - FAIL(); + FAIL("Not yet implemented"); // i++ - FAIL(); + FAIL("Not yet implemented"); // *i++ - FAIL(); + FAIL("Not yet implemented"); } TEST_CASE("Collection and std::algorithms", "[collection][container][algorithm][std]") { auto a = CollectionType(); - FAIL(); + FAIL("Not yet implemented"); } From b1d59a8d45fe472403d1fd5cd65681d7ee818a81 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Wed, 8 May 2024 14:16:03 +0200 Subject: [PATCH 08/43] check Erasable, fix consistency, add links to reference --- doc/collections_as_container.md | 62 ++++++++++++++---------- tests/unittests/std_interoperability.cpp | 31 +++++++++++- 2 files changed, 66 insertions(+), 27 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 09fcd474b..564affc58 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -4,50 +4,58 @@ Comparison of the PODIO `Collection`s with a C++ named requirement [*Container*] The PODIO `Collection`s are move-only classes with emphasis on the distinction between mutable and immutable access to the elements. -### Types +### Container Types | Name | Type | Requirements | Fulfilled by Collection? | Comment | |------|------|--------------|--------------------------|---------| -| `value_type` | `T` | *Erasable* | ❌ no | defined as immutable component type| -| `reference` | `T&` | | ❌ no | not defined | -| `const_reference` | `const T&` | | ❌ no | not defined | -| `iterator` | Iterator whose value type is `T` | *LegacyForwardIterator* convertible to `const_iterator` | ❌ no | not *LegacyForwardIterator*, not convertible to `const_iterator`| -| `const_iterator` | Constant iterator whose value type is `T` | *LegacyForwardIterator* | ❌ no | value type is mutable component type, not *LegacyForwardIterator* -| `difference_type`| Signed integer | Must be the same as `iterator_traits::difference_type` for `iterator` and `const_iterator` | ❌ no | `iterator_traits::difference_type` doesn't | +| `value_type` | `T` | *[Erasable](https://en.cppreference.com/w/cpp/named_req/Erasable)* | ✔️ yes | Defined as immutable component type | +| `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 | `iterator` doesn't have `value_type`, 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 | `const_iterator` doesn't have `value_type`, 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` doesn't exist | | `size_type` | Unsigned integer | Large enough to represent all positive values of `difference_type` | ✔️ yes | | -### Member functions and operators +### Container member functions and operators | Expression | Return type | Semantics | Fulfilled by Collection? | Comment | |------------|-------------|-----------|--------------------------|---------| | `C()` | `C` | Creates an empty container | ✔️ yes | -| `C(a)` | `C` | Creates a copy of `a` | ❌ no | non-copyable +| `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 | non-copyable +| `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 | | `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(a).begin()` | ✔️ yes | | `a.cend()` | `const_iterator` | Same as `const_cast(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 | not defined | +| `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 | 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 | not defined | +| `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 iterator as a *Iterator* +## Collection iterators as a *Iterator* + +### Iterator summary + +| Named requirement | Collection::`iterator`| Collection::`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)) | +| [LegacyForwardIterator](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) | ❌ no ([see below](#legacyforwarditerator)) | ❌ no ([see below](#legacyforwarditerator)) | ### LegacyIterator | Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment | |-------------|-------------------------------------------|---------| -| *CopyConstructible* | ❌ no / ❌ no | | -| *CopyAssignable* | ❌ no / ❌ no | | -| *Destructible* | ✔️ yes / ✔️ yes | | -| *Swappable* | ✔️ yes / ✔️ yes | | +| [*CopyConstructible*](https://en.cppreference.com/w/cpp/named_req/CopyConstructible) | ❌ no / ❌ no | | +| [*CopyAssignable*](https://en.cppreference.com/w/cpp/named_req/CopyAssignable) | ❌ no / ❌ no | | +| [*Destructible*](https://en.cppreference.com/w/cpp/named_req/Destructible) | ✔️ yes / ✔️ yes | | +| [*Swappable*](https://en.cppreference.com/w/cpp/named_req/Swappable) | ✔️ yes / ✔️ yes | | | `std::iterator_traits::value_type` (Until C++20 ) | ❌ no / ❌ no | | | `std::iterator_traits::difference_type` | ❌ no / ❌ no | | | `std::iterator_traits::reference` | ❌ no / ❌ no | | @@ -63,8 +71,8 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment | |-------------|-------------------------------------------|---------| -| *LegacyIterator* | ❌ no | ❌ no | | -| *EqualityComparable* | ✔️ yes | ✔️ yes | | +| [*LegacyIterator*](https://en.cppreference.com/w/cpp/named_req/Iterator) | ❌ no / ❌ no | | +| [*EqualityComparable*](https://en.cppreference.com/w/cpp/named_req/EqualityComparable) | ✔️ yes / ✔️ yes | | | Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment | |------------|-------------|-----------|-------------------------------------------|---------| @@ -75,19 +83,23 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | `(void)r++` | | Same as `(void)++r` | ❌ no / ❌ no | | | `*r++` | Convertible to `value_type` | Same as `value_type x = *r; ++r; return x;` | ❌ no / ❌ no | | - ### LegacyForwardIterator | Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment | |-------------|-------------------------------------------|---------| -| *LegacyInputIterator* | ❌ no / ❌ no | | -| *DefaultConstructible* | ❌ no / ❌ no | | +| [*LegacyInputIterator*](https://en.cppreference.com/w/cpp/named_req/InputIterator) | ❌ no / ❌ no | | +| [*DefaultConstructible*](https://en.cppreference.com/w/cpp/named_req/DefaultConstructible) | ❌ no / ❌ no | | +| If immutable `reference` same as `value_type&` or `value_type&&`, otherwise same as `const value_type&` or `const value_type&&` | ❌ no / ❌ no | | +| Multipass guarantee | ❌ no / ❌ no | | +| Singular iterators | ❌ no / ❌ no | | | Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment | |------------|-------------|-----------|-------------------------------------------|---------| | `i++` | `It` | Same as `It ip = i; ++i; return ip;` | ❌ no / ❌ no | | | `*i++` | `reference` | | ❌ no / ❌ no | | +## Collection iterators and standard iterator adapters + ## Collection and standard algorithms ### Non-modifying sequence operations diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index a90a57375..9e99f7c69 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #if __cplusplus >= 202002L @@ -76,6 +77,26 @@ struct has_pointer> : std::true_type {}; template inline constexpr bool has_pointer_v = has_pointer::value; +// typename T::allocator_type +template +struct has_allocator_type : std::false_type {}; +template +struct has_allocator_type> : std::true_type {}; +template +inline constexpr bool has_allocator_type_v = has_allocator_type::value; + +// is_erasable_allocator_unaware +template +struct is_erasable_allocator_unaware : std::false_type {}; +template +struct is_erasable_allocator_unaware< + T, + std::void_t>::destroy( + std::declval>>(), + std::declval>()))>> : std::true_type {}; +template +inline constexpr bool is_erasable_allocator_unaware_v = is_erasable_allocator_unaware::value; + // typename T::iterator_category template struct has_iterator_category : std::false_type {}; @@ -169,6 +190,12 @@ TEST_CASE("Collection types", "[collection][container][types][std]") { // value_type STATIC_REQUIRE(traits::has_value_type_v); + // Erasable -allocator aware - mutually exclusive with Erasable -allocator not aware + STATIC_REQUIRE_FALSE(traits::has_allocator_type_v); + // add check for `std::allocator_traits::destroy(m, p);` expression here + // STATIC_REQUIRE(...) + // Erasable -allocator not aware - mutually exclusive // Erasable -allocator aware + STATIC_REQUIRE(traits::is_erasable_allocator_unaware_v); // reference STATIC_REQUIRE_FALSE(traits::has_reference_v); @@ -207,7 +234,7 @@ TEST_CASE("Collection types", "[collection][container][types][std]") { std::numeric_limits::max()); } -TEST_CASE("Collection members", "[collection][container][types][std]") { +TEST_CASE("Collection members", "[collection][container][members][std]") { // C() STATIC_REQUIRE(std::is_default_constructible_v); REQUIRE(CollectionType().empty() == true); @@ -296,7 +323,7 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { STATIC_REQUIRE_FALSE(std::is_move_assignable_v && std::is_copy_assignable_v); // Destructible - STATIC_REQUIRE(std::is_destructible_v); + STATIC_REQUIRE(std::is_nothrow_destructible_v); // Swappable STATIC_REQUIRE_FALSE(std::is_swappable_v); From 874094589fd4cf71424b90ca256b0ccdbaa0f9d8 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Thu, 9 May 2024 00:44:02 +0200 Subject: [PATCH 09/43] fix consistency, add macro to indicate checks that may need updating docs, add more checks --- doc/collections_as_container.md | 50 ++++---- tests/unittests/std_interoperability.cpp | 147 +++++++++++++++++------ 2 files changed, 135 insertions(+), 62 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 564affc58..5854f006e 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -11,9 +11,9 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | `value_type` | `T` | *[Erasable](https://en.cppreference.com/w/cpp/named_req/Erasable)* | ✔️ yes | Defined as immutable component type | | `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 | `iterator` doesn't have `value_type`, 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 | `const_iterator` doesn't have `value_type`, 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` doesn't exist | +| `iterator` | Iterator whose `value_type` is `T` | [*LegacyForwardIterator*](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) convertible to `const_iterator` | ❌ no | `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 | `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 | | ### Container member functions and operators @@ -33,7 +33,7 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | `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 | Not defined | +| `swap(a,b)` | `void` | Same as `a.swap(b)` | ✔️ yes | `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.empty()` | Convertible to `bool` | Same as `a.begin() == a.end()` | ✔️ yes | @@ -52,51 +52,51 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment | |-------------|-------------------------------------------|---------| -| [*CopyConstructible*](https://en.cppreference.com/w/cpp/named_req/CopyConstructible) | ❌ no / ❌ no | | -| [*CopyAssignable*](https://en.cppreference.com/w/cpp/named_req/CopyAssignable) | ❌ no / ❌ no | | +| [*CopyConstructible*](https://en.cppreference.com/w/cpp/named_req/CopyConstructible) | ❌ no / ❌ no | Move constructor not defined, copy constructor not defined | +| [*CopyAssignable*](https://en.cppreference.com/w/cpp/named_req/CopyAssignable) | ❌ no / ❌ no | Move assignment not defined, copy assignment not defined | | [*Destructible*](https://en.cppreference.com/w/cpp/named_req/Destructible) | ✔️ yes / ✔️ yes | | | [*Swappable*](https://en.cppreference.com/w/cpp/named_req/Swappable) | ✔️ yes / ✔️ yes | | -| `std::iterator_traits::value_type` (Until C++20 ) | ❌ no / ❌ no | | -| `std::iterator_traits::difference_type` | ❌ no / ❌ no | | -| `std::iterator_traits::reference` | ❌ no / ❌ no | | -| `std::iterator_traits::pointer` | ❌ no / ❌ no | | -| `std::iterator_traits::iterator_category` | ❌ 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 | | Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment | |------------|-------------|-----------|-------------------------------------------|---------| -| `*r` | Unspecified | Dereferenceable | ❌ no / ❌ no | | -| `++r` | `It&` | Incrementable | ❌ no / ❌ no | | +| `*r` | Unspecified | Dereferenceable | ✔️ yes / ✔️ yes | | +| `++r` | `It&` | Incrementable | ✔️ yes / ✔️ yes | | ### LegacyInputIterator | Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment | |-------------|-------------------------------------------|---------| -| [*LegacyIterator*](https://en.cppreference.com/w/cpp/named_req/Iterator) | ❌ no / ❌ no | | +| [*LegacyIterator*](https://en.cppreference.com/w/cpp/named_req/Iterator) | ❌ no / ❌ no | [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` | Convertible to `bool` | Same as `!(i==j)` | ❌ no / ❌ no | | -| `*i` | Reference, convertible to `value_type` | | ❌ no / ❌ no | | -| `i->m` | | Same as `(*i).m` | ❌ no / ❌ no | | -| `++r` | `It&` | | ❌ no / ❌ no | | -| `(void)r++` | | Same as `(void)++r` | ❌ no / ❌ no | | -| `*r++` | Convertible to `value_type` | Same as `value_type x = *r; ++r; return x;` | ❌ no / ❌ no | | +| `i != j` | Convertible to `bool` | Same as `!(i==j)` | ✔️ yes / ✔️ yes | | +| `*i` | `reference`, convertible to `value_type` | | ❌ no / ❌ no | `reference` and `value_type` not defined | +| `i->m` | | Same as `(*i).m` | ✔️ yes / ✔️ yes | | +| `++r` | `It&` | | ✔️ yes / ✔️ yes | | +| `(void)r++` | | Same as `(void)++r` | ❌ no / ❌ no | Postfix not defined | +| `*r++` | Convertible to `value_type` | Same as `value_type x = *r; ++r; return x;` | ❌ no / ❌ no | Postfix not defined, `value_type` not defined | ### LegacyForwardIterator | Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment | |-------------|-------------------------------------------|---------| -| [*LegacyInputIterator*](https://en.cppreference.com/w/cpp/named_req/InputIterator) | ❌ no / ❌ no | | -| [*DefaultConstructible*](https://en.cppreference.com/w/cpp/named_req/DefaultConstructible) | ❌ no / ❌ no | | -| If immutable `reference` same as `value_type&` or `value_type&&`, otherwise same as `const value_type&` or `const value_type&&` | ❌ no / ❌ no | | +| [*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 | Default initialization no 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` not defined, `value_type` not defined | | Multipass guarantee | ❌ no / ❌ no | | | Singular iterators | ❌ no / ❌ no | | | Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment | |------------|-------------|-----------|-------------------------------------------|---------| -| `i++` | `It` | Same as `It ip = i; ++i; return ip;` | ❌ no / ❌ no | | -| `*i++` | `reference` | | ❌ no / ❌ no | | +| `i++` | `It` | Same as `It ip = i; ++i; return ip;` | ❌ no / ❌ no | Postfix not defined | +| `*i++` | `reference` | | ❌ no / ❌ no | Postfix not defined, `reference` not defined| ## Collection iterators and standard iterator adapters diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index 9e99f7c69..17818b72e 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -9,6 +9,10 @@ #include #endif +// Some of the tests check . If the check annotated with 'REQUIREMENT_NOT_MET' fails, update documentation +// 'doc/collection_as_container.md' +#define REQUIREMENT_NOT_MET(...) STATIC_REQUIRE_FALSE(__VA_ARGS__) + using CollectionType = ExampleHitCollection; namespace traits { @@ -184,6 +188,22 @@ template struct has_empty().empty())>> : std::true_type {}; template inline constexpr bool has_empty_v = has_empty::value; + +// T::operator++() (prefix) +template +struct has_prefix : std::false_type {}; +template +struct has_prefix())>> : std::true_type {}; +template +inline constexpr bool has_prefix_v = has_prefix::value; + +// T::operator++(int) (postfix) +template +struct has_postfix : std::false_type {}; +template +struct has_postfix()++)>> : std::true_type {}; +template +inline constexpr bool has_postfix_v = has_postfix::value; } // namespace traits TEST_CASE("Collection types", "[collection][container][types][std]") { @@ -191,38 +211,40 @@ TEST_CASE("Collection types", "[collection][container][types][std]") { // value_type STATIC_REQUIRE(traits::has_value_type_v); // Erasable -allocator aware - mutually exclusive with Erasable -allocator not aware - STATIC_REQUIRE_FALSE(traits::has_allocator_type_v); + REQUIREMENT_NOT_MET(traits::has_allocator_type_v); // add check for `std::allocator_traits::destroy(m, p);` expression here // STATIC_REQUIRE(...) // Erasable -allocator not aware - mutually exclusive // Erasable -allocator aware STATIC_REQUIRE(traits::is_erasable_allocator_unaware_v); // reference - STATIC_REQUIRE_FALSE(traits::has_reference_v); + REQUIREMENT_NOT_MET(traits::has_reference_v); // STATIC_REQUIRE(std::is_same_v>); // const_reference - STATIC_REQUIRE_FALSE(traits::has_const_reference_v); + REQUIREMENT_NOT_MET(traits::has_const_reference_v); // The check will fail once the support is added. + // In that case replace it with STATIC_REQUIRE, + // uncomment checks immediately below, and update + // documentation // STATIC_REQUIRE(std::is_same_v>>); // iterator STATIC_REQUIRE(traits::has_iterator_v); - STATIC_REQUIRE_FALSE(std::is_convertible_v); + REQUIREMENT_NOT_MET(std::is_convertible_v); // const_iterator STATIC_REQUIRE(traits::has_const_iterator_v); // difference_type STATIC_REQUIRE(traits::has_difference_type_v); - STATIC_REQUIRE(std::is_signed_v); STATIC_REQUIRE(std::is_integral_v); - STATIC_REQUIRE_FALSE(traits::has_difference_type_v>); + REQUIREMENT_NOT_MET(traits::has_difference_type_v>); // STATIC_REQUIRE( // std::is_same_v::difference_type>); - STATIC_REQUIRE_FALSE(traits::has_difference_type_v>); + REQUIREMENT_NOT_MET(traits::has_difference_type_v>); // STATIC_REQUIRE(std::is_same_v::difference_type>); @@ -240,13 +262,13 @@ TEST_CASE("Collection members", "[collection][container][members][std]") { REQUIRE(CollectionType().empty() == true); // C(a) - STATIC_REQUIRE_FALSE(std::is_copy_constructible_v); + REQUIREMENT_NOT_MET(std::is_copy_constructible_v); // C(rv) STATIC_REQUIRE(std::is_move_constructible_v); // a = b - STATIC_REQUIRE_FALSE(std::is_copy_assignable_v); + REQUIREMENT_NOT_MET(std::is_copy_assignable_v); // a = rv STATIC_REQUIRE(std::is_move_assignable_v); @@ -274,17 +296,17 @@ TEST_CASE("Collection members", "[collection][container][members][std]") { STATIC_REQUIRE(std::is_same_v().cend()), CollectionType::const_iterator>); // a == b - STATIC_REQUIRE_FALSE(traits::has_equality_comparator_v); + REQUIREMENT_NOT_MET(traits::has_equality_comparator_v); // STATIC_REQUIRE(std::is_convertible_v()==std::declval()), // bool>); // a != b - STATIC_REQUIRE_FALSE(traits::has_inequality_comparator_v); + REQUIREMENT_NOT_MET(traits::has_inequality_comparator_v); // STATIC_REQUIRE(std::is_convertible_v()!=std::declval()), // bool>); // a.swap(b) - STATIC_REQUIRE_FALSE(traits::has_swap_v); + REQUIREMENT_NOT_MET(traits::has_swap_v); // STATIC_REQUIRE( // std::is_same_v().swap(std::declval())), void>); @@ -307,45 +329,56 @@ TEST_CASE("Collection members", "[collection][container][members][std]") { TEST_CASE("Collection iterators", "[collection][container][interator][std]") { using iterator = CollectionType::iterator; using const_iterator = CollectionType::const_iterator; -#if (__cplusplus >= 202002L) - STATIC_REQUIRE_FALSE(std::forward_iterator); - STATIC_REQUIRE_FALSE(std::forward_iterator); -#endif SECTION("LegacyForwardIterator") { +#if (__cplusplus >= 202002L) + REQUIREMENT_NOT_MET(std::forward_iterator); + REQUIREMENT_NOT_MET(std::forward_iterator); +#endif SECTION("LegacyInputIterator") { +#if (__cplusplus >= 202002L) + REQUIREMENT_NOT_MET(std::input_iterator); + REQUIREMENT_NOT_MET(std::input_iterator); +#endif + SECTION("LegacyIterator") { +#if (__cplusplus >= 202002L) + REQUIREMENT_NOT_MET(std::input_or_output_iterator); + REQUIREMENT_NOT_MET(std::input_or_output_iterator); +#endif // CopyConstructible - STATIC_REQUIRE_FALSE(std::is_move_constructible_v && std::is_copy_constructible_v); + REQUIREMENT_NOT_MET(std::is_move_constructible_v); + REQUIREMENT_NOT_MET(std::is_copy_constructible_v); // CopyAssignable - STATIC_REQUIRE_FALSE(std::is_move_assignable_v && std::is_copy_assignable_v); + REQUIREMENT_NOT_MET(std::is_move_assignable_v); + REQUIREMENT_NOT_MET(std::is_copy_assignable_v); // Destructible STATIC_REQUIRE(std::is_nothrow_destructible_v); // Swappable - STATIC_REQUIRE_FALSE(std::is_swappable_v); + REQUIREMENT_NOT_MET(std::is_swappable_v); #if (__cplusplus < 202002L) // std::iterator_traits::value_type - STATIC_REQUIRE_FALSE(traits::has_value_type_v>); + REQUIREMENT_NOT_MET(traits::has_value_type_v>); #endif // std::iterator_traits::difference_type - STATIC_REQUIRE_FALSE(traits::has_difference_type_v>); + REQUIREMENT_NOT_MET(traits::has_difference_type_v>); // std::iterator_traits::reference - STATIC_REQUIRE_FALSE(traits::has_reference_v>); + REQUIREMENT_NOT_MET(traits::has_reference_v>); // std::iterator_traits::pointer - STATIC_REQUIRE_FALSE(traits::has_pointer_v>); + REQUIREMENT_NOT_MET(traits::has_pointer_v>); // std::iterator_traits::iterator_category - STATIC_REQUIRE_FALSE(traits::has_iterator_category_v>); - + REQUIREMENT_NOT_MET(traits::has_iterator_category_v>); // *r - FAIL("Not yet implemented"); - + STATIC_REQUIRE(!std::is_same_v())>); // ++r - FAIL("Not yet implemented"); + STATIC_REQUIRE(traits::has_prefix_v); + STATIC_REQUIRE(std::is_same_v()), + std::add_lvalue_reference_t>); } // EqualityComparable @@ -353,40 +386,80 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { // i != j STATIC_REQUIRE(traits::has_inequality_comparator_v); + STATIC_REQUIRE(std::is_convertible_v() != std::declval()), bool>); // *i - FAIL("Not yet implemented"); + REQUIREMENT_NOT_MET(traits::has_reference_v); + // STATIC_REQUIRE(!std::is_same_v::reference, + // decltype(*std::declval())>); + REQUIREMENT_NOT_MET(traits::has_value_type_v); + // STATIC_REQUIRE(!std::is_convertible_v()), + // std::iterator_traits::value_type>); // i->m - FAIL("Not yet implemented"); + STATIC_REQUIRE( + std::is_same_v()->energy()), decltype((*std::declval()).energy())>); // ++r - FAIL("Not yet implemented"); + STATIC_REQUIRE(traits::has_prefix_v); + STATIC_REQUIRE(std::is_same_v()), + std::add_lvalue_reference_t>); // (void)r++ - FAIL("Not yet implemented"); + REQUIREMENT_NOT_MET(traits::has_postfix_v); + STATIC_REQUIRE(traits::has_prefix_v); + REQUIREMENT_NOT_MET(traits::has_value_type_v>); + // STATIC_REQUIRE(std::is_same_v()), + // decltype((void)std::declval()++)>); //*r++ - STATIC_REQUIRE_FALSE(traits::has_value_type_v>); + REQUIREMENT_NOT_MET(traits::has_postfix_v); + REQUIREMENT_NOT_MET(traits::has_value_type_v>); // STATIC_REQUIRE(std::is_convertible_v < decltype(*std::declval()++), // std::iterator_traits::value_type >>); } + // Mutable iterator: reference same as value_type& or value_type&& + REQUIREMENT_NOT_MET(traits::has_reference_v); + REQUIREMENT_NOT_MET(traits::has_value_type_v); + // STATIC_REQUIRE(std::is_same_v::reference, + // std::add_lvalue_reference_t::value_type>> || + // std::is_same_v::reference, + // std::add_rvalue_reference_t::value_type>>); + + // Immutable iterator: reference same as const value_type& or const value_type&& + REQUIREMENT_NOT_MET(traits::has_reference_v); + REQUIREMENT_NOT_MET(traits::has_value_type_v); + // STATIC_REQUIRE(std::is_same_v::reference, + // std::add_const_t::value_type>>> + // || + // std::is_same_v::reference, + // std::add_const_t::value_type>>>); + // DefaultConstructible - STATIC_REQUIRE_FALSE(std::is_default_constructible_v); + REQUIREMENT_NOT_MET(std::is_default_constructible_v); } - // multipass guarantee + // Multipass guarantee FAIL("Not yet implemented"); - // i++ + // Singular iterators FAIL("Not yet implemented"); + // i++ + REQUIREMENT_NOT_MET(traits::has_postfix_v); + // STATIC_REQUIRE(std::is_same_v()++), CollectionType::iterator>); + // *i++ - FAIL("Not yet implemented"); + REQUIREMENT_NOT_MET(traits::has_postfix_v); + REQUIREMENT_NOT_MET(traits::has_reference_v>); + // STATIC_REQUIRE(std::is_same_v < decltype(*std::declval()++), + // std::iterator_traits::reference >>); } TEST_CASE("Collection and std::algorithms", "[collection][container][algorithm][std]") { auto a = CollectionType(); FAIL("Not yet implemented"); } + +#undef REQUIREMENT_NOT_MET \ No newline at end of file From 0a98b448c9c6af023da1203a1402f6c4a833a747 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Thu, 9 May 2024 01:05:41 +0200 Subject: [PATCH 10/43] add short note on AllocatorAwareContainer, simplify comments --- doc/collections_as_container.md | 24 ++++++++++++++++++------ tests/unittests/std_interoperability.cpp | 9 +++++++-- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 5854f006e..700376d5e 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -38,7 +38,7 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | `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 iterators as a *Iterator* +## Collection iterators as an *Iterator* ### Iterator summary @@ -52,8 +52,8 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment | |-------------|-------------------------------------------|---------| -| [*CopyConstructible*](https://en.cppreference.com/w/cpp/named_req/CopyConstructible) | ❌ no / ❌ no | Move constructor not defined, copy constructor not defined | -| [*CopyAssignable*](https://en.cppreference.com/w/cpp/named_req/CopyAssignable) | ❌ no / ❌ no | Move assignment not defined, copy assignment not defined | +| [*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 | | [*Destructible*](https://en.cppreference.com/w/cpp/named_req/Destructible) | ✔️ yes / ✔️ yes | | | [*Swappable*](https://en.cppreference.com/w/cpp/named_req/Swappable) | ✔️ yes / ✔️ yes | | | `std::iterator_traits::value_type` (Until C++20 ) | ❌ no / ❌ no | Not defined | @@ -81,7 +81,7 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | `i->m` | | Same as `(*i).m` | ✔️ yes / ✔️ yes | | | `++r` | `It&` | | ✔️ yes / ✔️ yes | | | `(void)r++` | | Same as `(void)++r` | ❌ no / ❌ no | Postfix not defined | -| `*r++` | Convertible to `value_type` | Same as `value_type x = *r; ++r; return x;` | ❌ no / ❌ no | Postfix not defined, `value_type` not defined | +| `*r++` | Convertible to `value_type` | Same as `value_type x = *r; ++r; return x;` | ❌ no / ❌ no | Postfix and `value_type` not defined | ### LegacyForwardIterator @@ -89,14 +89,26 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b |-------------|-------------------------------------------|---------| | [*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 | Default initialization no 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` not defined, `value_type` 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 | ❌ no / ❌ no | | | Singular iterators | ❌ no / ❌ no | | | Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment | |------------|-------------|-----------|-------------------------------------------|---------| | `i++` | `It` | Same as `It ip = i; ++i; return ip;` | ❌ no / ❌ no | Postfix not defined | -| `*i++` | `reference` | | ❌ no / ❌ no | Postfix not defined, `reference` not defined| +| `*i++` | `reference` | | ❌ no / ❌ no | Postfix and `reference` not defined| + +## Collection as AllocatorAwareContainer + +The C++ standard specifies [AllocatorAwareContainer](https://en.cppreference.com/w/cpp/named_req/AllocatorAwareContainer) for containers that can use other allocators beside the default allocator. + +PODIO collections don't provide customization point for allocators and use only the default allocator. Therefore they are not *AllocatorAwareContainers*. + +### AllocatorAwareContainer types + +| Name | Requirements | Fulfilled by Collection? | Comment | +|------|--------------|--------------------------|---------| +| `allocator_type` | `allocator_type::value_type` same as `value_type` | ❌ no | `allocator_type` not defined | ## Collection iterators and standard iterator adapters diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index 17818b72e..cc923e51c 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -206,7 +206,7 @@ template inline constexpr bool has_postfix_v = has_postfix::value; } // namespace traits -TEST_CASE("Collection types", "[collection][container][types][std]") { +TEST_CASE("Collection container types", "[collection][container][types][std]") { // value_type STATIC_REQUIRE(traits::has_value_type_v); @@ -256,7 +256,7 @@ TEST_CASE("Collection types", "[collection][container][types][std]") { std::numeric_limits::max()); } -TEST_CASE("Collection members", "[collection][container][members][std]") { +TEST_CASE("Collection container members", "[collection][container][members][std]") { // C() STATIC_REQUIRE(std::is_default_constructible_v); REQUIRE(CollectionType().empty() == true); @@ -326,6 +326,11 @@ TEST_CASE("Collection members", "[collection][container][members][std]") { STATIC_REQUIRE(std::is_convertible_v().empty()), bool>); } +TEST_CASE("Collection AllocatorAwareContainer types", "[collection][container][types][std]") { + REQUIREMENT_NOT_MET(traits::has_allocator_type_v); +} +// TODO add tests for AllocatorAwareContainer statements and expressions + TEST_CASE("Collection iterators", "[collection][container][interator][std]") { using iterator = CollectionType::iterator; using const_iterator = CollectionType::const_iterator; From a8af6eab03a86add7e2b4d4a35b335c69fe35bca Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Thu, 9 May 2024 01:29:49 +0200 Subject: [PATCH 11/43] add singularity iterators check --- doc/collections_as_container.md | 4 ++-- tests/unittests/std_interoperability.cpp | 8 +++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 700376d5e..efb67fe5b 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -88,10 +88,10 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | 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 | Default initialization no defined | +| [*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 | ❌ no / ❌ no | | -| Singular iterators | ❌ no / ❌ no | | +| Singular iterators | ❌ no / ❌ no | Value initialization not defined | | Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment | |------------|-------------|-----------|-------------------------------------------|---------| diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index cc923e51c..4d2716d87 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -449,7 +449,13 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { FAIL("Not yet implemented"); // Singular iterators - FAIL("Not yet implemented"); + STATIC_REQUIRE(traits::has_equality_comparator_v); + REQUIREMENT_NOT_MET(std::is_default_constructible_v); + //{ + // CollectionType some_collection{}; + // REQUIRE(iterator{} == some_collection.end()); + // REQUIRE(iterator{} == iterator{}); + //} // i++ REQUIREMENT_NOT_MET(traits::has_postfix_v); From 18f9e71f900c8681df6521d38c4e60883521871b Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Thu, 9 May 2024 12:18:16 +0200 Subject: [PATCH 12/43] rename pre-increment and post-increment --- doc/collections_as_container.md | 8 +++---- tests/unittests/std_interoperability.cpp | 30 ++++++++++++------------ 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index efb67fe5b..09eae40fb 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -80,8 +80,8 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | `*i` | `reference`, convertible to `value_type` | | ❌ no / ❌ no | `reference` and `value_type` not defined | | `i->m` | | Same as `(*i).m` | ✔️ yes / ✔️ yes | | | `++r` | `It&` | | ✔️ yes / ✔️ yes | | -| `(void)r++` | | Same as `(void)++r` | ❌ no / ❌ no | Postfix not defined | -| `*r++` | Convertible to `value_type` | Same as `value_type x = *r; ++r; return x;` | ❌ no / ❌ no | Postfix and `value_type` not defined | +| `(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 | ### LegacyForwardIterator @@ -95,8 +95,8 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment | |------------|-------------|-----------|-------------------------------------------|---------| -| `i++` | `It` | Same as `It ip = i; ++i; return ip;` | ❌ no / ❌ no | Postfix not defined | -| `*i++` | `reference` | | ❌ no / ❌ no | Postfix and `reference` not defined| +| `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| ## Collection as AllocatorAwareContainer diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index 4d2716d87..003881e1e 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -189,21 +189,21 @@ struct has_empty().empty())>> : std::tru template inline constexpr bool has_empty_v = has_empty::value; -// T::operator++() (prefix) +// T::operator++() (preincrement) template -struct has_prefix : std::false_type {}; +struct has_preincrement : std::false_type {}; template -struct has_prefix())>> : std::true_type {}; +struct has_preincrement())>> : std::true_type {}; template -inline constexpr bool has_prefix_v = has_prefix::value; +inline constexpr bool has_preincrement_v = has_preincrement::value; -// T::operator++(int) (postfix) +// T::operator++(int) (postincrement) template -struct has_postfix : std::false_type {}; +struct has_postincrement : std::false_type {}; template -struct has_postfix()++)>> : std::true_type {}; +struct has_postincrement()++)>> : std::true_type {}; template -inline constexpr bool has_postfix_v = has_postfix::value; +inline constexpr bool has_postincrement_v = has_postincrement::value; } // namespace traits TEST_CASE("Collection container types", "[collection][container][types][std]") { @@ -381,7 +381,7 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { // *r STATIC_REQUIRE(!std::is_same_v())>); // ++r - STATIC_REQUIRE(traits::has_prefix_v); + STATIC_REQUIRE(traits::has_preincrement_v); STATIC_REQUIRE(std::is_same_v()), std::add_lvalue_reference_t>); } @@ -406,19 +406,19 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { std::is_same_v()->energy()), decltype((*std::declval()).energy())>); // ++r - STATIC_REQUIRE(traits::has_prefix_v); + STATIC_REQUIRE(traits::has_preincrement_v); STATIC_REQUIRE(std::is_same_v()), std::add_lvalue_reference_t>); // (void)r++ - REQUIREMENT_NOT_MET(traits::has_postfix_v); - STATIC_REQUIRE(traits::has_prefix_v); + REQUIREMENT_NOT_MET(traits::has_postincrement_v); + STATIC_REQUIRE(traits::has_preincrement_v); REQUIREMENT_NOT_MET(traits::has_value_type_v>); // STATIC_REQUIRE(std::is_same_v()), // decltype((void)std::declval()++)>); //*r++ - REQUIREMENT_NOT_MET(traits::has_postfix_v); + REQUIREMENT_NOT_MET(traits::has_postincrement_v); REQUIREMENT_NOT_MET(traits::has_value_type_v>); // STATIC_REQUIRE(std::is_convertible_v < decltype(*std::declval()++), // std::iterator_traits::value_type >>); @@ -458,11 +458,11 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { //} // i++ - REQUIREMENT_NOT_MET(traits::has_postfix_v); + REQUIREMENT_NOT_MET(traits::has_postincrement_v); // STATIC_REQUIRE(std::is_same_v()++), CollectionType::iterator>); // *i++ - REQUIREMENT_NOT_MET(traits::has_postfix_v); + REQUIREMENT_NOT_MET(traits::has_postincrement_v); REQUIREMENT_NOT_MET(traits::has_reference_v>); // STATIC_REQUIRE(std::is_same_v < decltype(*std::declval()++), // std::iterator_traits::reference >>); From 43a6b38f6c96be096a74a44f8e8ecc866db34c57 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Thu, 9 May 2024 12:24:30 +0200 Subject: [PATCH 13/43] add comment on expression and statements in AllocatorAwareContainer --- doc/collections_as_container.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 09eae40fb..71b44a3fc 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -110,6 +110,10 @@ PODIO collections don't provide customization point for allocators and use only |------|--------------|--------------------------|---------| | `allocator_type` | `allocator_type::value_type` same as `value_type` | ❌ no | `allocator_type` not defined | +### AllocatorAwareContainer expression and statements + +The PODIO Collections currently are not checked against expression and statements requirements for *AllocatorAwareContainer*. + ## Collection iterators and standard iterator adapters ## Collection and standard algorithms From 0f4975dc93a072ee2bf6abe16661e4077e33ea63 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Thu, 9 May 2024 14:27:31 +0200 Subject: [PATCH 14/43] fix contextually convertible, add multipass guarantee --- doc/collections_as_container.md | 6 +++--- tests/unittests/std_interoperability.cpp | 26 ++++++++++++++++++++++-- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 71b44a3fc..e4a64f56a 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -76,7 +76,7 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment | |------------|-------------|-----------|-------------------------------------------|---------| -| `i != j` | Convertible to `bool` | Same as `!(i==j)` | ✔️ yes / ✔️ yes | | +| `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->m` | | Same as `(*i).m` | ✔️ yes / ✔️ yes | | | `++r` | `It&` | | ✔️ yes / ✔️ yes | | @@ -90,8 +90,8 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | [*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 | ❌ no / ❌ no | | -| Singular iterators | ❌ no / ❌ no | Value initialization 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 | | Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment | |------------|-------------|-----------|-------------------------------------------|---------| diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index 003881e1e..4fe86d663 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -299,6 +299,11 @@ TEST_CASE("Collection container members", "[collection][container][members][std] REQUIREMENT_NOT_MET(traits::has_equality_comparator_v); // STATIC_REQUIRE(std::is_convertible_v()==std::declval()), // bool>); + // value_type is EqualityComparable + STATIC_REQUIRE(traits::has_equality_comparator_v); + STATIC_REQUIRE( + std::is_convertible_v< + decltype(std::declval() != std::declval()), bool>); // a != b REQUIREMENT_NOT_MET(traits::has_inequality_comparator_v); @@ -388,10 +393,11 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { // EqualityComparable STATIC_REQUIRE(traits::has_equality_comparator_v); + STATIC_REQUIRE(std::is_convertible_v() != std::declval()), bool>); // i != j STATIC_REQUIRE(traits::has_inequality_comparator_v); - STATIC_REQUIRE(std::is_convertible_v() != std::declval()), bool>); + STATIC_REQUIRE(std::is_constructible_v() != std::declval())>); // *i REQUIREMENT_NOT_MET(traits::has_reference_v); @@ -446,7 +452,23 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { } // Multipass guarantee - FAIL("Not yet implemented"); + { + CollectionType coll; + for (int i = 0; i < 3; ++i) { + coll.create(); + } + auto a = coll.begin(); + auto b = coll.begin(); + REQUIRE(a == b); + REQUIRE(*a == *b); + REQUIRE(++a == ++b); + REQUIRE(*a == *b); + REQUIREMENT_NOT_MET(std::is_copy_constructible_v); + // auto a_copy = a; + // ++a_copy; + // REQUIRE(a == b); + // REQUIRE(*a == *b); + } // Singular iterators STATIC_REQUIRE(traits::has_equality_comparator_v); From 9ebbeeab5a4e43efb99a263e9fc9330126cf767c Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Thu, 9 May 2024 15:49:12 +0200 Subject: [PATCH 15/43] add placeholders for LegacyOutputIterator --- doc/collections_as_container.md | 15 ++++++++++++ tests/unittests/std_interoperability.cpp | 31 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index e4a64f56a..db5d86423 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -47,6 +47,7 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | [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)) | | [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)) | ### LegacyIterator @@ -98,6 +99,20 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | `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| +### LegacyOutputIterator + +| Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment | +|-------------|-------------------------------------------|---------| +| [*LegacyIterator*](https://en.cppreference.com/w/cpp/named_req/Iterator) | ❌ no / ❌ no | [See above](#legacyiterator) | +| Is pointer type or class type | ✔️ yes / ✔️ yes | | + +| Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment | +|------------|-------------|-----------|-------------------------------------------|---------| +| `*r = o` | | | ❌ no / ❌ no | | +| `++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;`| ✔️ yes / ❌ no | | + ## Collection as AllocatorAwareContainer The C++ standard specifies [AllocatorAwareContainer](https://en.cppreference.com/w/cpp/named_req/AllocatorAwareContainer) for containers that can use other allocators beside the default allocator. diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index 4fe86d663..d74416ef4 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -488,6 +488,37 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { REQUIREMENT_NOT_MET(traits::has_reference_v>); // STATIC_REQUIRE(std::is_same_v < decltype(*std::declval()++), // std::iterator_traits::reference >>); + + SECTION("LegacyOutputIterator") { +#if (__cplusplus >= 202002L) + REQUIREMENT_NOT_MET(std::output_iterator); + REQUIREMENT_NOT_MET(std::output_iterator); +#endif + + // is class type or pointer type + STATIC_REQUIRE(std::is_pointer_v || std::is_class_v); + STATIC_REQUIRE(std::is_pointer_v || std::is_class_v); + + // *r = o + FAIL("Not implemented yet"); + + // ++r + STATIC_REQUIRE(traits::has_preincrement_v); + STATIC_REQUIRE(std::is_same_v()), std::add_lvalue_reference_t>); + + // r++ + REQUIREMENT_NOT_MET(traits::has_postincrement_v); + // STATIC_REQUIRE(std::is_convertible_v()++), + // std::add_const_t>>); + + //*r++ =o + FAIL("Not implemented yet"); + } +} + +TEST_CASE("Collection and std iterator adaptors", "[collection][container][adapter][std]") { + auto a = CollectionType(); + FAIL("Not yet implemented"); } TEST_CASE("Collection and std::algorithms", "[collection][container][algorithm][std]") { From cd4ea94dda8755aa5d863c48d205251ac5b293fa Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Thu, 9 May 2024 18:37:40 +0200 Subject: [PATCH 16/43] add adaptors table, algorithms table --- doc/collections_as_container.md | 175 ++++++++++++++++++++++++++------ 1 file changed, 146 insertions(+), 29 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index db5d86423..01cfe74e8 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -129,7 +129,18 @@ PODIO collections don't provide customization point for allocators and use only The PODIO Collections currently are not checked against expression and statements requirements for *AllocatorAwareContainer*. -## Collection iterators and standard iterator adapters +## Collection iterators and standard iterator adaptors + +| Adaptor | Compatible with Collection? | Comment | +|---------|-----------------------------|---------| +| `std::reverse_iterator` | ❌ no | `iterator` and `const_iterator` not *LegacyBidirectionalIterator* or `std::bidirectional_iterator` | +| `std::back_insert_iterator` | ❌ no | | +| `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 | Move from collection conflicts collection elements ownership semantic | +| `std::counted_iterator` | ✔️ yes | | + ## Collection and standard algorithms @@ -137,41 +148,147 @@ The PODIO Collections currently are not checked against expression and statement | Algorithm | Compatible with Collection? | Comment | | ----------|-----------------------------|---------| -| `all_of` | | | -| `any_of` | | | -| `none_of` | | | -| `for_each` | | | -| `for_each_n` | | | -| `count` | | | -| `count_if` | | | -| `mismatch` | | | -| `find` | | | -| `find_if` | | | -| `find_if_not` | | | -| `find_end` | | | -| `find_first_of` | | | -| `adjacent_find` | | | -| `search` | | | -| `search_n` | | | +| `std::all_of` | | | +| `std::any_of` | | | +| `std::none_of` | | | +| `std::for_each` | | | +| `std::for_each_n` | | | +| `std::count` | | | +| `std::count_if` | | | +| `std::mismatch` | | | +| `std::find` | | | +| `std::find_if` | | | +| `std::find_if_not` | | | +| `std::find_end` | | | +| `std::find_first_of` | | | +| `std::adjacent_find` | | | +| `std::search` | | | +| `std::search_n` | | | ### Modifying sequence operations -### Partitioning operations - -### Sorting operations - -### Binary search operations (on sorted ranges) - -### Other operations on sorted ranges - -### Set operations (on sorted ranges) - -### Heap operations +| Algorithm | Compatible with Collection? | Comment | +|----------------------|-----------------------------|---------| +| `std::copy` | | | +| `std::copy_if` | | | +| `std::copy_n` | | | +| `std::copy_backward` | | | +| `std::move` | | | +| `std::move_backward` | | | +| `std::fill` | | | +| `std::fill_n` | | | +| `std::transform` | | | +| `std::generate` | | | +| `std::generate_n` | | | +| `std::remove` | | | +| `std::remove_if` | | | +| `std::remove_copy` | | | +| `std::remove_copy_if` | | | +| `std::replace` | | | +| `std::replace_if` | | | +| `std::replace_copy` | | | +| `std::replace_copy_if` | | | +| `std::swap` | | | +| `std::swap_ranges` | | | +| `std::iter_swap` | | | +| `std::reverse` | | | +| `std::reverse_copy` | | | +| `std::rotate` | | | +| `std::rotate_copy` | | | +| `std::shift_left` | | | +| `std::shift_right` | | | +| `std::random_shuffle` | | | +| `std::shuffle` | | | +| `std::sample` | | | +| `std::unique` | | | +| `std::unique_copy` | | | + +### Partitioning Operations + +| Algorithm | Compatible with Collection? | Comment | +|----------------------|-----------------------------|---------| +| `std::is_partitioned` | | | +| `std::partition` | | | +| `std::partition_copy` | | | +| `std::stable_partition` | | | +| `std::partition_point` | | | + +### Sorting Operations + +| Algorithm | Compatible with Collection? | Comment | +|----------------------|-----------------------------|---------| +| `std::is_sorted` | | | +| `std::is_sorted_until` | | | +| `std::sort` | | | +| `std::partial_sort` | | | +| `std::partial_sort_copy` | | | +| `std::stable_sort` | | | +| `std::nth_element` | | | + +### Binary Search Operations (on Sorted Ranges) + +| Algorithm | Compatible with Collection? | Comment | +|----------------------|-----------------------------|---------| +| `std::lower_bound` | | | +| `std::upper_bound` | | | +| `std::binary_search` | | | +| `std::equal_range` | | | + +### Other Operations on Sorted Ranges + +| Algorithm | Compatible with Collection? | Comment | +|----------------------|-----------------------------|---------| +| `std::merge` | | | +| `std::inplace_merge` | | | + +### Set Operations (on Sorted Ranges) + +| Algorithm | Compatible with Collection? | Comment | +|----------------------|-----------------------------|---------| +| `std::includes` | | | +| `std::set_difference` | | | +| `std::set_intersection` | | | +| `std::set_symmetric_difference` | | | +| `std::set_union` | | | + +### Heap Operations + +| Algorithm | Compatible with Collection? | Comment | +|----------------------|-----------------------------|---------| +| `std::is_heap` | | | +| `std::is_heap_until` | | | +| `std::make_heap` | | | +| `std::push_heap` | | | +| `std::pop_heap` | | | +| `std::sort_heap` | | | + +### Minimum/Maximum Operations + +| Algorithm | Compatible with Collection? | Comment | +|----------------------|-----------------------------|---------| +| `std::max` | | | +| `std::max_element` | | | +| `std::min` | | | +| `std::min_element` | | | +| `std::minmax` | | | +| `std::minmax_element` | | | +| `std::clamp` | | | -### Minimum/maximum operations ### Comparison operations +| Algorithm | Compatible with Collection? | Comment | +| ----------|-----------------------------|---------| +| `std::equal` | | | +| `std::lexicographical_compare` | | | +| `std::lexicographical_compare_three_way` | | | + ### Permutation operations +| Algorithm | Compatible with Collection? | Comment | +| ----------|-----------------------------|---------| +| `std::is_permutation` | | | +| `std::next_permutation ` | | | +| `std::prev_permutation` | | | + ## Standard ranges From 01b54276c7caf3a89f86985afade90c0a89d56fb Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Thu, 9 May 2024 21:03:35 +0200 Subject: [PATCH 17/43] add comment on algorithms and ranges --- doc/collections_as_container.md | 153 ++------------------------------ 1 file changed, 5 insertions(+), 148 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 01cfe74e8..f9bf6dc2f 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -144,151 +144,8 @@ The PODIO Collections currently are not checked against expression and statement ## Collection and standard algorithms -### Non-modifying sequence operations - -| Algorithm | Compatible with Collection? | Comment | -| ----------|-----------------------------|---------| -| `std::all_of` | | | -| `std::any_of` | | | -| `std::none_of` | | | -| `std::for_each` | | | -| `std::for_each_n` | | | -| `std::count` | | | -| `std::count_if` | | | -| `std::mismatch` | | | -| `std::find` | | | -| `std::find_if` | | | -| `std::find_if_not` | | | -| `std::find_end` | | | -| `std::find_first_of` | | | -| `std::adjacent_find` | | | -| `std::search` | | | -| `std::search_n` | | | - -### Modifying sequence operations - -| Algorithm | Compatible with Collection? | Comment | -|----------------------|-----------------------------|---------| -| `std::copy` | | | -| `std::copy_if` | | | -| `std::copy_n` | | | -| `std::copy_backward` | | | -| `std::move` | | | -| `std::move_backward` | | | -| `std::fill` | | | -| `std::fill_n` | | | -| `std::transform` | | | -| `std::generate` | | | -| `std::generate_n` | | | -| `std::remove` | | | -| `std::remove_if` | | | -| `std::remove_copy` | | | -| `std::remove_copy_if` | | | -| `std::replace` | | | -| `std::replace_if` | | | -| `std::replace_copy` | | | -| `std::replace_copy_if` | | | -| `std::swap` | | | -| `std::swap_ranges` | | | -| `std::iter_swap` | | | -| `std::reverse` | | | -| `std::reverse_copy` | | | -| `std::rotate` | | | -| `std::rotate_copy` | | | -| `std::shift_left` | | | -| `std::shift_right` | | | -| `std::random_shuffle` | | | -| `std::shuffle` | | | -| `std::sample` | | | -| `std::unique` | | | -| `std::unique_copy` | | | - -### Partitioning Operations - -| Algorithm | Compatible with Collection? | Comment | -|----------------------|-----------------------------|---------| -| `std::is_partitioned` | | | -| `std::partition` | | | -| `std::partition_copy` | | | -| `std::stable_partition` | | | -| `std::partition_point` | | | - -### Sorting Operations - -| Algorithm | Compatible with Collection? | Comment | -|----------------------|-----------------------------|---------| -| `std::is_sorted` | | | -| `std::is_sorted_until` | | | -| `std::sort` | | | -| `std::partial_sort` | | | -| `std::partial_sort_copy` | | | -| `std::stable_sort` | | | -| `std::nth_element` | | | - -### Binary Search Operations (on Sorted Ranges) - -| Algorithm | Compatible with Collection? | Comment | -|----------------------|-----------------------------|---------| -| `std::lower_bound` | | | -| `std::upper_bound` | | | -| `std::binary_search` | | | -| `std::equal_range` | | | - -### Other Operations on Sorted Ranges - -| Algorithm | Compatible with Collection? | Comment | -|----------------------|-----------------------------|---------| -| `std::merge` | | | -| `std::inplace_merge` | | | - -### Set Operations (on Sorted Ranges) - -| Algorithm | Compatible with Collection? | Comment | -|----------------------|-----------------------------|---------| -| `std::includes` | | | -| `std::set_difference` | | | -| `std::set_intersection` | | | -| `std::set_symmetric_difference` | | | -| `std::set_union` | | | - -### Heap Operations - -| Algorithm | Compatible with Collection? | Comment | -|----------------------|-----------------------------|---------| -| `std::is_heap` | | | -| `std::is_heap_until` | | | -| `std::make_heap` | | | -| `std::push_heap` | | | -| `std::pop_heap` | | | -| `std::sort_heap` | | | - -### Minimum/Maximum Operations - -| Algorithm | Compatible with Collection? | Comment | -|----------------------|-----------------------------|---------| -| `std::max` | | | -| `std::max_element` | | | -| `std::min` | | | -| `std::min_element` | | | -| `std::minmax` | | | -| `std::minmax_element` | | | -| `std::clamp` | | | - - -### Comparison operations - -| Algorithm | Compatible with Collection? | Comment | -| ----------|-----------------------------|---------| -| `std::equal` | | | -| `std::lexicographical_compare` | | | -| `std::lexicographical_compare_three_way` | | | - -### Permutation operations - -| Algorithm | Compatible with Collection? | Comment | -| ----------|-----------------------------|---------| -| `std::is_permutation` | | | -| `std::next_permutation ` | | | -| `std::prev_permutation` | | | - -## Standard ranges +Most of the standard algorithms require the iterators to be at least *InputIterator*. The iterators of PODIO collection don't fulfil this requirement, therefore they are not compatible with standard algorithms according to specification. In practice some algorithms may still compile with the collections depending on a implementation of a given algorithm. + +## Standard range algorithms + +The standard range algorithm use constrains to operate at least on `std::input_iterator`s and `std::ranges::input_range`s. The iterators of PODIO collection don't model these concepts, therefore can't be use with standard range algorithms. From b5065fb9bb377b992944dcffcdd2ac6cfc961318 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Thu, 9 May 2024 23:01:02 +0200 Subject: [PATCH 18/43] add iterator concepts table, move iterator concepts to separate test --- doc/collections_as_container.md | 20 ++++++++-- tests/unittests/std_interoperability.cpp | 50 ++++++++++++++++-------- 2 files changed, 51 insertions(+), 19 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index f9bf6dc2f..a6371286c 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -42,13 +42,27 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b ### Iterator summary -| Named requirement | Collection::`iterator`| Collection::`const_iterator`| +| 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)) | | [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)) | +| Concept | `iterator` | `const_iterator` | +|---------|------------------------|------------------------------| +| `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::input_iterator` | ❌ no | ❌ no | +| `std::output_iterator` | ❌ no | ❌ no | +| `std::forward_iterator` | ❌ no | ❌ no | +| `std::bidirectional_iterator` | ❌ no | ❌ no | +| `std::random_access_iterator` | ❌ no | ❌ no | +| `std::contiguous_iterator` | ❌ no | ❌ no | + ### LegacyIterator | Requirement | Fulfilled by `iterator`/`const_iterator`? | Comment | @@ -65,8 +79,8 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment | |------------|-------------|-----------|-------------------------------------------|---------| -| `*r` | Unspecified | Dereferenceable | ✔️ yes / ✔️ yes | | -| `++r` | `It&` | Incrementable | ✔️ yes / ✔️ yes | | +| `*r` | Unspecified | | ✔️ yes / ✔️ yes | | +| `++r` | `It&` | | ✔️ yes / ✔️ yes | | ### LegacyInputIterator diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index d74416ef4..3c25132e9 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -341,22 +341,11 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { using const_iterator = CollectionType::const_iterator; SECTION("LegacyForwardIterator") { -#if (__cplusplus >= 202002L) - REQUIREMENT_NOT_MET(std::forward_iterator); - REQUIREMENT_NOT_MET(std::forward_iterator); -#endif SECTION("LegacyInputIterator") { -#if (__cplusplus >= 202002L) - REQUIREMENT_NOT_MET(std::input_iterator); - REQUIREMENT_NOT_MET(std::input_iterator); -#endif SECTION("LegacyIterator") { -#if (__cplusplus >= 202002L) - REQUIREMENT_NOT_MET(std::input_or_output_iterator); - REQUIREMENT_NOT_MET(std::input_or_output_iterator); -#endif + // CopyConstructible REQUIREMENT_NOT_MET(std::is_move_constructible_v); REQUIREMENT_NOT_MET(std::is_copy_constructible_v); @@ -490,10 +479,6 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { // std::iterator_traits::reference >>); SECTION("LegacyOutputIterator") { -#if (__cplusplus >= 202002L) - REQUIREMENT_NOT_MET(std::output_iterator); - REQUIREMENT_NOT_MET(std::output_iterator); -#endif // is class type or pointer type STATIC_REQUIRE(std::is_pointer_v || std::is_class_v); @@ -516,6 +501,39 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { } } +TEST_CASE("Collection and iterator concepts") { +#if (__cplusplus >= 202002L) + SECTION("Iterator") { + using iterator = CollectionType::iterator; + REQUIREMENT_NOT_MET(std::indirectly_readable); + REQUIREMENT_NOT_MET(std::indirectly_writable); + REQUIREMENT_NOT_MET(std::weakly_incrementable); + REQUIREMENT_NOT_MET(std::incrementable); + REQUIREMENT_NOT_MET(std::input_or_output_iterator); + REQUIREMENT_NOT_MET(std::input_iterator); + REQUIREMENT_NOT_MET(std::output_iterator); + REQUIREMENT_NOT_MET(std::forward_iterator); + REQUIREMENT_NOT_MET(std::bidirectional_iterator); + REQUIREMENT_NOT_MET(std::random_access_iterator); + REQUIREMENT_NOT_MET(std::contiguous_iterator); + } + SECTION("Const iterator") { + using const_iterator = CollectionType::const_iterator; + REQUIREMENT_NOT_MET(std::indirectly_readable); + REQUIREMENT_NOT_MET(std::indirectly_writable); + REQUIREMENT_NOT_MET(std::weakly_incrementable); + REQUIREMENT_NOT_MET(std::incrementable); + REQUIREMENT_NOT_MET(std::input_or_output_iterator); + REQUIREMENT_NOT_MET(std::input_iterator); + REQUIREMENT_NOT_MET(std::output_iterator); + REQUIREMENT_NOT_MET(std::forward_iterator); + REQUIREMENT_NOT_MET(std::bidirectional_iterator); + REQUIREMENT_NOT_MET(std::random_access_iterator); + REQUIREMENT_NOT_MET(std::contiguous_iterator); + } +#endif +} + TEST_CASE("Collection and std iterator adaptors", "[collection][container][adapter][std]") { auto a = CollectionType(); FAIL("Not yet implemented"); From d8dabe580f39e2f093288f47f6c37b358b2d4b09 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Thu, 9 May 2024 23:57:58 +0200 Subject: [PATCH 19/43] add tests for dereference assignment (and increment) --- doc/collections_as_container.md | 4 +-- tests/unittests/std_interoperability.cpp | 32 ++++++++++++++++++------ 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index a6371286c..9efb10d33 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -122,10 +122,10 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment | |------------|-------------|-----------|-------------------------------------------|---------| -| `*r = o` | | | ❌ no / ❌ no | | +| `*r = o` | | | ✔️ yes / ❌ no | `iterator` defines assigning `value_type::mutable_type`, `const_iterator` doesn't define assignment | | `++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;`| ✔️ yes / ❌ no | | +| `*r++ = o` | | Same as `*r = o; ++r;`| ❌ no / ❌ no | Post-increment not defined | ## Collection as AllocatorAwareContainer diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index 3c25132e9..0357d2b40 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -1,4 +1,5 @@ #include "datamodel/ExampleHitCollection.h" +#include "datamodel/MutableExampleHit.h" #include #include #include @@ -204,6 +205,25 @@ template struct has_postincrement()++)>> : std::true_type {}; template inline constexpr bool has_postincrement_v = has_postincrement::value; + +// *It = val +template +struct has_dereference_assignment : std::false_type {}; +template +struct has_dereference_assignment() = std::declval())>> + : std::true_type {}; +template +inline constexpr bool has_dereference_assignment_v = has_dereference_assignment::value; + +// *It++ = val +template +struct has_dereference_assignment_increment : std::false_type {}; +template +struct has_dereference_assignment_increment()++ = std::declval())>> + : std::true_type {}; +template +inline constexpr bool has_dereference_assignment_increment_v = has_dereference_assignment_increment::value; } // namespace traits TEST_CASE("Collection container types", "[collection][container][types][std]") { @@ -485,7 +505,8 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { STATIC_REQUIRE(std::is_pointer_v || std::is_class_v); // *r = o - FAIL("Not implemented yet"); + REQUIREMENT_NOT_MET(traits::has_dereference_assignment_v); + STATIC_REQUIRE(traits::has_dereference_assignment_v); // ++r STATIC_REQUIRE(traits::has_preincrement_v); @@ -497,7 +518,9 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { // std::add_const_t>>); //*r++ =o - FAIL("Not implemented yet"); + REQUIREMENT_NOT_MET(traits::has_dereference_assignment_increment_v); + REQUIREMENT_NOT_MET( + traits::has_dereference_assignment_increment_v); } } @@ -539,9 +562,4 @@ TEST_CASE("Collection and std iterator adaptors", "[collection][container][adapt FAIL("Not yet implemented"); } -TEST_CASE("Collection and std::algorithms", "[collection][container][algorithm][std]") { - auto a = CollectionType(); - FAIL("Not yet implemented"); -} - #undef REQUIREMENT_NOT_MET \ No newline at end of file From b3760e0bb0e2f09ea4fa8db58778ed9237b3333a Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Fri, 10 May 2024 00:29:07 +0200 Subject: [PATCH 20/43] updated adaptors --- doc/collections_as_container.md | 4 ++-- tests/unittests/std_interoperability.cpp | 19 ++++++++++++++++--- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 9efb10d33..823d9f823 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -148,12 +148,12 @@ The PODIO Collections currently are not checked against expression and statement | Adaptor | Compatible with Collection? | Comment | |---------|-----------------------------|---------| | `std::reverse_iterator` | ❌ no | `iterator` and `const_iterator` not *LegacyBidirectionalIterator* or `std::bidirectional_iterator` | -| `std::back_insert_iterator` | ❌ no | | +| `std::back_insert_iterator` | ❗ attention | Compatible only with subcollections, 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 | Move from collection conflicts collection elements ownership semantic | -| `std::counted_iterator` | ✔️ yes | | +| `std::counted_iterator` | ❌ no | `iterator` and `const_iterator` not `std::input_or_output_iterator` ## Collection and standard algorithms diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index 0357d2b40..a6b17d968 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #if __cplusplus >= 202002L @@ -517,7 +518,7 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { // STATIC_REQUIRE(std::is_convertible_v()++), // std::add_const_t>>); - //*r++ =o + // *r++ = o REQUIREMENT_NOT_MET(traits::has_dereference_assignment_increment_v); REQUIREMENT_NOT_MET( traits::has_dereference_assignment_increment_v); @@ -558,8 +559,20 @@ TEST_CASE("Collection and iterator concepts") { } TEST_CASE("Collection and std iterator adaptors", "[collection][container][adapter][std]") { - auto a = CollectionType(); - FAIL("Not yet implemented"); + auto coll = CollectionType(); + SECTION("Back inserter") { + auto it = std::back_inserter(coll); + // insert immutable to not-subcollection + REQUIRE_THROWS_AS(it = CollectionType::value_type{}, std::invalid_argument); + // insert mutable (implicit cast to immutable) to not-subcollection + REQUIRE_THROWS_AS(it = CollectionType::value_type::mutable_type{}, std::invalid_argument); + auto subColl = CollectionType{}; + subColl.setSubsetCollection(true); + auto subIt = std::back_inserter(subColl); + auto val = coll.create(); + // insert immutable to subcollection + REQUIRE_NOTHROW(subIt = val); + } } #undef REQUIREMENT_NOT_MET \ No newline at end of file From 1a63c215f1333d761f5d50c8f64eb0a5cd742932 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Tue, 14 May 2024 17:19:29 +0200 Subject: [PATCH 21/43] Return type aliases in UserDataContainer begin and end methods Co-authored-by: hegner --- include/podio/UserDataCollection.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/podio/UserDataCollection.h b/include/podio/UserDataCollection.h index ce1f3bba3..866af99be 100644 --- a/include/podio/UserDataCollection.h +++ b/include/podio/UserDataCollection.h @@ -205,22 +205,22 @@ class UserDataCollection : public CollectionBase { // ----- some wrappers for std::vector and access to the complete std::vector (if really needed) - typename std::vector::iterator begin() { + iterator begin() { return _vec.begin(); } - typename std::vector::iterator end() { + iterator end() { return _vec.end(); } - typename std::vector::const_iterator begin() const { + const_iterator begin() const { return _vec.begin(); } - typename std::vector::const_iterator end() const { + const_iterator end() const { return _vec.end(); } - typename std::vector::const_iterator cbegin() const { + const_iterator cbegin() const { return _vec.cbegin(); } - typename std::vector::const_iterator cend() const { + const_iterator cend() const { return _vec.cend(); } From 55e77e51960ea4df5f1e021c02b97c82bb02c7a3 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Tue, 14 May 2024 17:21:28 +0200 Subject: [PATCH 22/43] Don't use decltype in UserDataCollection type aliases Co-authored-by: tmadlener --- include/podio/UserDataCollection.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/podio/UserDataCollection.h b/include/podio/UserDataCollection.h index 866af99be..35407cddb 100644 --- a/include/podio/UserDataCollection.h +++ b/include/podio/UserDataCollection.h @@ -77,11 +77,11 @@ class UserDataCollection : public CollectionBase { VectorMembersInfo m_vecmem_info{}; public: - using value_type = typename decltype(_vec)::value_type; - using const_iterator = typename decltype(_vec)::const_iterator; - using iterator = typename decltype(_vec)::iterator; - using difference_type = typename decltype(_vec)::difference_type; - using size_type = typename decltype(_vec)::size_type; + using value_type = typename std::vector::value_type; + using const_iterator = typename std::vector::const_iterator; + using iterator = typename std::vector::iterator; + using difference_type = typename std::vector::difference_type; + using size_type = typename std::vector::size_type; UserDataCollection() = default; /// Constructor from an existing vector (which will be moved from!) From b6ab5d3d6201f9933613c3d87ee2f53901441817 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Tue, 21 May 2024 22:26:35 +0200 Subject: [PATCH 23/43] reorder sections, fix consistency and typos --- doc/collections_as_container.md | 36 +++++++++++++++++---------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 823d9f823..b3320e354 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -38,8 +38,26 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | `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* + +The C++ standard specifies [AllocatorAwareContainer](https://en.cppreference.com/w/cpp/named_req/AllocatorAwareContainer) for containers that can use other allocators beside the default allocator. + +PODIO collections don't provide customization point for allocators and use only the default allocator. Therefore they are not *AllocatorAwareContainers*. + +### AllocatorAwareContainer types + +| Name | Requirements | Fulfilled by Collection? | Comment | +|------|--------------|--------------------------|---------| +| `allocator_type` | `allocator_type::value_type` same as `value_type` | ❌ no | `allocator_type` not defined | + +### *AllocatorAwareContainer* expression and statements + +The PODIO Collections currently are not checked against expression and statements requirements for *AllocatorAwareContainer*. + ## Collection iterators as an *Iterator* +The C++ specifies a set of named requirements for iterators. Starting with C++20 the standard specifies also iterator concepts. The requirements imposed by the concepts and named requirements are similar but not identical. + ### Iterator summary | Named requirement | `iterator` | `const_iterator` | @@ -127,28 +145,12 @@ The PODIO `Collection`s are move-only classes with emphasis on the distinction b | `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 | -## Collection as AllocatorAwareContainer - -The C++ standard specifies [AllocatorAwareContainer](https://en.cppreference.com/w/cpp/named_req/AllocatorAwareContainer) for containers that can use other allocators beside the default allocator. - -PODIO collections don't provide customization point for allocators and use only the default allocator. Therefore they are not *AllocatorAwareContainers*. - -### AllocatorAwareContainer types - -| Name | Requirements | Fulfilled by Collection? | Comment | -|------|--------------|--------------------------|---------| -| `allocator_type` | `allocator_type::value_type` same as `value_type` | ❌ no | `allocator_type` not defined | - -### AllocatorAwareContainer expression and statements - -The PODIO Collections currently are not checked against expression and statements requirements for *AllocatorAwareContainer*. - ## Collection iterators and standard iterator adaptors | Adaptor | Compatible with Collection? | Comment | |---------|-----------------------------|---------| | `std::reverse_iterator` | ❌ no | `iterator` and `const_iterator` not *LegacyBidirectionalIterator* or `std::bidirectional_iterator` | -| `std::back_insert_iterator` | ❗ attention | Compatible only with subcollections, otherwise throws `std::invalid_argument` | +| `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` | From 04ea18a2dd0cc45488eea22bd0c0a0228d8c8165 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Tue, 21 May 2024 23:49:45 +0200 Subject: [PATCH 24/43] rename check macro, check dereference assignment, duplicate tests for const_iterator --- doc/collections_as_container.md | 2 +- tests/unittests/std_interoperability.cpp | 367 ++++++++++++++++------- 2 files changed, 264 insertions(+), 105 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index b3320e354..b3ef3432b 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -140,7 +140,7 @@ The C++ specifies a set of named requirements for iterators. Starting with C++20 | Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment | |------------|-------------|-----------|-------------------------------------------|---------| -| `*r = o` | | | ✔️ yes / ❌ no | `iterator` defines assigning `value_type::mutable_type`, `const_iterator` doesn't define assignment | +| `*r = o` | | | ❌ no / ❌ no | 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 | diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index a6b17d968..f15b0bc1e 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -1,3 +1,4 @@ +#include "datamodel/ExampleHit.h" #include "datamodel/ExampleHitCollection.h" #include "datamodel/MutableExampleHit.h" #include @@ -7,13 +8,15 @@ #include #include -#if __cplusplus >= 202002L - #include -#endif - -// Some of the tests check . If the check annotated with 'REQUIREMENT_NOT_MET' fails, update documentation -// 'doc/collection_as_container.md' -#define REQUIREMENT_NOT_MET(...) STATIC_REQUIRE_FALSE(__VA_ARGS__) +// DOCUMENTED_STATIC_FAILURE and DOCUMENTED_FAILURE macro are used to indicate checks that corresponds to requirements +// imposed but the standard but currently not met by podio. These check use inverted logic (they pass when the +// requirement is not met) in order to detect when the support for the requirement is added. +// On their failure: +// - replace DOCUMENTED_STATIC_FAILURE with STATIC_REQUIRE_FALSE or DOCUMENTED_FAILURE with REQUIRE_FALSE +// - uncomment checks below it +// - update documentation 'doc/collection_as_container.md' +#define DOCUMENTED_STATIC_FAILURE(...) STATIC_REQUIRE_FALSE(__VA_ARGS__) +#define DOCUMENTED_FAILURE(...) REQUIRE_FALSE(__VA_ARGS__) using CollectionType = ExampleHitCollection; @@ -231,28 +234,26 @@ TEST_CASE("Collection container types", "[collection][container][types][std]") { // value_type STATIC_REQUIRE(traits::has_value_type_v); + // Erasable -allocator aware - mutually exclusive with Erasable -allocator not aware - REQUIREMENT_NOT_MET(traits::has_allocator_type_v); + DOCUMENTED_STATIC_FAILURE(traits::has_allocator_type_v); // add check for `std::allocator_traits::destroy(m, p);` expression here // STATIC_REQUIRE(...) // Erasable -allocator not aware - mutually exclusive // Erasable -allocator aware STATIC_REQUIRE(traits::is_erasable_allocator_unaware_v); // reference - REQUIREMENT_NOT_MET(traits::has_reference_v); + DOCUMENTED_STATIC_FAILURE(traits::has_reference_v); // STATIC_REQUIRE(std::is_same_v>); // const_reference - REQUIREMENT_NOT_MET(traits::has_const_reference_v); // The check will fail once the support is added. - // In that case replace it with STATIC_REQUIRE, - // uncomment checks immediately below, and update - // documentation + DOCUMENTED_STATIC_FAILURE(traits::has_const_reference_v); // STATIC_REQUIRE(std::is_same_v>>); // iterator STATIC_REQUIRE(traits::has_iterator_v); - REQUIREMENT_NOT_MET(std::is_convertible_v); + DOCUMENTED_STATIC_FAILURE(std::is_convertible_v); // const_iterator STATIC_REQUIRE(traits::has_const_iterator_v); @@ -261,11 +262,11 @@ TEST_CASE("Collection container types", "[collection][container][types][std]") { STATIC_REQUIRE(traits::has_difference_type_v); STATIC_REQUIRE(std::is_signed_v); STATIC_REQUIRE(std::is_integral_v); - REQUIREMENT_NOT_MET(traits::has_difference_type_v>); + DOCUMENTED_STATIC_FAILURE(traits::has_difference_type_v>); // STATIC_REQUIRE( // std::is_same_v::difference_type>); - REQUIREMENT_NOT_MET(traits::has_difference_type_v>); + DOCUMENTED_STATIC_FAILURE(traits::has_difference_type_v>); // STATIC_REQUIRE(std::is_same_v::difference_type>); @@ -283,13 +284,13 @@ TEST_CASE("Collection container members", "[collection][container][members][std] REQUIRE(CollectionType().empty() == true); // C(a) - REQUIREMENT_NOT_MET(std::is_copy_constructible_v); + DOCUMENTED_STATIC_FAILURE(std::is_copy_constructible_v); // C(rv) STATIC_REQUIRE(std::is_move_constructible_v); // a = b - REQUIREMENT_NOT_MET(std::is_copy_assignable_v); + DOCUMENTED_STATIC_FAILURE(std::is_copy_assignable_v); // a = rv STATIC_REQUIRE(std::is_move_assignable_v); @@ -316,23 +317,23 @@ TEST_CASE("Collection container members", "[collection][container][members][std] STATIC_REQUIRE(traits::has_cend_v); STATIC_REQUIRE(std::is_same_v().cend()), CollectionType::const_iterator>); - // a == b - REQUIREMENT_NOT_MET(traits::has_equality_comparator_v); - // STATIC_REQUIRE(std::is_convertible_v()==std::declval()), - // bool>); // value_type is EqualityComparable STATIC_REQUIRE(traits::has_equality_comparator_v); STATIC_REQUIRE( std::is_convertible_v< decltype(std::declval() != std::declval()), bool>); + // a == b + DOCUMENTED_STATIC_FAILURE(traits::has_equality_comparator_v); + // STATIC_REQUIRE(std::is_convertible_v()==std::declval()), + // bool>); // a != b - REQUIREMENT_NOT_MET(traits::has_inequality_comparator_v); + DOCUMENTED_STATIC_FAILURE(traits::has_inequality_comparator_v); // STATIC_REQUIRE(std::is_convertible_v()!=std::declval()), // bool>); // a.swap(b) - REQUIREMENT_NOT_MET(traits::has_swap_v); + DOCUMENTED_STATIC_FAILURE(traits::has_swap_v); // STATIC_REQUIRE( // std::is_same_v().swap(std::declval())), void>); @@ -353,14 +354,47 @@ TEST_CASE("Collection container members", "[collection][container][members][std] } TEST_CASE("Collection AllocatorAwareContainer types", "[collection][container][types][std]") { - REQUIREMENT_NOT_MET(traits::has_allocator_type_v); + DOCUMENTED_STATIC_FAILURE(traits::has_allocator_type_v); } // TODO add tests for AllocatorAwareContainer statements and expressions -TEST_CASE("Collection iterators", "[collection][container][interator][std]") { +TEST_CASE("Collection and iterator concepts") { +#if (__cplusplus >= 202002L) + SECTION("Iterator") { + using iterator = CollectionType::iterator; + DOCUMENTED_STATIC_FAILURE(std::indirectly_readable); + DOCUMENTED_STATIC_FAILURE(std::indirectly_writable); + DOCUMENTED_STATIC_FAILURE(std::weakly_incrementable); + DOCUMENTED_STATIC_FAILURE(std::incrementable); + DOCUMENTED_STATIC_FAILURE(std::input_or_output_iterator); + DOCUMENTED_STATIC_FAILURE(std::input_iterator); + DOCUMENTED_STATIC_FAILURE(std::output_iterator); + DOCUMENTED_STATIC_FAILURE(std::forward_iterator); + DOCUMENTED_STATIC_FAILURE(std::bidirectional_iterator); + DOCUMENTED_STATIC_FAILURE(std::random_access_iterator); + DOCUMENTED_STATIC_FAILURE(std::contiguous_iterator); + } + SECTION("Const iterator") { + using const_iterator = CollectionType::const_iterator; + DOCUMENTED_STATIC_FAILURE(std::indirectly_readable); + DOCUMENTED_STATIC_FAILURE(std::indirectly_writable); + DOCUMENTED_STATIC_FAILURE(std::weakly_incrementable); + DOCUMENTED_STATIC_FAILURE(std::incrementable); + DOCUMENTED_STATIC_FAILURE(std::input_or_output_iterator); + DOCUMENTED_STATIC_FAILURE(std::input_iterator); + DOCUMENTED_STATIC_FAILURE(std::output_iterator); + DOCUMENTED_STATIC_FAILURE(std::forward_iterator); + DOCUMENTED_STATIC_FAILURE(std::bidirectional_iterator); + DOCUMENTED_STATIC_FAILURE(std::random_access_iterator); + DOCUMENTED_STATIC_FAILURE(std::contiguous_iterator); + } +#endif +} + +TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { using iterator = CollectionType::iterator; using const_iterator = CollectionType::const_iterator; - + // the checks are duplicated for iterator and const_iterator as expectations on them are slightly different SECTION("LegacyForwardIterator") { SECTION("LegacyInputIterator") { @@ -368,89 +402,169 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { SECTION("LegacyIterator") { // CopyConstructible - REQUIREMENT_NOT_MET(std::is_move_constructible_v); - REQUIREMENT_NOT_MET(std::is_copy_constructible_v); + // iterator + DOCUMENTED_STATIC_FAILURE(std::is_move_constructible_v); + DOCUMENTED_STATIC_FAILURE(std::is_copy_constructible_v); + // const_iterator + DOCUMENTED_STATIC_FAILURE(std::is_move_constructible_v); + DOCUMENTED_STATIC_FAILURE(std::is_copy_constructible_v); // CopyAssignable - REQUIREMENT_NOT_MET(std::is_move_assignable_v); - REQUIREMENT_NOT_MET(std::is_copy_assignable_v); + // iterator + DOCUMENTED_STATIC_FAILURE(std::is_move_assignable_v); + DOCUMENTED_STATIC_FAILURE(std::is_copy_assignable_v); + // const_iterator + DOCUMENTED_STATIC_FAILURE(std::is_move_assignable_v); + DOCUMENTED_STATIC_FAILURE(std::is_copy_assignable_v); // Destructible + // iterator STATIC_REQUIRE(std::is_nothrow_destructible_v); + // const_iterator + STATIC_REQUIRE(std::is_nothrow_destructible_v); // Swappable - REQUIREMENT_NOT_MET(std::is_swappable_v); + // iterator + DOCUMENTED_STATIC_FAILURE(std::is_swappable_v); + // const_iterator + DOCUMENTED_STATIC_FAILURE(std::is_swappable_v); #if (__cplusplus < 202002L) // std::iterator_traits::value_type - REQUIREMENT_NOT_MET(traits::has_value_type_v>); + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v>); + // const_iterator + DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v>); #endif // std::iterator_traits::difference_type - REQUIREMENT_NOT_MET(traits::has_difference_type_v>); + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_difference_type_v>); + // const_iterator + DOCUMENTED_STATIC_FAILURE(traits::has_difference_type_v>); + // std::iterator_traits::reference - REQUIREMENT_NOT_MET(traits::has_reference_v>); + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_reference_v>); + // const_iterator + DOCUMENTED_STATIC_FAILURE(traits::has_reference_v>); + // std::iterator_traits::pointer - REQUIREMENT_NOT_MET(traits::has_pointer_v>); + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_pointer_v>); + // const_iterator + DOCUMENTED_STATIC_FAILURE(traits::has_pointer_v>); + // std::iterator_traits::iterator_category - REQUIREMENT_NOT_MET(traits::has_iterator_category_v>); + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_iterator_category_v>); + // const_iterator + DOCUMENTED_STATIC_FAILURE(traits::has_iterator_category_v>); + // *r - STATIC_REQUIRE(!std::is_same_v())>); + // iterator + STATIC_REQUIRE(!std::is_same_v())>); + // const_iterator + STATIC_REQUIRE(!std::is_same_v())>); + // ++r + // iterator STATIC_REQUIRE(traits::has_preincrement_v); - STATIC_REQUIRE(std::is_same_v()), - std::add_lvalue_reference_t>); + STATIC_REQUIRE(std::is_same_v()), std::add_lvalue_reference_t>); + // const_iterator + STATIC_REQUIRE(traits::has_preincrement_v); + STATIC_REQUIRE( + std::is_same_v()), std::add_lvalue_reference_t>); } // EqualityComparable + // iterator STATIC_REQUIRE(traits::has_equality_comparator_v); STATIC_REQUIRE(std::is_convertible_v() != std::declval()), bool>); + // const_iterator + STATIC_REQUIRE(traits::has_equality_comparator_v); + STATIC_REQUIRE( + std::is_convertible_v() != std::declval()), bool>); // i != j + // iterator STATIC_REQUIRE(traits::has_inequality_comparator_v); STATIC_REQUIRE(std::is_constructible_v() != std::declval())>); + // const_ iterator + STATIC_REQUIRE(traits::has_inequality_comparator_v); + STATIC_REQUIRE( + std::is_constructible_v() != std::declval())>); // *i - REQUIREMENT_NOT_MET(traits::has_reference_v); + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_reference_v); // STATIC_REQUIRE(!std::is_same_v::reference, - // decltype(*std::declval())>); - REQUIREMENT_NOT_MET(traits::has_value_type_v); - // STATIC_REQUIRE(!std::is_convertible_v()), + // decltype(*std::declval())>); + DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v); + // STATIC_REQUIRE(!std::is_convertible_v()), // std::iterator_traits::value_type>); + // const_iterator + DOCUMENTED_STATIC_FAILURE(traits::has_reference_v); + // STATIC_REQUIRE(!std::is_same_v::reference, + // decltype(*std::declval())>); + DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v); + // STATIC_REQUIRE(!std::is_convertible_v()), + // std::iterator_traits::value_type>); // i->m + // iterator STATIC_REQUIRE( std::is_same_v()->energy()), decltype((*std::declval()).energy())>); + // const_iterator + STATIC_REQUIRE(std::is_same_v()->energy()), + decltype((*std::declval()).energy())>); // ++r + // iterator STATIC_REQUIRE(traits::has_preincrement_v); - STATIC_REQUIRE(std::is_same_v()), - std::add_lvalue_reference_t>); + STATIC_REQUIRE(std::is_same_v()), std::add_lvalue_reference_t>); + // const_iterator + STATIC_REQUIRE(traits::has_preincrement_v); + STATIC_REQUIRE( + std::is_same_v()), std::add_lvalue_reference_t>); // (void)r++ - REQUIREMENT_NOT_MET(traits::has_postincrement_v); + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); STATIC_REQUIRE(traits::has_preincrement_v); - REQUIREMENT_NOT_MET(traits::has_value_type_v>); + DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v>); // STATIC_REQUIRE(std::is_same_v()), // decltype((void)std::declval()++)>); + // const_iterator + DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); + STATIC_REQUIRE(traits::has_preincrement_v); + DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v>); + // STATIC_REQUIRE(std::is_same_v()), + // decltype((void)std::declval()++)>); //*r++ - REQUIREMENT_NOT_MET(traits::has_postincrement_v); - REQUIREMENT_NOT_MET(traits::has_value_type_v>); + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); + DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v>); // STATIC_REQUIRE(std::is_convertible_v < decltype(*std::declval()++), // std::iterator_traits::value_type >>); + // const_iterator + DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); + DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v>); + // STATIC_REQUIRE(std::is_convertible_v < decltype(*std::declval()++), + // std::iterator_traits::value_type >>); } // Mutable iterator: reference same as value_type& or value_type&& - REQUIREMENT_NOT_MET(traits::has_reference_v); - REQUIREMENT_NOT_MET(traits::has_value_type_v); + DOCUMENTED_STATIC_FAILURE(traits::has_reference_v); + DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v); // STATIC_REQUIRE(std::is_same_v::reference, // std::add_lvalue_reference_t::value_type>> || // std::is_same_v::reference, // std::add_rvalue_reference_t::value_type>>); // Immutable iterator: reference same as const value_type& or const value_type&& - REQUIREMENT_NOT_MET(traits::has_reference_v); - REQUIREMENT_NOT_MET(traits::has_value_type_v); + DOCUMENTED_STATIC_FAILURE(traits::has_reference_v); + DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v); // STATIC_REQUIRE(std::is_same_v::reference, // std::add_const_t::value_type>>> // || @@ -458,121 +572,166 @@ TEST_CASE("Collection iterators", "[collection][container][interator][std]") { // std::add_const_t::value_type>>>); // DefaultConstructible - REQUIREMENT_NOT_MET(std::is_default_constructible_v); + // iterator + DOCUMENTED_STATIC_FAILURE(std::is_default_constructible_v); + // const_iterator + DOCUMENTED_STATIC_FAILURE(std::is_default_constructible_v); } // Multipass guarantee + // iterator { CollectionType coll; for (int i = 0; i < 3; ++i) { coll.create(); } + // iterator auto a = coll.begin(); auto b = coll.begin(); REQUIRE(a == b); REQUIRE(*a == *b); REQUIRE(++a == ++b); REQUIRE(*a == *b); - REQUIREMENT_NOT_MET(std::is_copy_constructible_v); + DOCUMENTED_STATIC_FAILURE(std::is_copy_constructible_v); // auto a_copy = a; // ++a_copy; // REQUIRE(a == b); // REQUIRE(*a == *b); + + // const_iterator + auto ca = coll.cbegin(); + auto cb = coll.cbegin(); + REQUIRE(ca == cb); + REQUIRE(*ca == *cb); + REQUIRE(++ca == ++cb); + REQUIRE(*ca == *cb); + DOCUMENTED_STATIC_FAILURE(std::is_copy_constructible_v); + // auto ca_copy = ca; + // ++ca_copy; + // REQUIRE(ca == cb); + // REQUIRE(*ca == *cb); } // Singular iterators + // iterator STATIC_REQUIRE(traits::has_equality_comparator_v); - REQUIREMENT_NOT_MET(std::is_default_constructible_v); + DOCUMENTED_STATIC_FAILURE(std::is_default_constructible_v); //{ - // CollectionType some_collection{}; - // REQUIRE(iterator{} == some_collection.end()); // REQUIRE(iterator{} == iterator{}); //} + // const_iterator + STATIC_REQUIRE(traits::has_equality_comparator_v); + DOCUMENTED_STATIC_FAILURE(std::is_default_constructible_v); + //{ + // REQUIRE(const_iterator{} == const_iterator{}); + //} // i++ - REQUIREMENT_NOT_MET(traits::has_postincrement_v); - // STATIC_REQUIRE(std::is_same_v()++), CollectionType::iterator>); + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); + // STATIC_REQUIRE(std::is_same_v()++), iterator>); + // const_iterator + DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); + // STATIC_REQUIRE(std::is_same_v()++), const_iterator>); // *i++ - REQUIREMENT_NOT_MET(traits::has_postincrement_v); - REQUIREMENT_NOT_MET(traits::has_reference_v>); + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); + DOCUMENTED_STATIC_FAILURE(traits::has_reference_v>); // STATIC_REQUIRE(std::is_same_v < decltype(*std::declval()++), // std::iterator_traits::reference >>); + // const_iterator + DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); + DOCUMENTED_STATIC_FAILURE(traits::has_reference_v>); + // STATIC_REQUIRE(std::is_same_v < decltype(*std::declval()++), + // std::iterator_traits::reference >>); SECTION("LegacyOutputIterator") { // is class type or pointer type + // iterator STATIC_REQUIRE(std::is_pointer_v || std::is_class_v); + // const_iterator STATIC_REQUIRE(std::is_pointer_v || std::is_class_v); // *r = o - REQUIREMENT_NOT_MET(traits::has_dereference_assignment_v); + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_dereference_assignment_v); STATIC_REQUIRE(traits::has_dereference_assignment_v); + { + auto coll = CollectionType{}; + auto item = coll.create(13ull, 0., 0., 0., 0.); + REQUIRE(coll.begin()->cellID() == 13ull); + auto new_item = CollectionType::value_type::mutable_type{42ull, 0., 0., 0., 0.}; + *coll.begin() = new_item; + DOCUMENTED_FAILURE(coll.begin()->cellID() == 42ull); + } + // const_iterator + STATIC_REQUIRE(traits::has_dereference_assignment_v); + STATIC_REQUIRE(traits::has_dereference_assignment_v); + { + auto coll = CollectionType{}; + auto item = coll.create(13ull, 0., 0., 0., 0.); + REQUIRE(coll.cbegin()->cellID() == 13ull); + auto new_item = CollectionType::value_type::mutable_type{42ull, 0., 0., 0., 0.}; + *coll.cbegin() = new_item; + DOCUMENTED_FAILURE(coll.cbegin()->cellID() == 42ull); + new_item.cellID(44ull); + *coll.cbegin() = static_cast(new_item); + DOCUMENTED_FAILURE(coll.cbegin()->cellID() == 44ull); + } // ++r + // iterator STATIC_REQUIRE(traits::has_preincrement_v); STATIC_REQUIRE(std::is_same_v()), std::add_lvalue_reference_t>); + // const_iterator + STATIC_REQUIRE(traits::has_preincrement_v); + STATIC_REQUIRE( + std::is_same_v()), std::add_lvalue_reference_t>); // r++ - REQUIREMENT_NOT_MET(traits::has_postincrement_v); + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); // STATIC_REQUIRE(std::is_convertible_v()++), // std::add_const_t>>); + // const_iterator + DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); + // STATIC_REQUIRE(std::is_convertible_v()++), + // std::add_const_t>>); // *r++ = o - REQUIREMENT_NOT_MET(traits::has_dereference_assignment_increment_v); - REQUIREMENT_NOT_MET( + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_dereference_assignment_increment_v); + DOCUMENTED_STATIC_FAILURE( traits::has_dereference_assignment_increment_v); + // TODO add runtime check for assignment validity like in '*r = o' case + // const_iterator + DOCUMENTED_STATIC_FAILURE( + traits::has_dereference_assignment_increment_v); + DOCUMENTED_STATIC_FAILURE( + traits::has_dereference_assignment_increment_v); + // TODO add runtime check for assignment validity like in '*r = o' case } } -TEST_CASE("Collection and iterator concepts") { -#if (__cplusplus >= 202002L) - SECTION("Iterator") { - using iterator = CollectionType::iterator; - REQUIREMENT_NOT_MET(std::indirectly_readable); - REQUIREMENT_NOT_MET(std::indirectly_writable); - REQUIREMENT_NOT_MET(std::weakly_incrementable); - REQUIREMENT_NOT_MET(std::incrementable); - REQUIREMENT_NOT_MET(std::input_or_output_iterator); - REQUIREMENT_NOT_MET(std::input_iterator); - REQUIREMENT_NOT_MET(std::output_iterator); - REQUIREMENT_NOT_MET(std::forward_iterator); - REQUIREMENT_NOT_MET(std::bidirectional_iterator); - REQUIREMENT_NOT_MET(std::random_access_iterator); - REQUIREMENT_NOT_MET(std::contiguous_iterator); - } - SECTION("Const iterator") { - using const_iterator = CollectionType::const_iterator; - REQUIREMENT_NOT_MET(std::indirectly_readable); - REQUIREMENT_NOT_MET(std::indirectly_writable); - REQUIREMENT_NOT_MET(std::weakly_incrementable); - REQUIREMENT_NOT_MET(std::incrementable); - REQUIREMENT_NOT_MET(std::input_or_output_iterator); - REQUIREMENT_NOT_MET(std::input_iterator); - REQUIREMENT_NOT_MET(std::output_iterator); - REQUIREMENT_NOT_MET(std::forward_iterator); - REQUIREMENT_NOT_MET(std::bidirectional_iterator); - REQUIREMENT_NOT_MET(std::random_access_iterator); - REQUIREMENT_NOT_MET(std::contiguous_iterator); - } -#endif -} - TEST_CASE("Collection and std iterator adaptors", "[collection][container][adapter][std]") { auto coll = CollectionType(); + SECTION("Back inserter") { auto it = std::back_inserter(coll); - // insert immutable to not-subcollection + // insert immutable to not-SubsetCollection REQUIRE_THROWS_AS(it = CollectionType::value_type{}, std::invalid_argument); - // insert mutable (implicit cast to immutable) to not-subcollection + // insert mutable (implicit cast to immutable) to not-SubsetCollection REQUIRE_THROWS_AS(it = CollectionType::value_type::mutable_type{}, std::invalid_argument); auto subColl = CollectionType{}; subColl.setSubsetCollection(true); auto subIt = std::back_inserter(subColl); auto val = coll.create(); - // insert immutable to subcollection + // insert immutable to SubsetCollection REQUIRE_NOTHROW(subIt = val); } } -#undef REQUIREMENT_NOT_MET \ No newline at end of file +#undef DOCUMENTED_STATIC_FAILURE +#undef DOCUMENTED_FAILURE \ No newline at end of file From f79ac291de8ffe04d728813250d2d750d3be9b36 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Wed, 22 May 2024 00:25:02 +0200 Subject: [PATCH 25/43] fix commented test, add some comments, use static_require_false instead of ! (as suggested in catch2 docs) --- tests/unittests/std_interoperability.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index f15b0bc1e..f060e84f4 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -430,7 +430,7 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { DOCUMENTED_STATIC_FAILURE(std::is_swappable_v); #if (__cplusplus < 202002L) - // std::iterator_traits::value_type + // std::iterator_traits::value_type (required prior to C++20) // iterator DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v>); // const_iterator @@ -462,9 +462,9 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { // *r // iterator - STATIC_REQUIRE(!std::is_same_v())>); + STATIC_REQUIRE_FALSE(std::is_same_v())>); // const_iterator - STATIC_REQUIRE(!std::is_same_v())>); + STATIC_REQUIRE_FALSE(std::is_same_v())>); // ++r // iterator @@ -485,7 +485,7 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { STATIC_REQUIRE( std::is_convertible_v() != std::declval()), bool>); - // i != j + // i != j (contextually convertible) // iterator STATIC_REQUIRE(traits::has_inequality_comparator_v); STATIC_REQUIRE(std::is_constructible_v() != std::declval())>); @@ -497,17 +497,17 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { // *i // iterator DOCUMENTED_STATIC_FAILURE(traits::has_reference_v); - // STATIC_REQUIRE(!std::is_same_v::reference, + // STATIC_REQUIRE(std::is_same_v::reference, // decltype(*std::declval())>); DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v); - // STATIC_REQUIRE(!std::is_convertible_v()), + // STATIC_REQUIRE(std::is_convertible_v()), // std::iterator_traits::value_type>); // const_iterator DOCUMENTED_STATIC_FAILURE(traits::has_reference_v); - // STATIC_REQUIRE(!std::is_same_v::reference, + // STATIC_REQUIRE(std::is_same_v::reference, // decltype(*std::declval())>); DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v); - // STATIC_REQUIRE(!std::is_convertible_v()), + // STATIC_REQUIRE(std::is_convertible_v()), // std::iterator_traits::value_type>); // i->m From 4c1afbbdce6a2404dcc3656f72760a4680bf70fb Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Wed, 22 May 2024 00:32:37 +0200 Subject: [PATCH 26/43] don't create separate target for tests --- tests/unittests/CMakeLists.txt | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/unittests/CMakeLists.txt b/tests/unittests/CMakeLists.txt index 498fe63be..0cb3a7214 100644 --- a/tests/unittests/CMakeLists.txt +++ b/tests/unittests/CMakeLists.txt @@ -40,17 +40,12 @@ if(NOT Catch2_FOUND) endif() find_package(Threads REQUIRED) -add_executable(unittest_podio unittest.cpp frame.cpp buffer_factory.cpp interface_types.cpp) +add_executable(unittest_podio unittest.cpp frame.cpp buffer_factory.cpp interface_types.cpp std_interoperability.cpp) target_link_libraries(unittest_podio PUBLIC TestDataModel PRIVATE Catch2::Catch2WithMain Threads::Threads podio::podioRootIO) if (ENABLE_SIO) target_link_libraries(unittest_podio PRIVATE podio::podioSioIO) endif() -add_executable(interoperability_test std_interoperability.cpp) -target_link_libraries(interoperability_test PUBLIC TestDataModel PRIVATE Catch2::Catch2WithMain) -# target_compile_definitions(interoperability_test PUBLIC "CATCH_CONFIG_RUNTIME_STATIC_REQUIRE") - target_compile_features(interoperability_test PUBLIC cxx_std_20) - # The unittests can easily be filtered and they are labelled so we can put together a # list of labels that we want to ignore set(filter_tests "") @@ -89,5 +84,4 @@ else() ENVIRONMENT LD_LIBRARY_PATH=${CMAKE_CURRENT_BINARY_DIR}:${PROJECT_BINARY_DIR}/src:${PROJECT_BINARY_DIR}/tests:$:$<$:$>:$ENV{LD_LIBRARY_PATH} ) - catch_discover_tests(interoperability_test) endif() From b844c09aaa97da929d7c3621806f56eb95136f92 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Wed, 22 May 2024 09:48:09 +0200 Subject: [PATCH 27/43] add comments, reorganize test sections --- doc/collections_as_container.md | 4 +- tests/unittests/std_interoperability.cpp | 139 ++++++++++++----------- 2 files changed, 76 insertions(+), 67 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index b3ef3432b..8f80a8c7e 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -2,7 +2,7 @@ Comparison of the PODIO `Collection`s with a C++ named requirement [*Container*](https://en.cppreference.com/w/cpp/named_req/Container). -The PODIO `Collection`s are move-only classes with emphasis on the distinction between mutable and immutable access to the elements. +The PODIO `Collection`s interface was designed to remind the standard *Container* interface, in particular `std::vector`. The perfect compliance with the *Container* is not achieved as the `Collection`s are concerned with additional semantics such as mutable/immutable element access, associations and relations, and IO which are not part of *Container*. ### Container Types @@ -140,7 +140,7 @@ The C++ specifies a set of named requirements for iterators. Starting with C++20 | Expression | Return type | Semantics | Fulfilled by `iterator`/`const_iterator`? | Comment | |------------|-------------|-----------|-------------------------------------------|---------| -| `*r = o` | | | ❌ no / ❌ no | Assignment doesn't modify objects inside collection | +| `*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 | diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index f060e84f4..46f46aa67 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -395,10 +395,14 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { using iterator = CollectionType::iterator; using const_iterator = CollectionType::const_iterator; // the checks are duplicated for iterator and const_iterator as expectations on them are slightly different + + // nested sections as the requirements make a hierarchy SECTION("LegacyForwardIterator") { + // LegacyForwardIterator requires LegacyInputIterator SECTION("LegacyInputIterator") { + // LegacyInputIterator requires LegacyIterator SECTION("LegacyIterator") { // CopyConstructible @@ -474,7 +478,8 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { STATIC_REQUIRE(traits::has_preincrement_v); STATIC_REQUIRE( std::is_same_v()), std::add_lvalue_reference_t>); - } + + } // end of LegacyIterator // EqualityComparable // iterator @@ -552,7 +557,8 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v>); // STATIC_REQUIRE(std::is_convertible_v < decltype(*std::declval()++), // std::iterator_traits::value_type >>); - } + + } // end of LegacyInputIterator // Mutable iterator: reference same as value_type& or value_type&& DOCUMENTED_STATIC_FAILURE(traits::has_reference_v); @@ -576,75 +582,76 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { DOCUMENTED_STATIC_FAILURE(std::is_default_constructible_v); // const_iterator DOCUMENTED_STATIC_FAILURE(std::is_default_constructible_v); - } - // Multipass guarantee - // iterator - { - CollectionType coll; - for (int i = 0; i < 3; ++i) { - coll.create(); - } + // Multipass guarantee // iterator - auto a = coll.begin(); - auto b = coll.begin(); - REQUIRE(a == b); - REQUIRE(*a == *b); - REQUIRE(++a == ++b); - REQUIRE(*a == *b); - DOCUMENTED_STATIC_FAILURE(std::is_copy_constructible_v); - // auto a_copy = a; - // ++a_copy; - // REQUIRE(a == b); - // REQUIRE(*a == *b); + { + CollectionType coll; + for (int i = 0; i < 3; ++i) { + coll.create(); + } + // iterator + auto a = coll.begin(); + auto b = coll.begin(); + REQUIRE(a == b); + REQUIRE(*a == *b); + REQUIRE(++a == ++b); + REQUIRE(*a == *b); + DOCUMENTED_STATIC_FAILURE(std::is_copy_constructible_v); + // auto a_copy = a; + // ++a_copy; + // REQUIRE(a == b); + // REQUIRE(*a == *b); + + // const_iterator + auto ca = coll.cbegin(); + auto cb = coll.cbegin(); + REQUIRE(ca == cb); + REQUIRE(*ca == *cb); + REQUIRE(++ca == ++cb); + REQUIRE(*ca == *cb); + DOCUMENTED_STATIC_FAILURE(std::is_copy_constructible_v); + // auto ca_copy = ca; + // ++ca_copy; + // REQUIRE(ca == cb); + // REQUIRE(*ca == *cb); + } + // Singular iterators + // iterator + STATIC_REQUIRE(traits::has_equality_comparator_v); + DOCUMENTED_STATIC_FAILURE(std::is_default_constructible_v); + //{ + // REQUIRE(iterator{} == iterator{}); + //} // const_iterator - auto ca = coll.cbegin(); - auto cb = coll.cbegin(); - REQUIRE(ca == cb); - REQUIRE(*ca == *cb); - REQUIRE(++ca == ++cb); - REQUIRE(*ca == *cb); - DOCUMENTED_STATIC_FAILURE(std::is_copy_constructible_v); - // auto ca_copy = ca; - // ++ca_copy; - // REQUIRE(ca == cb); - // REQUIRE(*ca == *cb); - } + STATIC_REQUIRE(traits::has_equality_comparator_v); + DOCUMENTED_STATIC_FAILURE(std::is_default_constructible_v); + //{ + // REQUIRE(const_iterator{} == const_iterator{}); + //} - // Singular iterators - // iterator - STATIC_REQUIRE(traits::has_equality_comparator_v); - DOCUMENTED_STATIC_FAILURE(std::is_default_constructible_v); - //{ - // REQUIRE(iterator{} == iterator{}); - //} - // const_iterator - STATIC_REQUIRE(traits::has_equality_comparator_v); - DOCUMENTED_STATIC_FAILURE(std::is_default_constructible_v); - //{ - // REQUIRE(const_iterator{} == const_iterator{}); - //} + // i++ + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); + // STATIC_REQUIRE(std::is_same_v()++), iterator>); + // const_iterator + DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); + // STATIC_REQUIRE(std::is_same_v()++), const_iterator>); - // i++ - // iterator - DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); - // STATIC_REQUIRE(std::is_same_v()++), iterator>); - // const_iterator - DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); - // STATIC_REQUIRE(std::is_same_v()++), const_iterator>); + // *i++ + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); + DOCUMENTED_STATIC_FAILURE(traits::has_reference_v>); + // STATIC_REQUIRE(std::is_same_v < decltype(*std::declval()++), + // std::iterator_traits::reference >>); + // const_iterator + DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); + DOCUMENTED_STATIC_FAILURE(traits::has_reference_v>); + // STATIC_REQUIRE(std::is_same_v < decltype(*std::declval()++), + // std::iterator_traits::reference >>); - // *i++ - // iterator - DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); - DOCUMENTED_STATIC_FAILURE(traits::has_reference_v>); - // STATIC_REQUIRE(std::is_same_v < decltype(*std::declval()++), - // std::iterator_traits::reference >>); - // const_iterator - DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); - DOCUMENTED_STATIC_FAILURE(traits::has_reference_v>); - // STATIC_REQUIRE(std::is_same_v < decltype(*std::declval()++), - // std::iterator_traits::reference >>); + } // end of LegacyForwardIterator SECTION("LegacyOutputIterator") { @@ -712,7 +719,9 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { DOCUMENTED_STATIC_FAILURE( traits::has_dereference_assignment_increment_v); // TODO add runtime check for assignment validity like in '*r = o' case - } + + } // end of LegacyOutputIterator + } TEST_CASE("Collection and std iterator adaptors", "[collection][container][adapter][std]") { From e3356b771e34cf12a4dc833f7be5cdc5501c64fb Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Wed, 22 May 2024 09:56:36 +0200 Subject: [PATCH 28/43] fix pre-commit --- tests/unittests/std_interoperability.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index 46f46aa67..719c412d8 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -721,7 +721,6 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { // TODO add runtime check for assignment validity like in '*r = o' case } // end of LegacyOutputIterator - } TEST_CASE("Collection and std iterator adaptors", "[collection][container][adapter][std]") { From 04db87fea1e2f42aa5e0dd7049e38718d237c2b5 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Fri, 24 May 2024 21:04:10 +0200 Subject: [PATCH 29/43] fix iterators are not swappable --- doc/collections_as_container.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 8f80a8c7e..3a216881f 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -88,7 +88,7 @@ The C++ specifies a set of named requirements for iterators. Starting with C++20 | [*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 | | [*Destructible*](https://en.cppreference.com/w/cpp/named_req/Destructible) | ✔️ yes / ✔️ yes | | -| [*Swappable*](https://en.cppreference.com/w/cpp/named_req/Swappable) | ✔️ 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 | From 4975d44779f3a10900ffc3d8dfd43c9999b57d45 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Fri, 24 May 2024 23:02:39 +0200 Subject: [PATCH 30/43] fix not needed requirement, fix typo --- doc/collections_as_container.md | 2 +- tests/unittests/std_interoperability.cpp | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 3a216881f..36fc278ac 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -129,7 +129,7 @@ The C++ specifies a set of named requirements for iterators. Starting with C++20 | 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++` | `reference` | | ❌ no / ❌ no | Post-increment and `reference` not defined | ### LegacyOutputIterator diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index 719c412d8..2576edab6 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -534,15 +534,13 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { // (void)r++ // iterator - DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); STATIC_REQUIRE(traits::has_preincrement_v); - DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v>); + DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); // STATIC_REQUIRE(std::is_same_v()), // decltype((void)std::declval()++)>); // const_iterator - DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); STATIC_REQUIRE(traits::has_preincrement_v); - DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v>); + DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); // STATIC_REQUIRE(std::is_same_v()), // decltype((void)std::declval()++)>); From 0f733e24e75491541f005cbc43f42062d3af4177 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Sat, 25 May 2024 16:33:51 +0200 Subject: [PATCH 31/43] fix typo --- doc/collections_as_container.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 36fc278ac..8664c820c 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -155,7 +155,7 @@ The C++ specifies a set of named requirements for iterators. Starting with C++20 | `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 | Move from collection conflicts collection elements ownership semantic | -| `std::counted_iterator` | ❌ no | `iterator` and `const_iterator` not `std::input_or_output_iterator` +| `std::counted_iterator` | ❌ no | `iterator` and `const_iterator` not `std::input_or_output_iterator` | ## Collection and standard algorithms From 3fdf08b75bb9ef860eb4b891bb1ff09cf86ba4e9 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Sat, 25 May 2024 17:37:06 +0200 Subject: [PATCH 32/43] fix cv and ref qualifiers, add traits for * and -> --- tests/unittests/std_interoperability.cpp | 122 +++++++++++++---------- 1 file changed, 71 insertions(+), 51 deletions(-) diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index 2576edab6..c8c48ee67 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -101,7 +101,7 @@ template struct is_erasable_allocator_unaware< T, std::void_t>::destroy( - std::declval>>(), + std::declval&>(), std::declval>()))>> : std::true_type {}; template inline constexpr bool is_erasable_allocator_unaware_v = is_erasable_allocator_unaware::value; @@ -194,11 +194,27 @@ struct has_empty().empty())>> : std::tru template inline constexpr bool has_empty_v = has_empty::value; +// T::operator* +template +struct has_indirection : std::false_type {}; +template +struct has_indirection())>> : std::true_type {}; +template +constexpr bool has_indirection_v = has_indirection::value; + +// T::operator-> +template +struct has_member_of_pointer : std::false_type {}; +template +struct has_member_of_pointer().operator->())>> : std::true_type {}; +template +constexpr bool has_member_of_pointer_v = has_member_of_pointer::value; + // T::operator++() (preincrement) template struct has_preincrement : std::false_type {}; template -struct has_preincrement())>> : std::true_type {}; +struct has_preincrement())>> : std::true_type {}; template inline constexpr bool has_preincrement_v = has_preincrement::value; @@ -206,7 +222,7 @@ inline constexpr bool has_preincrement_v = has_preincrement::value; template struct has_postincrement : std::false_type {}; template -struct has_postincrement()++)>> : std::true_type {}; +struct has_postincrement()++)>> : std::true_type {}; template inline constexpr bool has_postincrement_v = has_postincrement::value; @@ -214,7 +230,7 @@ inline constexpr bool has_postincrement_v = has_postincrement::value; template struct has_dereference_assignment : std::false_type {}; template -struct has_dereference_assignment() = std::declval())>> +struct has_dereference_assignment() = std::declval())>> : std::true_type {}; template inline constexpr bool has_dereference_assignment_v = has_dereference_assignment::value; @@ -224,7 +240,7 @@ template struct has_dereference_assignment_increment : std::false_type {}; template struct has_dereference_assignment_increment()++ = std::declval())>> + std::void_t()++ = std::declval())>> : std::true_type {}; template inline constexpr bool has_dereference_assignment_increment_v = has_dereference_assignment_increment::value; @@ -244,12 +260,11 @@ TEST_CASE("Collection container types", "[collection][container][types][std]") { // reference DOCUMENTED_STATIC_FAILURE(traits::has_reference_v); - // STATIC_REQUIRE(std::is_same_v>); + // STATIC_REQUIRE(std::is_same_v); // const_reference DOCUMENTED_STATIC_FAILURE(traits::has_const_reference_v); - // STATIC_REQUIRE(std::is_same_v>>); + // STATIC_REQUIRE(std::is_same_v); // iterator STATIC_REQUIRE(traits::has_iterator_v); @@ -374,7 +389,7 @@ TEST_CASE("Collection and iterator concepts") { DOCUMENTED_STATIC_FAILURE(std::random_access_iterator); DOCUMENTED_STATIC_FAILURE(std::contiguous_iterator); } - SECTION("Const iterator") { + SECTION("Const_iterator") { using const_iterator = CollectionType::const_iterator; DOCUMENTED_STATIC_FAILURE(std::indirectly_readable); DOCUMENTED_STATIC_FAILURE(std::indirectly_writable); @@ -429,9 +444,9 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { // Swappable // iterator - DOCUMENTED_STATIC_FAILURE(std::is_swappable_v); + DOCUMENTED_STATIC_FAILURE(std::is_swappable_v); // const_iterator - DOCUMENTED_STATIC_FAILURE(std::is_swappable_v); + DOCUMENTED_STATIC_FAILURE(std::is_swappable_v); #if (__cplusplus < 202002L) // std::iterator_traits::value_type (required prior to C++20) @@ -466,18 +481,19 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { // *r // iterator - STATIC_REQUIRE_FALSE(std::is_same_v())>); + STATIC_REQUIRE(traits::has_indirection_v); + STATIC_REQUIRE_FALSE(std::is_same_v())>); // const_iterator - STATIC_REQUIRE_FALSE(std::is_same_v())>); + STATIC_REQUIRE(traits::has_indirection_v); + STATIC_REQUIRE_FALSE(std::is_same_v())>); // ++r // iterator - STATIC_REQUIRE(traits::has_preincrement_v); - STATIC_REQUIRE(std::is_same_v()), std::add_lvalue_reference_t>); + STATIC_REQUIRE(traits::has_preincrement_v); + STATIC_REQUIRE(std::is_same_v()), iterator&>); // const_iterator - STATIC_REQUIRE(traits::has_preincrement_v); - STATIC_REQUIRE( - std::is_same_v()), std::add_lvalue_reference_t>); + STATIC_REQUIRE(traits::has_preincrement_v); + STATIC_REQUIRE(std::is_same_v()), const_iterator&>); } // end of LegacyIterator @@ -501,6 +517,7 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { // *i // iterator + STATIC_REQUIRE(traits::has_indirection_v); DOCUMENTED_STATIC_FAILURE(traits::has_reference_v); // STATIC_REQUIRE(std::is_same_v::reference, // decltype(*std::declval())>); @@ -508,6 +525,7 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { // STATIC_REQUIRE(std::is_convertible_v()), // std::iterator_traits::value_type>); // const_iterator + STATIC_REQUIRE(traits::has_indirection_v); DOCUMENTED_STATIC_FAILURE(traits::has_reference_v); // STATIC_REQUIRE(std::is_same_v::reference, // decltype(*std::declval())>); @@ -517,63 +535,66 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { // i->m // iterator + STATIC_REQUIRE(traits::has_member_of_pointer_v); + STATIC_REQUIRE(traits::has_indirection_v); STATIC_REQUIRE( std::is_same_v()->energy()), decltype((*std::declval()).energy())>); // const_iterator + STATIC_REQUIRE(traits::has_member_of_pointer_v); + STATIC_REQUIRE(traits::has_indirection_v); STATIC_REQUIRE(std::is_same_v()->energy()), decltype((*std::declval()).energy())>); // ++r // iterator STATIC_REQUIRE(traits::has_preincrement_v); - STATIC_REQUIRE(std::is_same_v()), std::add_lvalue_reference_t>); + STATIC_REQUIRE(std::is_same_v()), iterator&>); // const_iterator STATIC_REQUIRE(traits::has_preincrement_v); - STATIC_REQUIRE( - std::is_same_v()), std::add_lvalue_reference_t>); + STATIC_REQUIRE(std::is_same_v()), const_iterator&>); // (void)r++ // iterator STATIC_REQUIRE(traits::has_preincrement_v); DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); - // STATIC_REQUIRE(std::is_same_v()), - // decltype((void)std::declval()++)>); + // STATIC_REQUIRE( + // std::is_same_v()), decltype((void)std::declval()++)>); // const_iterator STATIC_REQUIRE(traits::has_preincrement_v); DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); - // STATIC_REQUIRE(std::is_same_v()), - // decltype((void)std::declval()++)>); + // STATIC_REQUIRE(std::is_same_v()), + // decltype((void)std::declval()++)>); //*r++ // iterator + STATIC_REQUIRE(traits::has_indirection_v); DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v>); - // STATIC_REQUIRE(std::is_convertible_v < decltype(*std::declval()++), - // std::iterator_traits::value_type >>); + // STATIC_REQUIRE( + // std::is_convertible_v()++), std::iterator_traits::value_type>); // const_iterator + STATIC_REQUIRE(traits::has_indirection_v); DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v>); - // STATIC_REQUIRE(std::is_convertible_v < decltype(*std::declval()++), - // std::iterator_traits::value_type >>); + // STATIC_REQUIRE(std::is_convertible_v()++), + // std::iterator_traits::value_type>); } // end of LegacyInputIterator // Mutable iterator: reference same as value_type& or value_type&& DOCUMENTED_STATIC_FAILURE(traits::has_reference_v); DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v); - // STATIC_REQUIRE(std::is_same_v::reference, - // std::add_lvalue_reference_t::value_type>> || - // std::is_same_v::reference, - // std::add_rvalue_reference_t::value_type>>); + // STATIC_REQUIRE( + // std::is_same_v::reference, std::iterator_traits::value_type&> || + // std::is_same_v::reference, std::iterator_traits::value_type&&>); // Immutable iterator: reference same as const value_type& or const value_type&& DOCUMENTED_STATIC_FAILURE(traits::has_reference_v); DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v); // STATIC_REQUIRE(std::is_same_v::reference, - // std::add_const_t::value_type>>> - // || + // const std::iterator_traits::value_type&> || // std::is_same_v::reference, - // std::add_const_t::value_type>>>); + // const std::iterator_traits::value_type&&>); // DefaultConstructible // iterator @@ -632,22 +653,24 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { // i++ // iterator DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); - // STATIC_REQUIRE(std::is_same_v()++), iterator>); + // STATIC_REQUIRE(std::is_same_v()++), iterator>); // const_iterator DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); - // STATIC_REQUIRE(std::is_same_v()++), const_iterator>); + // STATIC_REQUIRE(std::is_same_v()++), const_iterator>); // *i++ // iterator + STATIC_REQUIRE(traits::has_indirection_v); DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); DOCUMENTED_STATIC_FAILURE(traits::has_reference_v>); - // STATIC_REQUIRE(std::is_same_v < decltype(*std::declval()++), - // std::iterator_traits::reference >>); - // const_iterator + // STATIC_REQUIRE(std::is_same_v()++), + // std::iterator_traits::reference>); const_iterator + STATIC_REQUIRE(traits::has_indirection_v); DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); DOCUMENTED_STATIC_FAILURE(traits::has_reference_v>); - // STATIC_REQUIRE(std::is_same_v < decltype(*std::declval()++), - // std::iterator_traits::reference >>); + // STATIC_REQUIRE( + // std::is_same_v()++), + // std::iterator_traits::reference>); } // end of LegacyForwardIterator @@ -689,21 +712,18 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { // ++r // iterator STATIC_REQUIRE(traits::has_preincrement_v); - STATIC_REQUIRE(std::is_same_v()), std::add_lvalue_reference_t>); + STATIC_REQUIRE(std::is_same_v()), iterator&>); // const_iterator STATIC_REQUIRE(traits::has_preincrement_v); - STATIC_REQUIRE( - std::is_same_v()), std::add_lvalue_reference_t>); + STATIC_REQUIRE(std::is_same_v()), const_iterator&>); // r++ // iterator DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); - // STATIC_REQUIRE(std::is_convertible_v()++), - // std::add_const_t>>); - // const_iterator + // STATIC_REQUIRE(std::is_convertible_v()++), const iterator&>); + // const_iterator DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); - // STATIC_REQUIRE(std::is_convertible_v()++), - // std::add_const_t>>); + // STATIC_REQUIRE(std::is_convertible_v()++), const const_iterator&>); // *r++ = o // iterator From 75c1414c06dc406d5b30892c2922b11d589aae5d Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Sat, 25 May 2024 17:45:04 +0200 Subject: [PATCH 33/43] fix trait consistency --- tests/unittests/std_interoperability.cpp | 108 +++++++++++------------ 1 file changed, 54 insertions(+), 54 deletions(-) diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index c8c48ee67..c4544796b 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -23,7 +23,7 @@ using CollectionType = ExampleHitCollection; namespace traits { // typename T::value_type -template +template struct has_value_type : std::false_type {}; template struct has_value_type> : std::true_type {}; @@ -31,7 +31,7 @@ template inline constexpr bool has_value_type_v = has_value_type::value; // typename T::reference -template +template struct has_reference : std::false_type {}; template struct has_reference> : std::true_type {}; @@ -39,7 +39,7 @@ template inline constexpr bool has_reference_v = has_reference::value; // typename T::const_reference -template +template struct has_const_reference : std::false_type {}; template struct has_const_reference> : std::true_type {}; @@ -47,7 +47,7 @@ template inline constexpr bool has_const_reference_v = has_const_reference::value; // typename T::iterator -template +template struct has_iterator : std::false_type {}; template struct has_iterator> : std::true_type {}; @@ -55,7 +55,7 @@ template inline constexpr bool has_iterator_v = has_iterator::value; // typename T::const_iterator -template +template struct has_const_iterator : std::false_type {}; template struct has_const_iterator> : std::true_type {}; @@ -63,7 +63,7 @@ template inline constexpr bool has_const_iterator_v = has_const_iterator::value; // typename T::difference_type -template +template struct has_difference_type : std::false_type {}; template struct has_difference_type> : std::true_type {}; @@ -71,7 +71,7 @@ template inline constexpr bool has_difference_type_v = has_difference_type::value; // typename T::size_type -template +template struct has_size_type : std::false_type {}; template struct has_size_type> : std::true_type {}; @@ -79,7 +79,7 @@ template inline constexpr bool has_size_type_v = has_size_type::value; // typename T::pointer -template +template struct has_pointer : std::false_type {}; template struct has_pointer> : std::true_type {}; @@ -87,7 +87,7 @@ template inline constexpr bool has_pointer_v = has_pointer::value; // typename T::allocator_type -template +template struct has_allocator_type : std::false_type {}; template struct has_allocator_type> : std::true_type {}; @@ -95,9 +95,9 @@ template inline constexpr bool has_allocator_type_v = has_allocator_type::value; // is_erasable_allocator_unaware -template +template struct is_erasable_allocator_unaware : std::false_type {}; -template +template struct is_erasable_allocator_unaware< T, std::void_t>::destroy( @@ -107,138 +107,138 @@ template inline constexpr bool is_erasable_allocator_unaware_v = is_erasable_allocator_unaware::value; // typename T::iterator_category -template +template struct has_iterator_category : std::false_type {}; template struct has_iterator_category> : std::true_type {}; template inline constexpr bool has_iterator_category_v = has_iterator_category::value; -// T::begin -template +// T::begin() +template struct has_begin : std::false_type {}; -template +template struct has_begin().begin())>> : std::true_type {}; template inline constexpr bool has_begin_v = has_begin::value; -// T::end -template +// T::end() +template struct has_end : std::false_type {}; -template +template struct has_end().end())>> : std::true_type {}; template inline constexpr bool has_end_v = has_end::value; -// T::cbegin -template +// T::cbegin() +template struct has_cbegin : std::false_type {}; -template +template struct has_cbegin().cbegin())>> : std::true_type {}; template inline constexpr bool has_cbegin_v = has_cbegin::value; -// T::cend -template +// T::cend() +template struct has_cend : std::false_type {}; -template +template struct has_cend().cend())>> : std::true_type {}; template inline constexpr bool has_cend_v = has_cend::value; // T::operator==(T) -template +template struct has_equality_comparator : std::false_type {}; -template +template struct has_equality_comparator() == std::declval())>> : std::true_type {}; template inline constexpr bool has_equality_comparator_v = has_equality_comparator::value; // T::operator!=(T) -template +template struct has_inequality_comparator : std::false_type {}; -template +template struct has_inequality_comparator() != std::declval())>> : std::true_type {}; template inline constexpr bool has_inequality_comparator_v = has_inequality_comparator::value; -// T::swap -template +// T::swap(T) +template struct has_swap : std::false_type {}; -template +template struct has_swap().swap(std::declval()))>> : std::true_type {}; template inline constexpr bool has_swap_v = has_swap::value; -// T::size -template +// T::size() +template struct has_size : std::false_type {}; -template +template struct has_size().size())>> : std::true_type {}; template inline constexpr bool has_size_v = has_size::value; -// T::max_size -template +// T::max_size() +template struct has_max_size : std::false_type {}; -template +template struct has_max_size().max_size())>> : std::true_type {}; template inline constexpr bool has_max_size_v = has_max_size::value; -// T::empty -template +// T::empty() +template struct has_empty : std::false_type {}; -template +template struct has_empty().empty())>> : std::true_type {}; template inline constexpr bool has_empty_v = has_empty::value; -// T::operator* -template +// T::operator*() +template struct has_indirection : std::false_type {}; -template +template struct has_indirection())>> : std::true_type {}; template constexpr bool has_indirection_v = has_indirection::value; -// T::operator-> -template +// T::operator->() +template struct has_member_of_pointer : std::false_type {}; -template +template struct has_member_of_pointer().operator->())>> : std::true_type {}; template constexpr bool has_member_of_pointer_v = has_member_of_pointer::value; // T::operator++() (preincrement) -template +template struct has_preincrement : std::false_type {}; -template +template struct has_preincrement())>> : std::true_type {}; template inline constexpr bool has_preincrement_v = has_preincrement::value; // T::operator++(int) (postincrement) -template +template struct has_postincrement : std::false_type {}; -template +template struct has_postincrement()++)>> : std::true_type {}; template inline constexpr bool has_postincrement_v = has_postincrement::value; // *It = val -template +template struct has_dereference_assignment : std::false_type {}; -template +template struct has_dereference_assignment() = std::declval())>> : std::true_type {}; template inline constexpr bool has_dereference_assignment_v = has_dereference_assignment::value; // *It++ = val -template +template struct has_dereference_assignment_increment : std::false_type {}; -template +template struct has_dereference_assignment_increment()++ = std::declval())>> : std::true_type {}; From 107dacb33575e0fb0d11ea17c63861ab5cfece86 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Sat, 25 May 2024 18:16:47 +0200 Subject: [PATCH 34/43] add mention the rest of legacy iterators --- doc/collections_as_container.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 8664c820c..6bac69019 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -66,6 +66,9 @@ The C++ specifies a set of named requirements for iterators. Starting with C++20 | [LegacyInputIterator](https://en.cppreference.com/w/cpp/named_req/InputIterator) | ❌ no ([see below](#legacyinputiterator)) | ❌ no ([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 | +| [LegacyRandomAccessIterator](https://en.cppreference.com/w/cpp/named_req/RandomAccessIterator) | ❌ no | ❌ no | +| [LegacyContiguousIterator](https://en.cppreference.com/w/cpp/named_req/ContiguousIterator) | ❌ no | ❌ no | | Concept | `iterator` | `const_iterator` | |---------|------------------------|------------------------------| From 72109a1874c387fc14e08875dbd507ab877b3425 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Sat, 25 May 2024 19:41:29 +0200 Subject: [PATCH 35/43] add check iterator_category --- tests/unittests/std_interoperability.cpp | 32 ++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index c4544796b..df9b1eefd 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -579,6 +579,15 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { // STATIC_REQUIRE(std::is_convertible_v()++), // std::iterator_traits::value_type>); + // iterator_category - not strictly necessary but advised + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_iterator_category_v>); + // STATIC_REQUIRE(std::is_base_of_v::iterator_category>); + // const_iterator + DOCUMENTED_STATIC_FAILURE(traits::has_iterator_category_v>); + // STATIC_REQUIRE(std::is_base_of_v::iterator_category>); + } // end of LegacyInputIterator // Mutable iterator: reference same as value_type& or value_type&& @@ -672,6 +681,15 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { // std::is_same_v()++), // std::iterator_traits::reference>); + // iterator_category - not strictly necessary but advised + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_iterator_category_v>); + // STATIC_REQUIRE(std::is_base_of_v::iterator_category>); + // const_iterator + DOCUMENTED_STATIC_FAILURE(traits::has_iterator_category_v>); + // STATIC_REQUIRE(std::is_base_of_v::iterator_category>); + } // end of LegacyForwardIterator SECTION("LegacyOutputIterator") { @@ -738,6 +756,20 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { traits::has_dereference_assignment_increment_v); // TODO add runtime check for assignment validity like in '*r = o' case + // iterator_category - not strictly necessary but advised + // Derived either from: + // - std::output_iterator_tag + // - std::forward_iterator_tag (for mutable LegacyForwardIterators) + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_iterator_category_v>); + // STATIC_REQUIRE(std::is_base_of_v::iterator_category> || + // std::is_base_of_v::iterator_category>); + // const_iterator + DOCUMENTED_STATIC_FAILURE(traits::has_iterator_category_v>); + // STATIC_REQUIRE(std::is_base_of_v::iterator_category> || + // std::is_base_of_v::iterator_category>); + } // end of LegacyOutputIterator } From e25de510a8fa29ee5b0773c371ee80495578ad10 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Sat, 25 May 2024 20:08:24 +0200 Subject: [PATCH 36/43] add clarification *mutable* in std iterators vs *mutable* in podio --- doc/collections_as_container.md | 5 ++++- tests/unittests/std_interoperability.cpp | 23 +++++++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 6bac69019..090ee5d19 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -121,11 +121,14 @@ The C++ specifies a set of named requirements for iterators. Starting with C++20 ### 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 | +| 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 | diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index df9b1eefd..ab6fd35d9 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -590,14 +590,33 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { } // end of LegacyInputIterator - // Mutable iterator: reference same as value_type& or value_type&& + // Mutable LegacyForwardIterator (LegacyForwardIterator that is also LegacyOutputIterator): + // - reference same as value_type& or value_type&& + // iterator DOCUMENTED_STATIC_FAILURE(traits::has_reference_v); DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v); // STATIC_REQUIRE( // std::is_same_v::reference, std::iterator_traits::value_type&> || // std::is_same_v::reference, std::iterator_traits::value_type&&>); + // const_iterator + DOCUMENTED_STATIC_FAILURE(traits::has_reference_v); + DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v); + // STATIC_REQUIRE( + // std::is_same_v::reference, + // std::iterator_traits::value_type&> || + // std::is_same_v::reference, + // std::iterator_traits::value_type&&>); - // Immutable iterator: reference same as const value_type& or const value_type&& + // (Immutable) iterator: + // - reference same as const value_type& or const value_type&& + // iterator + DOCUMENTED_STATIC_FAILURE(traits::has_reference_v); + DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v); + // STATIC_REQUIRE(std::is_same_v::reference, + // const std::iterator_traits::value_type&> || + // std::is_same_v::reference, + // const std::iterator_traits::value_type&&>); + // const_iterator DOCUMENTED_STATIC_FAILURE(traits::has_reference_v); DOCUMENTED_STATIC_FAILURE(traits::has_value_type_v); // STATIC_REQUIRE(std::is_same_v::reference, From a7248b065c79c271b9cdffb08366661c87f14112 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Sat, 25 May 2024 21:39:39 +0200 Subject: [PATCH 37/43] fix typos --- tests/unittests/std_interoperability.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index ab6fd35d9..d1ab379ea 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -565,7 +565,7 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { // STATIC_REQUIRE(std::is_same_v()), // decltype((void)std::declval()++)>); - //*r++ + // *r++ // iterator STATIC_REQUIRE(traits::has_indirection_v); DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); @@ -585,7 +585,7 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { // STATIC_REQUIRE(std::is_base_of_v::iterator_category>); // const_iterator DOCUMENTED_STATIC_FAILURE(traits::has_iterator_category_v>); - // STATIC_REQUIRE(std::is_base_of_v::iterator_category>); } // end of LegacyInputIterator @@ -692,7 +692,8 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); DOCUMENTED_STATIC_FAILURE(traits::has_reference_v>); // STATIC_REQUIRE(std::is_same_v()++), - // std::iterator_traits::reference>); const_iterator + // std::iterator_traits::reference>); + // const_iterator STATIC_REQUIRE(traits::has_indirection_v); DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); DOCUMENTED_STATIC_FAILURE(traits::has_reference_v>); @@ -758,7 +759,7 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { // iterator DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); // STATIC_REQUIRE(std::is_convertible_v()++), const iterator&>); - // const_iterator + // const_iterator DOCUMENTED_STATIC_FAILURE(traits::has_postincrement_v); // STATIC_REQUIRE(std::is_convertible_v()++), const const_iterator&>); From eb4a4936b2847ac965b0a9ec77242ea0257da44c Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Sun, 26 May 2024 00:26:38 +0200 Subject: [PATCH 38/43] add checks for adaptors --- tests/unittests/std_interoperability.cpp | 103 +++++++++++++++++++++-- 1 file changed, 98 insertions(+), 5 deletions(-) diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index d1ab379ea..33d58277c 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -19,6 +19,8 @@ #define DOCUMENTED_FAILURE(...) REQUIRE_FALSE(__VA_ARGS__) using CollectionType = ExampleHitCollection; +using iterator = CollectionType::iterator; +using const_iterator = CollectionType::const_iterator; namespace traits { @@ -244,6 +246,34 @@ struct has_dereference_assignment_increment inline constexpr bool has_dereference_assignment_increment_v = has_dereference_assignment_increment::value; + +// T::push_back(Value) +template +struct has_push_back : std::false_type {}; +template +struct has_push_back().push_back(std::declval()))>> + : std::true_type {}; +template +inline constexpr bool has_push_back_v = has_push_back::value; + +// T::push_front(Value) +template +struct has_push_front : std::false_type {}; +template +struct has_push_front().push_front(std::declval()))>> + : std::true_type {}; +template +inline constexpr bool has_push_front_v = has_push_front::value; + +// T::insert(Value) +template +struct has_insert : std::false_type {}; +template +struct has_insert().insert(std::declval(), std::declval()))>> + : std::true_type {}; +template +inline constexpr bool has_insert_v = has_insert::value; } // namespace traits TEST_CASE("Collection container types", "[collection][container][types][std]") { @@ -376,7 +406,6 @@ TEST_CASE("Collection AllocatorAwareContainer types", "[collection][container][t TEST_CASE("Collection and iterator concepts") { #if (__cplusplus >= 202002L) SECTION("Iterator") { - using iterator = CollectionType::iterator; DOCUMENTED_STATIC_FAILURE(std::indirectly_readable); DOCUMENTED_STATIC_FAILURE(std::indirectly_writable); DOCUMENTED_STATIC_FAILURE(std::weakly_incrementable); @@ -390,7 +419,6 @@ TEST_CASE("Collection and iterator concepts") { DOCUMENTED_STATIC_FAILURE(std::contiguous_iterator); } SECTION("Const_iterator") { - using const_iterator = CollectionType::const_iterator; DOCUMENTED_STATIC_FAILURE(std::indirectly_readable); DOCUMENTED_STATIC_FAILURE(std::indirectly_writable); DOCUMENTED_STATIC_FAILURE(std::weakly_incrementable); @@ -407,8 +435,6 @@ TEST_CASE("Collection and iterator concepts") { } TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { - using iterator = CollectionType::iterator; - using const_iterator = CollectionType::const_iterator; // the checks are duplicated for iterator and const_iterator as expectations on them are slightly different // nested sections as the requirements make a hierarchy @@ -795,8 +821,33 @@ TEST_CASE("Collection iterators", "[collection][container][iterator][std]") { TEST_CASE("Collection and std iterator adaptors", "[collection][container][adapter][std]") { auto coll = CollectionType(); - + SECTION("Reverse iterator") { + // iterator + STATIC_REQUIRE(traits::has_iterator_v); + DOCUMENTED_STATIC_FAILURE(traits::has_iterator_category_v>); + // STATIC_REQUIRE( + // std::is_base_of_v::iterator_category>); +#if (__cplusplus >= 202002L) + DOCUMENTED_STATIC_FAILURE(std::bidirectional_iterator); +#endif + // const_iterator + STATIC_REQUIRE(traits::has_const_iterator_v); + DOCUMENTED_STATIC_FAILURE(traits::has_iterator_category_v>); + // STATIC_REQUIRE( + // std::is_base_of_v::iterator_category>); +#if (__cplusplus >= 202002L) + DOCUMENTED_STATIC_FAILURE(std::bidirectional_iterator); +#endif + // TODO add runtime checks here + } SECTION("Back inserter") { + DOCUMENTED_STATIC_FAILURE(traits::has_const_reference_v); + // STATIC_REQUIRE(traits::has_push_back_v); + + STATIC_REQUIRE(traits::has_value_type_v); + STATIC_REQUIRE(traits::has_push_back_v); + STATIC_REQUIRE(traits::has_push_back_v); + auto it = std::back_inserter(coll); // insert immutable to not-SubsetCollection REQUIRE_THROWS_AS(it = CollectionType::value_type{}, std::invalid_argument); @@ -806,9 +857,51 @@ TEST_CASE("Collection and std iterator adaptors", "[collection][container][adapt subColl.setSubsetCollection(true); auto subIt = std::back_inserter(subColl); auto val = coll.create(); + val.cellID(42); // insert immutable to SubsetCollection REQUIRE_NOTHROW(subIt = val); + REQUIRE(subColl.begin()->cellID() == 42); + } + + SECTION("Front inserter") { + DOCUMENTED_STATIC_FAILURE(traits::has_const_reference_v); + // STATIC_REQUIRE(traits::has_push_front_v); + + STATIC_REQUIRE(traits::has_value_type_v); + DOCUMENTED_STATIC_FAILURE(traits::has_push_front_v); + DOCUMENTED_STATIC_FAILURE(traits::has_push_front_v); + // TODO add runtime checks here } + + SECTION("Inserter") { + STATIC_REQUIRE(traits::has_iterator_v); + DOCUMENTED_STATIC_FAILURE(traits::has_const_reference_v); + // STATIC_REQUIRE( + // traits::has_insert_v); + + STATIC_REQUIRE(traits::has_value_type_v); + DOCUMENTED_STATIC_FAILURE( + traits::has_insert_v); + DOCUMENTED_STATIC_FAILURE( + traits::has_insert_v); + // TODO add runtime checks here + } + + SECTION("Const iterator") { + // C++23 required + DOCUMENTED_STATIC_FAILURE((__cplusplus >= 202302L)); + } + +#if (__cplusplus >= 202002L) + SECTION("Counted iterator") { + // iterator + DOCUMENTED_STATIC_FAILURE(std::input_or_output_iterator); + // TODO add runtime checks + // const_iterator + DOCUMENTED_STATIC_FAILURE(std::input_or_output_iterator); + // TODO add runtime checks + } +#endif } #undef DOCUMENTED_STATIC_FAILURE From b3d953d6d558b5642d8f8c5ba72316a0cb0e11b0 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Sun, 26 May 2024 02:01:11 +0200 Subject: [PATCH 39/43] add checks for move iterator adaptor --- doc/collections_as_container.md | 2 +- tests/unittests/std_interoperability.cpp | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 090ee5d19..e5f38ddf8 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -160,7 +160,7 @@ In addition to the *LegacyForwardIterator* the C++ standard specifies also the * | `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 | Move from collection conflicts collection elements ownership semantic | +| `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` | diff --git a/tests/unittests/std_interoperability.cpp b/tests/unittests/std_interoperability.cpp index 33d58277c..b0a97843a 100644 --- a/tests/unittests/std_interoperability.cpp +++ b/tests/unittests/std_interoperability.cpp @@ -892,6 +892,27 @@ TEST_CASE("Collection and std iterator adaptors", "[collection][container][adapt DOCUMENTED_STATIC_FAILURE((__cplusplus >= 202302L)); } + SECTION("Move iterator") { + // iterator + STATIC_REQUIRE(traits::has_iterator_v); + DOCUMENTED_STATIC_FAILURE(traits::has_iterator_category_v>); + // STATIC_REQUIRE( + // std::is_base_of_v::iterator_category>); +#if (__cplusplus >= 202002L) + DOCUMENTED_STATIC_FAILURE(std::input_iterator); +#endif + // TODO add more checks here + // const_iterator + STATIC_REQUIRE(traits::has_iterator_v); + DOCUMENTED_STATIC_FAILURE(traits::has_iterator_category_v>); + // STATIC_REQUIRE( + // std::is_base_of_v::iterator_category>); +#if (__cplusplus >= 202002L) + DOCUMENTED_STATIC_FAILURE(std::input_iterator); +#endif + // TODO add more checks here + } + #if (__cplusplus >= 202002L) SECTION("Counted iterator") { // iterator From f86ac42781df2810848f5ea75c31296c34db04ca Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Sun, 26 May 2024 02:03:05 +0200 Subject: [PATCH 40/43] fix typos --- doc/collections_as_container.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index e5f38ddf8..082214dd4 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -166,8 +166,8 @@ In addition to the *LegacyForwardIterator* the C++ standard specifies also the * ## Collection and standard algorithms -Most of the standard algorithms require the iterators to be at least *InputIterator*. The iterators of PODIO collection don't fulfil this requirement, therefore they are not compatible with standard algorithms according to specification. In practice some algorithms may still compile with the collections depending on a implementation of a given algorithm. +Most of the standard algorithms require the iterators to be at least *InputIterator*. The iterators of PODIO collection don't fulfil this requirement, therefore they are not compatible with standard algorithms according to the specification. In practice some algorithms may still compile with the collections depending on an implementation of a given algorithm. ## Standard range algorithms -The standard range algorithm use constrains to operate at least on `std::input_iterator`s and `std::ranges::input_range`s. The iterators of PODIO collection don't model these concepts, therefore can't be use with standard range algorithms. +The standard range algorithm use constrains to operate at least on `std::input_iterator`s and `std::ranges::input_range`s. The iterators of PODIO collection don't model these concepts, therefore can't be used with standard range algorithms. From 5d7dd386a7e8da9b721d57f6905be501922b66e0 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Mon, 3 Jun 2024 15:36:58 +0200 Subject: [PATCH 41/43] add collection vs container docs to index --- doc/index.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/index.rst b/doc/index.rst index 706e039d0..ea20f1385 100644 --- a/doc/index.rst +++ b/doc/index.rst @@ -18,5 +18,6 @@ Welcome to PODIO's documentation! advanced_topics.md templates.md python.md + collections_as_container.md cpp_api/api py_api/modules From 747789053c44c3d621d393634adce9a5c7b6cc6a Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Tue, 4 Jun 2024 17:34:50 +0200 Subject: [PATCH 42/43] add paragraph on internals fix typos fix swap docs clarify iterator as CollectionIterator fix destructor behaviour docs add standard algorithm links and warnings Co-authored-by: tmadlener --- doc/collections_as_container.md | 48 ++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index 082214dd4..bb5014139 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -2,7 +2,14 @@ Comparison of the PODIO `Collection`s with a C++ named requirement [*Container*](https://en.cppreference.com/w/cpp/named_req/Container). -The PODIO `Collection`s interface was designed to remind the standard *Container* interface, in particular `std::vector`. The perfect compliance with the *Container* is not achieved as the `Collection`s are concerned with additional semantics such as mutable/immutable element access, associations and relations, and IO which are not part of *Container*. +The PODIO `Collection`s interface was designed to mimic the standard *Container* interface, in particular `std::vector`. Perfect compliance with the *Container* is not achieved as the `Collection`s are concerned with additional semantics such as mutable/immutable element access, associations and relations, and IO which that are not part of *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. + +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. ### Container Types @@ -11,8 +18,8 @@ The PODIO `Collection`s interface was designed to remind the standard *Container | `value_type` | `T` | *[Erasable](https://en.cppreference.com/w/cpp/named_req/Erasable)* | ✔️ yes | Defined as immutable component type | | `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 | `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 | `const_iterator::value_type` not defined, not [*LegacyForwardIterator*](https://en.cppreference.com/w/cpp/named_req/ForwardIterator) ([see below](#legacyforwarditerator)) +| `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 | | @@ -20,29 +27,29 @@ The PODIO `Collection`s interface was designed to remind the standard *Container | Expression | Return type | Semantics | Fulfilled by Collection? | Comment | |------------|-------------|-----------|--------------------------|---------| -| `C()` | `C` | Creates an empty container | ✔️ yes | -| `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 | -| `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(a).begin()` | ✔️ yes | -| `a.cend()` | `const_iterator` | Same as `const_cast(a).end()`| ✔️ yes | +| `C()` | `C` | Creates an empty container | ✔️ yes | | +| `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.end()` | `(const_)iterator` | Iterator to one past the last element of `a` | ✔️ yes | | +| `a.cbegin()` | `const_iterator` | Same as `const_cast(a).begin()` | ✔️ yes | | +| `a.cend()` | `const_iterator` | Same as `const_cast(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)` | ✔️ yes | `a.swap(b)` not defined | -| `a.size()` | `size_type` | Same as `std::distance(a.begin(), a.end())` | ✔️ yes | +| `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.empty()` | Convertible to `bool` | Same as `a.begin() == a.end()` | ✔️ yes | +| `a.empty()` | Convertible to `bool` | Same as `a.begin() == a.end()` | ✔️ yes | | ## Collection as an *AllocatorAwareContainer* The C++ standard specifies [AllocatorAwareContainer](https://en.cppreference.com/w/cpp/named_req/AllocatorAwareContainer) for containers that can use other allocators beside the default allocator. -PODIO collections don't provide customization point for allocators and use only the default allocator. Therefore they are not *AllocatorAwareContainers*. +PODIO collections don't provide a customization point for allocators and use only the default allocator. Therefore they are not *AllocatorAwareContainers*. ### AllocatorAwareContainer types @@ -58,6 +65,7 @@ The PODIO Collections currently are not checked against expression and statement The C++ specifies a set of named requirements for iterators. Starting with C++20 the standard specifies also iterator concepts. The requirements imposed by the concepts and named requirements are similar but not identical. +In the following tables a convention from `Collection` is used: `iterator` stands for PODIO `MutableCollectionIterator` and `const_iterator` stands for PODIO `CollectionIterator`. ### Iterator summary | Named requirement | `iterator` | `const_iterator` | @@ -166,8 +174,10 @@ In addition to the *LegacyForwardIterator* the C++ standard specifies also the * ## Collection and standard algorithms -Most of the standard algorithms require the iterators to be at least *InputIterator*. The iterators of PODIO collection don't fulfil this requirement, therefore they are not compatible with standard algorithms according to the specification. In practice some algorithms may still compile with the collections depending on an implementation of a given algorithm. +Most of the standard algorithms require the iterators to be at least *InputIterator*. The iterators of PODIO collection don't fulfil this requirement, therefore they are not compatible with standard algorithms according to the specification. + +In practice, some algorithms may still compile with the collections depending on the implementation of a given algorithm. In general, the standard **algorithms mutating a collection will give wrong results**, while the standard algorithms not mutating a collection in principle should give correct results if they compile. ## Standard range algorithms -The standard range algorithm use constrains to operate at least on `std::input_iterator`s and `std::ranges::input_range`s. The iterators of PODIO collection don't model these concepts, therefore can't be used with standard range algorithms. +The standard range algorithm use constrains to operate at least on `std::input_iterator`s and `std::ranges::input_range`s. The iterators of PODIO collection don't model these concepts, therefore can't be used with standard range algorithms. The range algorithms won't compile with PODIO `Collection` iterators. From a55733c978215efdaa2c41778a06af695d516393 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Wed, 5 Jun 2024 23:59:32 +0200 Subject: [PATCH 43/43] clarify container value_type --- doc/collections_as_container.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/collections_as_container.md b/doc/collections_as_container.md index bb5014139..6b8461e5b 100644 --- a/doc/collections_as_container.md +++ b/doc/collections_as_container.md @@ -15,7 +15,7 @@ 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 immutable component type | +| `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 | | `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`|