Update how type information is passed from Phlex to FORM (#252)

* Switch product from type_index to type_info
* Switch FORM from type_index to type_info
* FORM: Remove type_map
* FORM: Demangle type_info before printing name
* FORM: Add more information to exception msg

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: pcanal

form/form/form.cpp

@@ -2,12 +2,14 @@
 
 #include "form.hpp"
 
+#include <stdexcept>
+#include <typeinfo>
+
 namespace form::experimental {
 
-  form_interface::form_interface(std::shared_ptr<product_type_names> tm,
-                                 config::output_item_config const& output_config,
+  form_interface::form_interface(config::output_item_config const& output_config,
                                  config::tech_setting_config const& tech_config) :
-    m_pers(nullptr), m_type_map(tm)
+    m_pers(nullptr)
   {
     for (auto const& item : output_config.getItems()) {
       m_product_to_config.emplace(item.product_name,
@@ -30,12 +32,10 @@ namespace form::experimental {
       throw std::runtime_error("No configuration found for product: " + pb.label);
     }
 
-    std::string const type = m_type_map->names[pb.type];
-
-    std::map<std::string, std::string> products = {{pb.label, type}};
+    std::map<std::string, std::type_info const*> products = {{pb.label, pb.type}};
     m_pers->createContainers(creator, products);
 
-    m_pers->registerWrite(creator, pb.label, pb.data, type);
+    m_pers->registerWrite(creator, pb.label, pb.data, *pb.type);
 
     m_pers->commitOutput(creator, segment_id);
   }
@@ -54,18 +54,16 @@ namespace form::experimental {
     }
 
     // FIXME: Really only needed on first call
-    std::map<std::string, std::string> product_types;
+    std::map<std::string, std::type_info const*> product_types;
     for (auto const& pb : products) {
-      std::string const& type = m_type_map->names[pb.type];
-      product_types.insert(std::make_pair(pb.label, type));
+      product_types.insert(std::make_pair(pb.label, pb.type));
     }
 
     m_pers->createContainers(creator, product_types);
 
     for (auto const& pb : products) {
-      std::string const& type = m_type_map->names[pb.type];
       // FIXME: We could consider checking id to be identical for all product bases here
-      m_pers->registerWrite(creator, pb.label, pb.data, type);
+      m_pers->registerWrite(creator, pb.label, pb.data, *pb.type);
     }
 
     m_pers->commitOutput(creator, segment_id);
@@ -81,8 +79,6 @@ namespace form::experimental {
       throw std::runtime_error("No configuration found for product: " + pb.label);
     }
 
-    std::string type = m_type_map->names[pb.type];
-
-    m_pers->read(creator, pb.label, segment_id, &pb.data, type);
+    m_pers->read(creator, pb.label, segment_id, &pb.data, *pb.type);
   }
 }

form/form/form.hpp

@@ -9,26 +9,20 @@
 #include <map>
 #include <memory>
 #include <string>
-#include <typeindex>
-#include <unordered_map>
+#include <typeinfo>
 #include <vector>
 
 namespace form::experimental {
 
-  struct product_type_names {
-    std::unordered_map<std::type_index, std::string> names;
-  };
-
   struct product_with_name {
     std::string label;
     void const* data;
-    std::type_index type;
+    std::type_info const* type;
   };
 
   class form_interface {
   public:
-    form_interface(std::shared_ptr<product_type_names> tm,
-                   config::output_item_config const& output_config,
+    form_interface(config::output_item_config const& output_config,
                    config::tech_setting_config const& tech_config);
     ~form_interface() = default;
 
@@ -46,7 +40,6 @@ namespace form::experimental {
 
   private:
     std::unique_ptr<form::detail::experimental::IPersistence> m_pers;
-    std::shared_ptr<product_type_names> m_type_map;
     std::map<std::string, form::experimental::config::PersistenceItem> m_product_to_config;
   };
 }

form/form_module.cpp

@@ -14,11 +14,10 @@ namespace {
 
   class FormOutputModule {
   public:
-    FormOutputModule(std::shared_ptr<form::experimental::product_type_names> type_map,
-                     std::string output_file,
+    FormOutputModule(std::string output_file,
                      int technology,
                      std::vector<std::string> const& products_to_save) :
-      m_type_map(type_map), m_output_file(std::move(output_file)), m_technology(technology)
+      m_output_file(std::move(output_file)), m_technology(technology)
     {
       std::cout << "FormOutputModule initialized\n";
       std::cout << "  Output file: " << m_output_file << "\n";
@@ -39,8 +38,7 @@ namespace {
       }
 
       // Initialize FORM interface
-      m_form_interface =
-        std::make_unique<form::experimental::form_interface>(type_map, output_cfg, tech_cfg);
+      m_form_interface = std::make_unique<form::experimental::form_interface>(output_cfg, tech_cfg);
     }
 
     // This method is called by Phlex - signature must be: void(product_store const&)
@@ -82,7 +80,7 @@ namespace {
         // Create FORM product with metadata
         products.emplace_back(product_name,           // label, from map key
                               product_ptr->address(), // data,  from phlex product_base
-                              product_ptr->type()     // type,  from phlex product_base
+                              &product_ptr->type()    // type, from phlex product_base
         );
       }
 
@@ -96,7 +94,6 @@ namespace {
     }
 
   private:
-    std::shared_ptr<form::experimental::product_type_names> m_type_map;
     std::string m_output_file;
     int m_technology;
     std::unique_ptr<form::experimental::form_interface> m_form_interface;
@@ -108,19 +105,6 @@ PHLEX_REGISTER_ALGORITHMS(m, config)
 {
   std::cout << "Registering FORM output module...\n";
 
-  // Create type map
-  auto type_map = std::make_shared<form::experimental::product_type_names>();
-
-  // Register some fundamental type for simple products
-  type_map->names[std::type_index(typeid(int))] = "int";
-  type_map->names[std::type_index(typeid(long))] = "long";
-  type_map->names[std::type_index(typeid(float))] = "float";
-  type_map->names[std::type_index(typeid(double))] = "double";
-  type_map->names[std::type_index(typeid(std::vector<int>))] = "std::vector<int>";
-  type_map->names[std::type_index(typeid(std::vector<long>))] = "std::vector<long>";
-  type_map->names[std::type_index(typeid(std::vector<float>))] = "std::vector<float>";
-  type_map->names[std::type_index(typeid(std::vector<double>))] = "std::vector<double>";
-
   // Extract configuration from Phlex config
   std::string output_file = config.get<std::string>("output_file", "output.root");
   std::string tech_string = config.get<std::string>("technology", "ROOT_TTREE");
@@ -146,7 +130,7 @@ PHLEX_REGISTER_ALGORITHMS(m, config)
 
   // Phlex needs an OBJECT
   // Create the FORM output module
-  auto form_output = m.make<FormOutputModule>(type_map, output_file, technology, products_to_save);
+  auto form_output = m.make<FormOutputModule>(output_file, technology, products_to_save);
 
   // Phlex needs a MEMBER FUNCTION to call
   // Register the callback that Phlex will invoke

form/persistence/ipersistence.hpp

@@ -6,6 +6,7 @@
 #include <map>
 #include <memory>
 #include <string>
+#include <typeinfo>
 
 namespace form::experimental::config {
   class output_item_config;
@@ -26,18 +27,18 @@ namespace form::detail::experimental {
       form::experimental::config::output_item_config const& outputItems) = 0;
 
     virtual void createContainers(std::string const& creator,
-                                  std::map<std::string, std::string> const& products) = 0;
+                                  std::map<std::string, std::type_info const*> const& products) = 0;
     virtual void registerWrite(std::string const& creator,
                                std::string const& label,
                                void const* data,
-                               std::string const& type) = 0;
+                               std::type_info const& type) = 0;
     virtual void commitOutput(std::string const& creator, std::string const& id) = 0;
 
     virtual void read(std::string const& creator,
                       std::string const& label,
                       std::string const& id,
                       void const** data,
-                      std::string& type) = 0;
+                      std::type_info const& type) = 0;
   };
 
   std::unique_ptr<IPersistence> createPersistence();

form/persistence/persistence.cpp

@@ -4,6 +4,9 @@
 
 #include <algorithm>
 #include <cstring>
+#include <stdexcept>
+#include <string>
+#include <typeinfo>
 #include <utility>
 
 using namespace form::detail::experimental;
@@ -31,21 +34,21 @@ void Persistence::configureOutputItems(
 }
 
 void Persistence::createContainers(std::string const& creator,
-                                   std::map<std::string, std::string> const& products)
+                                   std::map<std::string, std::type_info const*> const& products)
 {
-  std::map<std::unique_ptr<Placement>, std::string> containers;
+  std::map<std::unique_ptr<Placement>, std::type_info const*> containers;
   for (auto const& [label, type] : products) {
     containers.insert(std::make_pair(getPlacement(creator, label), type));
   }
-  containers.insert(std::make_pair(getPlacement(creator, "index"), "std::string"));
+  containers.insert(std::make_pair(getPlacement(creator, "index"), &typeid(std::string)));
   m_store->createContainers(containers, m_tech_settings);
   return;
 }
 
 void Persistence::registerWrite(std::string const& creator,
                                 std::string const& label,
                                 void const* data,
-                                std::string const& type)
+                                std::type_info const& type)
 {
   std::unique_ptr<Placement> plcmnt = getPlacement(creator, label);
   m_store->fillContainer(*plcmnt, data, type);
@@ -55,7 +58,7 @@ void Persistence::registerWrite(std::string const& creator,
 void Persistence::commitOutput(std::string const& creator, std::string const& id)
 {
   std::unique_ptr<Placement> plcmnt = getPlacement(creator, "index");
-  m_store->fillContainer(*plcmnt, &id, "std::string");
+  m_store->fillContainer(*plcmnt, &id, typeid(std::string));
   m_store->commitContainers(*plcmnt);
   return;
 }
@@ -64,7 +67,7 @@ void Persistence::read(std::string const& creator,
                        std::string const& label,
                        std::string const& id,
                        void const** data,
-                       std::string& type)
+                       std::type_info const& type)
 {
   std::unique_ptr<Token> token = getToken(creator, label, id);
   m_store->readContainer(*token, data, type, m_tech_settings);

form/persistence/persistence.hpp

@@ -32,18 +32,18 @@ namespace form::detail::experimental {
       form::experimental::config::output_item_config const& output_items) override;
 
     void createContainers(std::string const& creator,
-                          std::map<std::string, std::string> const& products) override;
+                          std::map<std::string, std::type_info const*> const& products) override;
     void registerWrite(std::string const& creator,
                        std::string const& label,
                        void const* data,
-                       std::string const& type) override;
+                       std::type_info const& type) override;
     void commitOutput(std::string const& creator, std::string const& id) override;
 
     void read(std::string const& creator,
               std::string const& label,
               std::string const& id,
               void const** data,
-              std::string& type) override;
+              std::type_info const& type) override;
 
   private:
     std::unique_ptr<Placement> getPlacement(std::string const& creator, std::string const& label);

form/root_storage/root_tbranch_container.cpp

@@ -5,13 +5,28 @@
 #include "root_ttree_container.hpp"
 
 #include "TBranch.h"
+#include "TClassEdit.h"
 #include "TFile.h"
 #include "TLeaf.h"
 #include "TTree.h"
 
 #include <unordered_map>
 
 namespace {
+  // Return the demangled type name
+  std::string DemangleName(std::type_info const& type)
+  {
+    int errorCode;
+    // The TClassEdit version works on both linux and Windows.
+    char* demangledName = TClassEdit::DemangleTypeIdName(type, errorCode);
+    if (errorCode != 0) {
+      // NOTE: Instead of throwing, we could return the mangled name as a fallback.
+      throw std::runtime_error("Failed to demangle type name");
+    }
+    std::string result(demangledName);
+    std::free(demangledName);
+    return result;
+  }
   //Type name conversion based on https://root.cern.ch/doc/master/classTTree.html#ac1fa9466ce018d4aa739b357f981c615
   //An empty leaf list defaults to Float_t
   std::unordered_map<std::string, std::string> typeNameToLeafList = {{"int", "/I"},
@@ -63,16 +78,18 @@ void ROOT_TBranch_ContainerImp::setParent(std::shared_ptr<IStorage_Container> pa
   return;
 }
 
-void ROOT_TBranch_ContainerImp::setupWrite(std::string const& type)
+void ROOT_TBranch_ContainerImp::setupWrite(std::type_info const& type)
 {
   if (m_tree == nullptr) {
     throw std::runtime_error("ROOT_TBranch_ContainerImp::setupWrite no tree found");
   }
 
-  auto dictInfo = TDictionary::GetDictionary(type.c_str());
+  auto dictInfo = TDictionary::GetDictionary(type);
   if (m_branch == nullptr) {
     if (!dictInfo) {
-      throw std::runtime_error("ROOT_TBranch_ContainerImp::setupWrite unsupported type: " + type);
+      throw std::runtime_error(
+        std::string{"ROOT_TBranch_ContainerImp::setupWrite unsupported type: "} +
+        DemangleName(type));
     }
     if (dictInfo->Property() & EProperty::kIsFundamental) {
       m_branch = m_tree->Branch(col_name().c_str(),
@@ -117,7 +134,7 @@ void ROOT_TBranch_ContainerImp::commit()
   return;
 }
 
-bool ROOT_TBranch_ContainerImp::read(int id, void const** data, std::string& type)
+bool ROOT_TBranch_ContainerImp::read(int id, void const** data, std::type_info const& type)
 {
   if (m_tfile == nullptr) {
     throw std::runtime_error("ROOT_TBranch_ContainerImp::read no file attached");
@@ -138,23 +155,34 @@ bool ROOT_TBranch_ContainerImp::read(int id, void const** data, std::string& typ
     return false;
 
   void* branchBuffer = nullptr;
-  auto dictInfo = TClass::GetClass(type.c_str());
+  auto dictInfo = TDictionary::GetDictionary(type);
   int branchStatus = 0;
-  if (dictInfo) {
-    branchBuffer = dictInfo->New();
-    branchStatus = m_tree->SetBranchAddress(
-      col_name().c_str(), &branchBuffer, TClass::GetClass(type.c_str()), EDataType::kOther_t, true);
-  } else {
-    //Assume this is a fundamental type like int or double
-    auto fundInfo = static_cast<TDataType*>(TDictionary::GetDictionary(type.c_str()));
+  if (!dictInfo) {
+    throw std::runtime_error(std::string{"ROOT_TBranch_ContainerImp::read unsupported type: "} +
+                             DemangleName(type));
+  }
+
+  if (dictInfo->Property() & EProperty::kIsFundamental) {
+    auto fundInfo = static_cast<TDataType*>(dictInfo);
     branchBuffer = new char[fundInfo->Size()];
     branchStatus = m_tree->SetBranchAddress(
       col_name().c_str(), &branchBuffer, nullptr, EDataType(fundInfo->GetType()), true);
+  } else {
+    auto klass = TClass::GetClass(type);
+    if (!klass) {
+      throw std::runtime_error(std::string{"ROOT_TBranch_ContainerImp::read missing TClass"} +
+                               " (col_name='" + col_name() + "', type='" + DemangleName(type) +
+                               "')");
+    }
+    branchBuffer = klass->New();
+    branchStatus =
+      m_tree->SetBranchAddress(col_name().c_str(), &branchBuffer, klass, EDataType::kOther_t, true);
   }
 
   if (branchStatus < 0) {
     throw std::runtime_error(
-      "ROOT_TBranch_ContainerImp::read SetBranchAddress() failed with error code " +
+      std::string{"ROOT_TBranch_ContainerImp::read SetBranchAddress() failed"} + " (col_name='" +
+      col_name() + "', type='" + DemangleName(type) + "')" + " with error code " +
       std::to_string(branchStatus));
   }