Formalize common interface for collections with a concept (#758)

* add checks to `CollectionType` concept

* add `at` and `create` methods to `UserDataCollection`

* require collection typeName strings in concept

* use same_as for create and element access methods

* require to not be copyable

* un-nest general concept checks

* fix and add comments on stripping references

Author: m-fila

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<BasicType>::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<BasicType>::reference at(size_t idx) {
+    return _vec.at(idx);
+  }
+  typename std::vector<BasicType>::const_reference at(size_t idx) const {
+    return _vec.at(idx);
+  }
+
   void resize(size_t count) {
     _vec.resize(count);
   }

include/podio/utilities/TypeHelpers.h

@@ -1,7 +1,10 @@
 #ifndef PODIO_UTILITIES_TYPEHELPERS_H
 #define PODIO_UTILITIES_TYPEHELPERS_H
 
+#include <concepts>
+#include <iterator>
 #include <map>
+#include <ranges>
 #include <tuple>
 #include <type_traits>
 #include <unordered_map>
@@ -242,9 +245,65 @@ 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 <typename T>
-concept CollectionType = std::is_base_of_v<CollectionBase, T>;
+concept CollectionType = !std::is_abstract_v<T> && std::derived_from<T, CollectionBase> &&
+    std::default_initializable<T> && std::destructible<T> && std::movable<T> && !std::copyable<T> &&
+    std::ranges::random_access_range<T> && requires(T t, const T ct) {
+      // typeName's
+      { T::typeName } -> std::convertible_to<std::string_view>;
+      { std::bool_constant<(T::typeName, true)>() } -> std::same_as<std::true_type>; // ~is annotated with constexpr
+      { T::valueTypeName } -> std::convertible_to<std::string_view>;
+      {
+        std::bool_constant<(T::valueTypeName, true)>()
+      } -> std::same_as<std::true_type>; // ~is annotated with constexpr
+      { T::dataTypeName } -> std::convertible_to<std::string_view>;
+      { std::bool_constant<(T::dataTypeName, true)>() } -> std::same_as<std::true_type>; // ~is annotated with constexpr
+      // typedefs
+      typename T::value_type;
+      typename T::mutable_type;
+      requires std::convertible_to<typename T::mutable_type, typename T::value_type>;
+      typename T::difference_type;
+      requires std::signed_integral<typename T::difference_type>;
+      typename T::size_type;
+      requires std::unsigned_integral<typename T::size_type>;
+      typename T::const_iterator;
+      requires std::random_access_iterator<typename T::const_iterator>;
+      typename T::iterator;
+      requires std::random_access_iterator<typename T::iterator>;
+      typename T::const_reverse_iterator;
+      requires std::random_access_iterator<typename T::const_reverse_iterator>;
+      typename T::reverse_iterator;
+      requires std::random_access_iterator<typename T::reverse_iterator>;
+      // member functions
+      requires std::same_as<std::remove_reference_t<decltype(t.create())>,
+                            typename T::mutable_type>; // UserDataCollection::create() returns reference which has to be
+                                                       // stripped to be same as expected typedef
+      { t.push_back(std::declval<std::add_lvalue_reference_t<std::add_const_t<typename T::mutable_type>>>()) };
+      { t.push_back(std::declval<std::add_lvalue_reference_t<std::add_const_t<typename T::value_type>>>()) };
+      { t.begin() } -> std::same_as<typename T::iterator>;
+      { t.cbegin() } -> std::same_as<typename T::const_iterator>;
+      { ct.begin() } -> std::same_as<typename T::const_iterator>;
+      { t.end() } -> std::same_as<typename T::iterator>;
+      { t.cend() } -> std::same_as<typename T::const_iterator>;
+      { ct.end() } -> std::same_as<typename T::const_iterator>;
+      { t.rbegin() } -> std::same_as<typename T::reverse_iterator>;
+      { t.crbegin() } -> std::same_as<typename T::const_reverse_iterator>;
+      { ct.rbegin() } -> std::same_as<typename T::const_reverse_iterator>;
+      { t.rend() } -> std::same_as<typename T::reverse_iterator>;
+      { t.crend() } -> std::same_as<typename T::const_reverse_iterator>;
+      { ct.rend() } -> std::same_as<typename T::const_reverse_iterator>;
+      // UserDataCollection element access returns by reference or const reference which has to be stripped to be same
+      // as expected typedef
+      requires std::same_as<std::remove_reference_t<decltype(t[std::declval<typename T::size_type>()])>,
+                            typename T::mutable_type>;
+      requires std::same_as<std::remove_cvref_t<decltype(ct[std::declval<typename T::size_type>()])>,
+                            typename T::value_type>;
+      requires std::same_as<std::remove_reference_t<decltype(t.at(std::declval<typename T::size_type>()))>,
+                            typename T::mutable_type>;
+      requires std::same_as<std::remove_cvref_t<decltype(ct.at(std::declval<typename T::size_type>()))>,
+                            typename T::value_type>;
+    };
 
 namespace utils {
   template <typename... T>

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<TestLColl>);
+}
 
 TEST_CASE("LinkCollection constness", "[links][static-checks][const-correctness]") {
   // Test type-aliases in LinkCollection

tests/unittests/unittest.cpp

@@ -10,6 +10,7 @@
 #include <type_traits>
 #include <unordered_map>
 #include <unordered_set>
+#include <utility>
 #include <vector>
 
 #include "catch2/catch_test_macros.hpp"
@@ -23,6 +24,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 +392,15 @@ TEST_CASE("thread-safe prepareForWrite", "[basics][multithread]") {
   }
 }
 
+TEST_CASE("UserDataCollection collection concept", "[concepts]") {
+  // check each type in tuple
+  std::apply(
+      []<typename... Ts>(Ts...) {
+        ([]<typename T>(T) { STATIC_REQUIRE(podio::CollectionType<podio::UserDataCollection<T>>); }(Ts{}), ...);
+      },
+      podio::SupportedUserDataTypes{});
+}
+
 TEST_CASE("UserDataCollection print", "[basics]") {
   auto coll = podio::UserDataCollection<int32_t>();
   coll.push_back(1);
@@ -402,6 +413,19 @@ TEST_CASE("UserDataCollection print", "[basics]") {
   REQUIRE(sstr.str() == "[1, 2, 3]");
 }
 
+TEST_CASE("UserDataCollection access", "[basics]") {
+  auto coll = podio::UserDataCollection<int32_t>();
+  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();
@@ -609,6 +633,11 @@ TEST_CASE("UserInitialization", "[basics][code-gen]") {
   REQUIRE(ex.comp().arr[1] == 3.4);
 }
 
+TEST_CASE("Collection concepts", "[collections][concepts]") {
+  STATIC_REQUIRE(podio::CollectionType<ExampleClusterCollection>);
+  STATIC_REQUIRE(podio::CollectionType<ExampleHitCollection>);
+}
+
 TEST_CASE("Collection size and empty", "[basics][collections]") {
   ExampleClusterCollection coll{};
   REQUIRE(coll.empty());