From 18d4e9c79088ca1582856ac54a78cdc5d1e046ef Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Mon, 10 Mar 2025 17:18:13 +0100 Subject: [PATCH 1/7] add checks to `CollectionType` concept --- include/podio/utilities/TypeHelpers.h | 59 ++++++++++++++++++++++++++- tests/unittests/links.cpp | 4 ++ tests/unittests/unittest.cpp | 15 +++++++ 3 files changed, 76 insertions(+), 2 deletions(-) diff --git a/include/podio/utilities/TypeHelpers.h b/include/podio/utilities/TypeHelpers.h index a2206f963..48930a055 100644 --- a/include/podio/utilities/TypeHelpers.h +++ b/include/podio/utilities/TypeHelpers.h @@ -1,7 +1,10 @@ #ifndef PODIO_UTILITIES_TYPEHELPERS_H #define PODIO_UTILITIES_TYPEHELPERS_H +#include +#include #include +#include #include #include #include @@ -242,9 +245,61 @@ namespace detail { // forward declaration to be able to use it below class CollectionBase; -/// Concept for checking whether a passed type T inherits from podio::CollectionBase +/// Concept for checking whether a passed type T is a collection template -concept CollectionType = std::is_base_of_v; +concept CollectionType = requires(T t, const T ct) { + // general concepts + requires !std::is_abstract_v; + requires std::derived_from; + requires std::default_initializable; + requires std::destructible; + requires std::movable; + requires std::move_constructible; + requires std::ranges::random_access_range; + // typeName's + // { T::typeName } -> std::convertible_to; + // { std::bool_constant<(T::typeName, true)>() } -> std::same_as; // ~is annotated with constexpr + // { T::valueTypeName } -> std::convertible_to; + // { std::bool_constant<(T::valueTypeName, true)>() } -> std::same_as; // ~is annotated with constexpr + // { T::dataTypeName } -> std::convertible_to; + // { std::bool_constant<(T::dataTypeName, true)>() } -> std::same_as; // ~is annotated with constexpr + // typedefs + typename T::value_type; + typename T::mutable_type; + requires std::convertible_to; + typename T::difference_type; + requires std::signed_integral; + typename T::size_type; + requires std::unsigned_integral; + typename T::const_iterator; + requires std::random_access_iterator; + typename T::iterator; + requires std::random_access_iterator; + typename T::const_reverse_iterator; + requires std::random_access_iterator; + typename T::reverse_iterator; + requires std::random_access_iterator; + // member functions + { t.create() } -> std::convertible_to; + { t.push_back(std::declval>>()) }; + { t.push_back(std::declval>>()) }; + { t.begin() } -> std::same_as; + { t.cbegin() } -> std::same_as; + { ct.begin() } -> std::same_as; + { t.end() } -> std::same_as; + { t.cend() } -> std::same_as; + { ct.end() } -> std::same_as; + { t.rbegin() } -> std::same_as; + { t.crbegin() } -> std::same_as; + { ct.rbegin() } -> std::same_as; + { t.rend() } -> std::same_as; + { t.crend() } -> std::same_as; + { ct.rend() } -> std::same_as; + { t[std::declval()] } -> std::convertible_to; + { ct[std::declval()] } -> std::convertible_to; + { t.at(std::declval()) } -> std::convertible_to; + { ct.at(std::declval()) } -> std::convertible_to; +}; namespace utils { template diff --git a/tests/unittests/links.cpp b/tests/unittests/links.cpp index b48b758aa..2ffc96916 100644 --- a/tests/unittests/links.cpp +++ b/tests/unittests/links.cpp @@ -3,6 +3,7 @@ #include "podio/LinkCollection.h" #include "podio/LinkNavigator.h" +#include "podio/utilities/TypeHelpers.h" #include "datamodel/ExampleClusterCollection.h" #include "datamodel/ExampleHitCollection.h" @@ -297,6 +298,9 @@ TEST_CASE("Links templated accessors", "[links]") { } } // NOLINTEND(clang-analyzer-cplusplus.NewDeleteLeaks) +TEST_CASE("LinkCollection collection concept", "[links][concepts]") { + STATIC_REQUIRE(podio::CollectionType); +} TEST_CASE("LinkCollection constness", "[links][static-checks][const-correctness]") { // Test type-aliases in LinkCollection diff --git a/tests/unittests/unittest.cpp b/tests/unittests/unittest.cpp index a2dccf92a..e9641a77d 100644 --- a/tests/unittests/unittest.cpp +++ b/tests/unittests/unittest.cpp @@ -23,6 +23,7 @@ #include "podio/ROOTReader.h" #include "podio/ROOTWriter.h" #include "podio/podioVersion.h" +#include "podio/utilities/TypeHelpers.h" #ifndef PODIO_ENABLE_SIO #define PODIO_ENABLE_SIO 0 @@ -390,6 +391,15 @@ TEST_CASE("thread-safe prepareForWrite", "[basics][multithread]") { } } +TEST_CASE("UserDataCollection collection concept", "[concepts]") { + // check each type in tuple + std::apply( + [](Ts...) { + ([](T) { STATIC_REQUIRE(podio::CollectionType>); }(Ts{}), ...); + }, + podio::SupportedUserDataTypes{}); +} + TEST_CASE("UserDataCollection print", "[basics]") { auto coll = podio::UserDataCollection(); coll.push_back(1); @@ -609,6 +619,11 @@ TEST_CASE("UserInitialization", "[basics][code-gen]") { REQUIRE(ex.comp().arr[1] == 3.4); } +TEST_CASE("Collection concepts", "[collections][concepts]") { + STATIC_REQUIRE(podio::CollectionType); + STATIC_REQUIRE(podio::CollectionType); +} + TEST_CASE("Collection size and empty", "[basics][collections]") { ExampleClusterCollection coll{}; REQUIRE(coll.empty()); From 4853f16d06d71b5c9f0c20cea4588017fe7eed5c Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Mon, 10 Mar 2025 17:18:48 +0100 Subject: [PATCH 2/7] add `at` and `create` methods to `UserDataCollection` --- include/podio/UserDataCollection.h | 11 +++++++++++ tests/unittests/unittest.cpp | 14 ++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/include/podio/UserDataCollection.h b/include/podio/UserDataCollection.h index dae5c4c40..b7cd33508 100644 --- a/include/podio/UserDataCollection.h +++ b/include/podio/UserDataCollection.h @@ -206,6 +206,10 @@ class UserDataCollection : public CollectionBase { // ----- some wrappers for std::vector and access to the complete std::vector (if really needed) + typename std::vector::reference create() { + return _vec.emplace_back(); + } + iterator begin() { return _vec.begin(); } @@ -251,6 +255,13 @@ class UserDataCollection : public CollectionBase { return _vec[idx]; } + typename std::vector::reference at(size_t idx) { + return _vec.at(idx); + } + typename std::vector::const_reference at(size_t idx) const { + return _vec.at(idx); + } + void resize(size_t count) { _vec.resize(count); } diff --git a/tests/unittests/unittest.cpp b/tests/unittests/unittest.cpp index e9641a77d..ef96af9b3 100644 --- a/tests/unittests/unittest.cpp +++ b/tests/unittests/unittest.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include "catch2/catch_test_macros.hpp" @@ -412,6 +413,19 @@ TEST_CASE("UserDataCollection print", "[basics]") { REQUIRE(sstr.str() == "[1, 2, 3]"); } +TEST_CASE("UserDataCollection access", "[basics]") { + auto coll = podio::UserDataCollection(); + auto& x = coll.create(); + x = 42; + REQUIRE(coll.size() == 1); + REQUIRE(coll.at(0) == 42); + coll.at(0) = 43; + REQUIRE(coll[0] == 43); + coll[0] = 44; + REQUIRE(std::as_const(coll).at(0) == 44); + REQUIRE(std::as_const(coll)[0] == 44); +} + /* TEST_CASE("Arrays") { auto obj = ExampleWithArray(); From 9921d749a73fb25422cfa627b944ea9912fe9dc1 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Wed, 2 Apr 2025 10:46:59 +0200 Subject: [PATCH 3/7] require collection typeName strings in concept --- include/podio/utilities/TypeHelpers.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/include/podio/utilities/TypeHelpers.h b/include/podio/utilities/TypeHelpers.h index 48930a055..1d60fc3aa 100644 --- a/include/podio/utilities/TypeHelpers.h +++ b/include/podio/utilities/TypeHelpers.h @@ -257,12 +257,12 @@ concept CollectionType = requires(T t, const T ct) { requires std::move_constructible; requires std::ranges::random_access_range; // typeName's - // { T::typeName } -> std::convertible_to; - // { std::bool_constant<(T::typeName, true)>() } -> std::same_as; // ~is annotated with constexpr - // { T::valueTypeName } -> std::convertible_to; - // { std::bool_constant<(T::valueTypeName, true)>() } -> std::same_as; // ~is annotated with constexpr - // { T::dataTypeName } -> std::convertible_to; - // { std::bool_constant<(T::dataTypeName, true)>() } -> std::same_as; // ~is annotated with constexpr + { T::typeName } -> std::convertible_to; + { std::bool_constant<(T::typeName, true)>() } -> std::same_as; // ~is annotated with constexpr + { T::valueTypeName } -> std::convertible_to; + { std::bool_constant<(T::valueTypeName, true)>() } -> std::same_as; // ~is annotated with constexpr + { T::dataTypeName } -> std::convertible_to; + { std::bool_constant<(T::dataTypeName, true)>() } -> std::same_as; // ~is annotated with constexpr // typedefs typename T::value_type; typename T::mutable_type; From a410261c56506eafa63f39453daa52edee81628f Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Wed, 2 Apr 2025 13:18:29 +0200 Subject: [PATCH 4/7] use same_as for create and element access methods --- include/podio/utilities/TypeHelpers.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/include/podio/utilities/TypeHelpers.h b/include/podio/utilities/TypeHelpers.h index 1d60fc3aa..9c22b6147 100644 --- a/include/podio/utilities/TypeHelpers.h +++ b/include/podio/utilities/TypeHelpers.h @@ -280,7 +280,7 @@ concept CollectionType = requires(T t, const T ct) { typename T::reverse_iterator; requires std::random_access_iterator; // member functions - { t.create() } -> std::convertible_to; + requires std::same_as, typename T::mutable_type>; { t.push_back(std::declval>>()) }; { t.push_back(std::declval>>()) }; { t.begin() } -> std::same_as; @@ -295,10 +295,14 @@ concept CollectionType = requires(T t, const T ct) { { t.rend() } -> std::same_as; { t.crend() } -> std::same_as; { ct.rend() } -> std::same_as; - { t[std::declval()] } -> std::convertible_to; - { ct[std::declval()] } -> std::convertible_to; - { t.at(std::declval()) } -> std::convertible_to; - { ct.at(std::declval()) } -> std::convertible_to; + requires std::same_as()])>, + typename T::mutable_type>; + requires std::same_as()])>, + typename T::value_type>; + requires std::same_as()))>, + typename T::mutable_type>; + requires std::same_as()))>, + typename T::value_type>; }; namespace utils { From d71f1759cf458e455befbf48969b6ffc04a13edd Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Wed, 2 Apr 2025 13:42:39 +0200 Subject: [PATCH 5/7] require to not be copyable --- include/podio/utilities/TypeHelpers.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/podio/utilities/TypeHelpers.h b/include/podio/utilities/TypeHelpers.h index 9c22b6147..f1326d088 100644 --- a/include/podio/utilities/TypeHelpers.h +++ b/include/podio/utilities/TypeHelpers.h @@ -254,7 +254,7 @@ concept CollectionType = requires(T t, const T ct) { requires std::default_initializable; requires std::destructible; requires std::movable; - requires std::move_constructible; + requires !std::copyable; requires std::ranges::random_access_range; // typeName's { T::typeName } -> std::convertible_to; From 56254d10182780c4e8bc283ee6648d3967bd6f4d Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Tue, 13 May 2025 15:39:36 +0200 Subject: [PATCH 6/7] un-nest general concept checks --- include/podio/utilities/TypeHelpers.h | 110 +++++++++++++------------- 1 file changed, 53 insertions(+), 57 deletions(-) diff --git a/include/podio/utilities/TypeHelpers.h b/include/podio/utilities/TypeHelpers.h index f1326d088..905b4705b 100644 --- a/include/podio/utilities/TypeHelpers.h +++ b/include/podio/utilities/TypeHelpers.h @@ -247,63 +247,59 @@ class CollectionBase; /// Concept for checking whether a passed type T is a collection template -concept CollectionType = requires(T t, const T ct) { - // general concepts - requires !std::is_abstract_v; - requires std::derived_from; - requires std::default_initializable; - requires std::destructible; - requires std::movable; - requires !std::copyable; - requires std::ranges::random_access_range; - // typeName's - { T::typeName } -> std::convertible_to; - { std::bool_constant<(T::typeName, true)>() } -> std::same_as; // ~is annotated with constexpr - { T::valueTypeName } -> std::convertible_to; - { std::bool_constant<(T::valueTypeName, true)>() } -> std::same_as; // ~is annotated with constexpr - { T::dataTypeName } -> std::convertible_to; - { std::bool_constant<(T::dataTypeName, true)>() } -> std::same_as; // ~is annotated with constexpr - // typedefs - typename T::value_type; - typename T::mutable_type; - requires std::convertible_to; - typename T::difference_type; - requires std::signed_integral; - typename T::size_type; - requires std::unsigned_integral; - typename T::const_iterator; - requires std::random_access_iterator; - typename T::iterator; - requires std::random_access_iterator; - typename T::const_reverse_iterator; - requires std::random_access_iterator; - typename T::reverse_iterator; - requires std::random_access_iterator; - // member functions - requires std::same_as, typename T::mutable_type>; - { t.push_back(std::declval>>()) }; - { t.push_back(std::declval>>()) }; - { t.begin() } -> std::same_as; - { t.cbegin() } -> std::same_as; - { ct.begin() } -> std::same_as; - { t.end() } -> std::same_as; - { t.cend() } -> std::same_as; - { ct.end() } -> std::same_as; - { t.rbegin() } -> std::same_as; - { t.crbegin() } -> std::same_as; - { ct.rbegin() } -> std::same_as; - { t.rend() } -> std::same_as; - { t.crend() } -> std::same_as; - { ct.rend() } -> std::same_as; - requires std::same_as()])>, - typename T::mutable_type>; - requires std::same_as()])>, - typename T::value_type>; - requires std::same_as()))>, - typename T::mutable_type>; - requires std::same_as()))>, - typename T::value_type>; -}; +concept CollectionType = !std::is_abstract_v && std::derived_from && + std::default_initializable && std::destructible && std::movable && !std::copyable && + std::ranges::random_access_range && requires(T t, const T ct) { + // typeName's + { T::typeName } -> std::convertible_to; + { std::bool_constant<(T::typeName, true)>() } -> std::same_as; // ~is annotated with constexpr + { T::valueTypeName } -> std::convertible_to; + { + std::bool_constant<(T::valueTypeName, true)>() + } -> std::same_as; // ~is annotated with constexpr + { T::dataTypeName } -> std::convertible_to; + { std::bool_constant<(T::dataTypeName, true)>() } -> std::same_as; // ~is annotated with constexpr + // typedefs + typename T::value_type; + typename T::mutable_type; + requires std::convertible_to; + typename T::difference_type; + requires std::signed_integral; + typename T::size_type; + requires std::unsigned_integral; + typename T::const_iterator; + requires std::random_access_iterator; + typename T::iterator; + requires std::random_access_iterator; + typename T::const_reverse_iterator; + requires std::random_access_iterator; + typename T::reverse_iterator; + requires std::random_access_iterator; + // member functions + requires std::same_as, typename T::mutable_type>; + { t.push_back(std::declval>>()) }; + { t.push_back(std::declval>>()) }; + { t.begin() } -> std::same_as; + { t.cbegin() } -> std::same_as; + { ct.begin() } -> std::same_as; + { t.end() } -> std::same_as; + { t.cend() } -> std::same_as; + { ct.end() } -> std::same_as; + { t.rbegin() } -> std::same_as; + { t.crbegin() } -> std::same_as; + { ct.rbegin() } -> std::same_as; + { t.rend() } -> std::same_as; + { t.crend() } -> std::same_as; + { ct.rend() } -> std::same_as; + requires std::same_as()])>, + typename T::mutable_type>; + requires std::same_as()])>, + typename T::value_type>; + requires std::same_as()))>, + typename T::mutable_type>; + requires std::same_as()))>, + typename T::value_type>; + }; namespace utils { template From a8e913a6c6c5252ebd217bf35257881f9b542ac3 Mon Sep 17 00:00:00 2001 From: Mateusz Jakub Fila Date: Tue, 13 May 2025 16:06:58 +0200 Subject: [PATCH 7/7] fix and add comments on stripping references --- include/podio/utilities/TypeHelpers.h | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/include/podio/utilities/TypeHelpers.h b/include/podio/utilities/TypeHelpers.h index 905b4705b..f20334d9b 100644 --- a/include/podio/utilities/TypeHelpers.h +++ b/include/podio/utilities/TypeHelpers.h @@ -276,7 +276,9 @@ concept CollectionType = !std::is_abstract_v && std::derived_from; // member functions - requires std::same_as, typename T::mutable_type>; + requires std::same_as, + typename T::mutable_type>; // UserDataCollection::create() returns reference which has to be + // stripped to be same as expected typedef { t.push_back(std::declval>>()) }; { t.push_back(std::declval>>()) }; { t.begin() } -> std::same_as; @@ -291,11 +293,13 @@ concept CollectionType = !std::is_abstract_v && std::derived_from std::same_as; { t.crend() } -> std::same_as; { ct.rend() } -> std::same_as; - requires std::same_as()])>, + // UserDataCollection element access returns by reference or const reference which has to be stripped to be same + // as expected typedef + requires std::same_as()])>, typename T::mutable_type>; requires std::same_as()])>, typename T::value_type>; - requires std::same_as()))>, + requires std::same_as()))>, typename T::mutable_type>; requires std::same_as()))>, typename T::value_type>;