Add static constexpr char* names and hand out views for them (#402)

* Add static constexpr char* names and hand out views for them

* Work around Catch2 problem in test discovery

* Add AUTO as possible option to USE_EXTERNAL_CATCH2

* Switch key4hep workflows to build Catch2 if necessary

.github/workflows/key4hep.yml

@@ -30,7 +30,7 @@ jobs:
             -DCMAKE_INSTALL_PREFIX=../install \
             -DCMAKE_CXX_STANDARD=17 \
             -DCMAKE_CXX_FLAGS=" -fdiagnostics-color=always -Werror -Wno-error=deprecated-declarations " \
-            -DUSE_EXTERNAL_CATCH2=ON \
+            -DUSE_EXTERNAL_CATCH2=AUTO \
             -G Ninja ..
           echo "::endgroup::"
           echo "::group::Build"

CMakeLists.txt

@@ -142,7 +142,8 @@ endif()
 
 #--- enable unit testing capabilities ------------------------------------------
 include(CTest)
-option(USE_EXTERNAL_CATCH2 "Link against an external Catch2 v3 static library, otherwise build it locally" ON)
+set(USE_EXTERNAL_CATCH2 AUTO CACHE STRING "Link against an external Catch2 v3 static library, otherwise build it locally")
+set_property(CACHE USE_EXTERNAL_CATCH2 PROPERTY STRINGS AUTO ON OFF)
 
 #--- enable CPack --------------------------------------------------------------
 

include/podio/CollectionBase.h

@@ -6,7 +6,7 @@
 #include "podio/SchemaEvolution.h"
 
 #include <iostream>
-#include <string>
+#include <string_view>
 #include <utility>
 #include <vector>
 
@@ -56,11 +56,11 @@ class CollectionBase {
   virtual size_t size() const = 0;
 
   /// fully qualified type name
-  virtual std::string getTypeName() const = 0;
+  virtual const std::string_view getTypeName() const = 0;
   /// fully qualified type name of elements - with namespace
-  virtual std::string getValueTypeName() const = 0;
+  virtual const std::string_view getValueTypeName() const = 0;
   /// fully qualified type name of stored POD elements - with namespace
-  virtual std::string getDataTypeName() const = 0;
+  virtual const std::string_view getDataTypeName() const = 0;
   /// schema version of the collection
   virtual SchemaVersionT getSchemaVersion() const = 0;
 

include/podio/SIOBlock.h

@@ -220,7 +220,7 @@ class SIOBlockFactory {
 private:
   SIOBlockFactory() = default;
 
-  typedef std::map<std::string, SIOBlock*> BlockMap;
+  typedef std::unordered_map<std::string, SIOBlock*> BlockMap;
   BlockMap _map{};
 
 public:

include/podio/UserDataCollection.h

@@ -93,6 +93,10 @@ class UserDataCollection : public CollectionBase {
   /// The schema version of UserDataCollections
   static constexpr SchemaVersionT schemaVersion = 1;
 
+  constexpr static auto typeName = userDataCollTypeName<BasicType>();
+  constexpr static auto valueTypeName = userDataTypeName<BasicType>();
+  constexpr static auto dataTypeName = userDataTypeName<BasicType>();
+
   /// prepare buffers for serialization
   void prepareForWrite() const override {
   }
@@ -133,18 +137,18 @@ class UserDataCollection : public CollectionBase {
   }
 
   /// fully qualified type name
-  std::string getTypeName() const override {
-    return userDataCollTypeName<BasicType>();
+  const std::string_view getTypeName() const override {
+    return typeName;
   }
 
   /// fully qualified type name of elements - with namespace
-  std::string getValueTypeName() const override {
-    return userDataTypeName<BasicType>();
+  const std::string_view getValueTypeName() const override {
+    return valueTypeName;
   }
 
   /// fully qualified type name of stored POD elements - with namespace
-  std::string getDataTypeName() const override {
-    return userDataTypeName<BasicType>();
+  const std::string_view getDataTypeName() const override {
+    return dataTypeName;
   }
 
   /// clear the collection and all internal states

python/podio/test_Frame.py

@@ -58,7 +58,7 @@ def test_frame_collections(self):
     # Not going over all collections here, as that should all be covered by the
     # c++ test cases; Simply picking a few and doing some basic tests
     mc_particles = self.event.get('mcparticles')
-    self.assertEqual(mc_particles.getValueTypeName(), 'ExampleMC')
+    self.assertEqual(mc_particles.getValueTypeName().data(), 'ExampleMC')
     self.assertEqual(len(mc_particles), 10)
     self.assertEqual(len(mc_particles[0].daughters()), 4)
 

python/templates/Collection.h.jinja2

@@ -62,6 +62,10 @@ public:
 //  {{ class.bare_type }}Collection({{ class.bare_type }}Vector* data, uint32_t collectionID);
   ~{{ class.bare_type }}Collection();
 
+  constexpr static auto typeName = "{{ (class | string ).strip(':') + "Collection" }}";
+  constexpr static auto valueTypeName = "{{ (class | string ).strip(':') }}";
+  constexpr static auto dataTypeName = "{{ (class | string ).strip(':') + "Data" }}";
+
   void clear() final;
 
   /// Print this collection to the passed stream
@@ -82,11 +86,11 @@ public:
   std::size_t size() const final;
 
   /// fully qualified type name
-  std::string getTypeName() const final { return std::string("{{ (class | string ).strip(':')+"Collection" }}"); }
+  const std::string_view getTypeName() const final { return typeName; }
   /// fully qualified type name of elements - with namespace
-  std::string getValueTypeName() const final { return std::string("{{ (class | string ).strip(':') }}"); }
+  const std::string_view getValueTypeName() const final { return valueTypeName; }
   /// fully qualified type name of stored POD elements - with namespace
-  std::string getDataTypeName() const final { return std::string("{{ (class | string ).strip(':')+"Data" }}"); }
+  const std::string_view getDataTypeName() const final { return dataTypeName; }
   /// schema version
   podio::SchemaVersionT getSchemaVersion() const final;
 

src/ROOTFrameWriter.cc

@@ -76,7 +76,7 @@ void ROOTFrameWriter::initBranches(CategoryInfo& catInfo, const std::vector<Stor
       branches.refs.push_back(catInfo.tree->Branch(brName.c_str(), refColl.get()));
     } else {
       // For "proper" collections we populate all branches, starting with the data
-      auto bufferDataType = "vector<" + coll->getDataTypeName() + ">";
+      const auto bufferDataType = "vector<" + std::string(coll->getDataTypeName()) + ">";
       branches.data = catInfo.tree->Branch(name.c_str(), bufferDataType.c_str(), buffers.data);
 
       const auto relVecNames = podio::DatamodelRegistry::instance().getRelationNames(coll->getValueTypeName());

src/ROOTReader.cc

@@ -284,7 +284,7 @@ void ROOTReader::createCollectionBranches(const std::vector<root_utils::Collecti
       branches.vecs.push_back(root_utils::getBranch(m_chain, brName.c_str()));
     }
 
-    const std::string bufferClassName = "std::vector<" + collection->getDataTypeName() + ">";
+    const auto bufferClassName = "std::vector<" + std::string(collection->getDataTypeName()) + ">";
     const auto bufferClass = isSubsetColl ? nullptr : TClass::GetClass(bufferClassName.c_str());
 
     m_storedClasses.emplace(name, std::make_tuple(bufferClass, collectionClass, collectionIndex++));

src/ROOTWriter.cc

@@ -56,7 +56,7 @@ void ROOTWriter::createBranches(const std::vector<StoreCollection>& collections)
     if (collBuffers.data) {
       // only create the data buffer branch if necessary
 
-      auto collClassName = "vector<" + coll->getDataTypeName() + ">";
+      const auto collClassName = "vector<" + std::string(coll->getDataTypeName()) + ">";
 
       branches.data = m_datatree->Branch(name.c_str(), collClassName.c_str(), collBuffers.data);
     }

src/SIOBlock.cc

@@ -27,7 +27,7 @@ SIOCollectionIDTableBlock::SIOCollectionIDTableBlock(podio::EventStore* store) :
           << id << ", name: " << table->name(id) << ")" << std::endl;
     }
 
-    _types.push_back(tmp->getValueTypeName());
+    _types.emplace_back(tmp->getValueTypeName());
     _isSubsetColl.push_back(tmp->isSubsetCollection());
   }
 }
@@ -109,7 +109,7 @@ std::shared_ptr<SIOBlock> SIOBlockFactory::createBlock(const std::string& typeSt
 
 std::shared_ptr<SIOBlock> SIOBlockFactory::createBlock(const podio::CollectionBase* col,
                                                        const std::string& name) const {
-  const std::string typeStr = col->getValueTypeName();
+  const auto typeStr = std::string(col->getValueTypeName()); // Need c++20 for transparent lookup
   const auto it = _map.find(typeStr);
 
   if (it != _map.end()) {

src/SIOWriter.cc

@@ -98,7 +98,7 @@ void SIOWriter::registerForWrite(const std::string& name) {
   }
   // Check if we can instantiate the blocks here so that we can skip the checks later
   if (auto blk = podio::SIOBlockFactory::instance().createBlock(colB, name); !blk) {
-    const auto typName = colB->getValueTypeName();
+    const auto typName = std::string(colB->getValueTypeName());
     throw std::runtime_error(std::string("could not create SIOBlock for type: ") + typName);
   }
 

tests/CMakeLists.txt

@@ -128,8 +128,14 @@ CREATE_PODIO_TEST(ostream_operator.cpp "")
 CREATE_PODIO_TEST(write_ascii.cpp "")
 
 if(USE_EXTERNAL_CATCH2)
-  find_package(Catch2 3 REQUIRED)
-else()
+  if (USE_EXTERNAL_CATCH2 STREQUAL AUTO)
+    find_package(Catch2 3.1)
+  else()
+    find_package(Catch2 3.1 REQUIRED)
+  endif()
+endif()
+
+if(NOT Catch2_FOUND)
   message(STATUS "Fetching local copy of Catch2 library for unit-tests...")
   # Build Catch2 with the default flags, to avoid generating warnings when we
   # build it
@@ -139,7 +145,7 @@ else()
   FetchContent_Declare(
     Catch2
     GIT_REPOSITORY https://github.com/catchorg/Catch2.git
-    GIT_TAG        v3.0.1
+    GIT_TAG        v3.1.0
     )
   FetchContent_MakeAvailable(Catch2)
   set(CMAKE_MODULE_PATH ${Catch2_SOURCE_DIR}/extras ${CMAKE_MODULE_PATH})
@@ -248,6 +254,14 @@ endif()
 
 option(SKIP_CATCH_DISCOVERY "Skip the Catch2 test discovery" OFF)
 
+# To work around https://github.com/catchorg/Catch2/issues/2424 we need the
+# DL_PATH argument for catch_discoer_tests which requires CMake 3.22 at least
+# The whole issue can be avoied if we skip the catch test discovery and set the
+# environment on our own
+if (CMAKE_VERSION VERSION_LESS 3.22)
+  set(SKIP_CATCH_DISCOVERY ON)
+endif()
+
 if (USE_SANITIZER MATCHES "Memory(WithOrigin)?" OR SKIP_CATCH_DISCOVERY)
   # Automatic test discovery fails with Memory sanitizers due to some issues in
   # Catch2. So in that case we skip the discovery step and simply run the thing
@@ -259,9 +273,7 @@ if (USE_SANITIZER MATCHES "Memory(WithOrigin)?" OR SKIP_CATCH_DISCOVERY)
     add_test(NAME unittest COMMAND unittest ${filter_tests})
     set_property(TEST unittest
       PROPERTY ENVIRONMENT
-        LD_LIBRARY_PATH=${CMAKE_CURRENT_BINARY_DIR}:${CMAKE_BINARY_DIR}/src:$<TARGET_FILE_DIR:ROOT::Tree>:$<$<TARGET_EXISTS:SIO::sio>:$<TARGET_FILE_DIR:SIO::sio>>:$ENV{LD_LIBRARY_PATH}
-        # Only pick up this build for testing
-        PODIO_SIOBLOCK_PATH=${CMAKE_CURRENT_BINARY_DIR}
+        "LD_LIBRARY_PATH=${CMAKE_CURRENT_BINARY_DIR}:${CMAKE_BINARY_DIR}/src:$<TARGET_FILE_DIR:ROOT::Tree>:$<$<TARGET_EXISTS:SIO::sio>:$<TARGET_FILE_DIR:SIO::sio>>:$ENV{LD_LIBRARY_PATH};PODIO_SIOBLOCK_PATH=${CMAKE_CURRENT_BINARY_DIR}"
       )
   endif()
 else()
@@ -270,10 +282,9 @@ else()
       WORKING_DIRECTORY ${CMAKE_CURRENT_LIST_DIR}
       TEST_PREFIX "UT_" # make it possible to filter easily with -R ^UT
       TEST_SPEC ${filter_tests} # discover only tests that are known to not fail
+      DL_PATHS ${CMAKE_CURRENT_BINARY_DIR}:${CMAKE_BINARY_DIR}/src:$<TARGET_FILE_DIR:ROOT::Tree>:$<$<TARGET_EXISTS:SIO::sio>:$<TARGET_FILE_DIR:SIO::sio>>:$ENV{LD_LIBRARY_PATH}
       PROPERTIES
         ENVIRONMENT
-        LD_LIBRARY_PATH=${CMAKE_CURRENT_BINARY_DIR}:${CMAKE_BINARY_DIR}/src:$<TARGET_FILE_DIR:ROOT::Tree>:$<$<TARGET_EXISTS:SIO::sio>:$<TARGET_FILE_DIR:SIO::sio>>:$ENV{LD_LIBRARY_PATH}
-        # Only pick up this build for testing
         PODIO_SIOBLOCK_PATH=${CMAKE_CURRENT_BINARY_DIR}
   )
 endif()

tools/podio-dump

@@ -53,7 +53,7 @@ def print_frame(frame, cat_name, ientry, detailed):
       coll.print()
       print(flush=True)
     else:
-      print(f'{name:<38} {coll.getID():<4} {coll.getValueTypeName():<32} {len(coll):<10}')
+      print(f'{name:<38} {coll.getID():<4} {coll.getValueTypeName().data():<32} {len(coll):<10}')
 
   # And then parameters
   print('\nParameters:', flush=True)