From 802e307d39418c6e33810dc72e11b347fefe80cb Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Fri, 7 Aug 2026 11:33:04 -0500 Subject: [PATCH 01/11] redesign the technology for backends and move it from form/ to core/ --- form/core/technology.hpp | 66 ++++++++++++++++++++++++++++++++++++++++ form/form/technology.hpp | 27 ---------------- 2 files changed, 66 insertions(+), 27 deletions(-) create mode 100644 form/core/technology.hpp delete mode 100644 form/form/technology.hpp diff --git a/form/core/technology.hpp b/form/core/technology.hpp new file mode 100644 index 000000000..40b4bda6c --- /dev/null +++ b/form/core/technology.hpp @@ -0,0 +1,66 @@ +#ifndef FORM_CORE_TECHNOLOGY_HPP +#define FORM_CORE_TECHNOLOGY_HPP + +#include +#include +#include +#include + +namespace form::technology { + + // A storage technology, identified by a (family, variant) pair + + // Storage family (ROOT, HDF5, ...) + enum class Family : int { + unknown = 0, + root = 1, + hdf5 = 2, + }; + + // Variant in family + struct Id { + Family family{Family::unknown}; + int variant{0}; + + // Equality and ordering: ordering lets an Id be used as a std::map key + constexpr auto operator<=>(Id const&) const = default; + }; + + // Backends: valid (family, variant) pairs, stable numeric values as a future Token may persist them + inline constexpr Id ROOT_TTREE{Family::root, 1}; + inline constexpr Id ROOT_RNTUPLE{Family::root, 2}; + inline constexpr Id HDF5{Family::hdf5, 1}; + + // Canonical string -> technology mapping: the single place a technology string is parsed, replacing the copies that used to live in each module/source/test + inline Id from_string(std::string_view name) + { + if (name == "ROOT_TTREE") { + return ROOT_TTREE; + } + if (name == "ROOT_RNTUPLE") { + return ROOT_RNTUPLE; + } + if (name == "HDF5") { + return HDF5; + } + throw std::runtime_error("Unknown technology: " + std::string(name)); + } + + // Canonical technology -> string mapping + inline std::string to_string(Id tech) + { + if (tech == ROOT_TTREE) { + return "ROOT_TTREE"; + } + if (tech == ROOT_RNTUPLE) { + return "ROOT_RNTUPLE"; + } + if (tech == HDF5) { + return "HDF5"; + } + return "UNKNOWN"; + } + +} // namespace form::technology + +#endif // FORM_CORE_TECHNOLOGY_HPP diff --git a/form/form/technology.hpp b/form/form/technology.hpp deleted file mode 100644 index 01677eab8..000000000 --- a/form/form/technology.hpp +++ /dev/null @@ -1,27 +0,0 @@ -#ifndef FORM_FORM_TECHNOLOGY_HPP -#define FORM_FORM_TECHNOLOGY_HPP - -namespace form { - namespace technology { - // Helper constants - make these constexpr too - constexpr int ROOT_MAJOR = 1; - constexpr int ROOT_TTREE_MINOR = 1; - constexpr int ROOT_RNTUPLE_MINOR = 2; - constexpr int HDF5_MAJOR = 2; - - // Helper function for combining major/minor - constexpr int Combine(int major, int minor) { return (major * 256) + minor; } - - // Technology constants using the helper - constexpr int ROOT_TTREE = Combine(ROOT_MAJOR, ROOT_TTREE_MINOR); - constexpr int ROOT_RNTUPLE = Combine(ROOT_MAJOR, ROOT_RNTUPLE_MINOR); - constexpr int HDF5 = Combine(HDF5_MAJOR, 1); - - // Helper functions - inline int GetMajor(int tech) { return tech / 256; } - inline int GetMinor(int tech) { return tech % 256; } - } - -} // namespace form - -#endif // FORM_FORM_TECHNOLOGY_HPP From 85c489aca83dcf9897245e9ce4c892536dd121c2 Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Fri, 7 Aug 2026 12:20:28 -0500 Subject: [PATCH 02/11] redesign technology: replace packed-int encoding with typed form::technology::Id --- form/core/placement.cpp | 4 +-- form/core/placement.hpp | 8 +++-- form/core/technology.hpp | 6 ++-- form/core/token.cpp | 4 +-- form/core/token.hpp | 10 +++--- form/form/config.cpp | 6 ++-- form/form/config.hpp | 20 ++++++----- form/form_module.cpp | 23 +++---------- form/form_source.cpp | 14 ++------ form/storage/storage_reader.cpp | 4 +-- form/storage/storage_writer.cpp | 4 +-- form/util/factories.hpp | 42 +++++++++++------------ test/form/form_basics_test.cpp | 42 +++++++++++------------ test/form/form_root_schema_read_test.cpp | 2 +- test/form/form_root_schema_write_test.cpp | 2 +- test/form/form_storage_test.cpp | 15 ++++---- test/form/reader.cpp | 4 +-- test/form/test_utils.hpp | 24 ++++--------- test/form/writer.cpp | 4 +-- 19 files changed, 108 insertions(+), 130 deletions(-) diff --git a/form/core/placement.cpp b/form/core/placement.cpp index 8661ba246..8c5d5c1e1 100644 --- a/form/core/placement.cpp +++ b/form/core/placement.cpp @@ -7,7 +7,7 @@ using namespace form::detail::experimental; /// Constructor with initialization -Placement::Placement(std::string fileName, std::string containerName, int technology) : +Placement::Placement(std::string fileName, std::string containerName, technology::Id technology) : m_technology(technology), m_fileName(std::move(fileName)), m_containerName(std::move(containerName)) @@ -19,4 +19,4 @@ std::string const& Placement::fileName() const { return m_fileName; } /// Access container name std::string const& Placement::containerName() const { return m_containerName; } /// Access technology type -int Placement::technology() const { return m_technology; } +form::technology::Id Placement::technology() const { return m_technology; } diff --git a/form/core/placement.hpp b/form/core/placement.hpp index 95dc02ecb..be8360737 100644 --- a/form/core/placement.hpp +++ b/form/core/placement.hpp @@ -3,6 +3,8 @@ #ifndef FORM_CORE_PLACEMENT_HPP #define FORM_CORE_PLACEMENT_HPP +#include "core/technology.hpp" + #include /* @class Placement @@ -16,18 +18,18 @@ namespace form::detail::experimental { Placement() = default; /// Constructor with initialization - Placement(std::string fileName, std::string containerName, int technology); + Placement(std::string fileName, std::string containerName, technology::Id technology); /// Access file name std::string const& fileName() const; /// Access container name std::string const& containerName() const; /// Access technology type - int technology() const; + technology::Id technology() const; private: /// Technology identifier - int m_technology{}; + technology::Id m_technology{}; /// File name std::string m_fileName; /// Container name diff --git a/form/core/technology.hpp b/form/core/technology.hpp index 40b4bda6c..b66d2095f 100644 --- a/form/core/technology.hpp +++ b/form/core/technology.hpp @@ -6,17 +6,17 @@ #include #include +/* A storage technology, identified by a (family, variant) pair */ + namespace form::technology { - // A storage technology, identified by a (family, variant) pair - // Storage family (ROOT, HDF5, ...) enum class Family : int { unknown = 0, root = 1, hdf5 = 2, }; - + // Variant in family struct Id { Family family{Family::unknown}; diff --git a/form/core/token.cpp b/form/core/token.cpp index 348681430..dfd7529ff 100644 --- a/form/core/token.cpp +++ b/form/core/token.cpp @@ -7,7 +7,7 @@ using namespace form::detail::experimental; /// Constructor with initialization -Token::Token(std::string fileName, std::string containerName, int technology, int id) : +Token::Token(std::string fileName, std::string containerName, technology::Id technology, int id) : m_technology(technology), m_fileName(std::move(fileName)), m_containerName(std::move(containerName)), @@ -20,7 +20,7 @@ std::string const& Token::fileName() const { return m_fileName; } /// Access container name std::string const& Token::containerName() const { return m_containerName; } /// Access technology type -int Token::technology() const { return m_technology; } +form::technology::Id Token::technology() const { return m_technology; } /// Set technology type /// Access identifier/entry number int Token::id() const { return m_id; } diff --git a/form/core/token.hpp b/form/core/token.hpp index 0070a201d..821ef181a 100644 --- a/form/core/token.hpp +++ b/form/core/token.hpp @@ -3,6 +3,8 @@ #ifndef FORM_CORE_TOKEN_HPP #define FORM_CORE_TOKEN_HPP +#include "core/technology.hpp" + #include /* @class Token @@ -12,24 +14,24 @@ namespace form::detail::experimental { class Token { public: /// Default constructor; delegates to the named constructor so the -1 sentinel for id is defined once - Token() : Token("", "", 0) {} + Token() : Token("", "", {}) {} /// Named constructor; id defaults to -1 as a "not set" sentinel - Token(std::string fileName, std::string containerName, int technology, int id = -1); + Token(std::string fileName, std::string containerName, technology::Id technology, int id = -1); /// Access file name std::string const& fileName() const; /// Access container name std::string const& containerName() const; /// Access technology type - int technology() const; + technology::Id technology() const; /// Access identifier/entry number int id() const; private: /// Technology identifier - int m_technology; + technology::Id m_technology; /// File name std::string m_fileName; /// Container name diff --git a/form/form/config.cpp b/form/form/config.cpp index 3c5d98ca7..0fad4ea24 100644 --- a/form/form/config.cpp +++ b/form/form/config.cpp @@ -16,7 +16,7 @@ namespace form::experimental::config { void ItemConfig::addItem(std::string const& product_name, std::string const& file_name, - int technology) + technology::Id technology) { m_items.emplace_back(product_name, file_name, technology); } @@ -31,7 +31,7 @@ namespace form::experimental::config { return std::nullopt; } - tech_setting_config::table_t tech_setting_config::getFileTable(int const technology, + tech_setting_config::table_t tech_setting_config::getFileTable(technology::Id const technology, std::string const& fileName) const { auto const per_tech = ::const_lookup(file_settings, technology); @@ -39,7 +39,7 @@ namespace form::experimental::config { } tech_setting_config::table_t tech_setting_config::getContainerTable( - int const technology, std::string const& containerName) const + technology::Id const technology, std::string const& containerName) const { auto const per_tech = ::const_lookup(container_settings, technology); return ::const_lookup(per_tech, containerName); diff --git a/form/form/config.hpp b/form/form/config.hpp index 45972cd13..15c421c39 100644 --- a/form/form/config.hpp +++ b/form/form/config.hpp @@ -1,6 +1,8 @@ #ifndef FORM_FORM_CONFIG_HPP #define FORM_FORM_CONFIG_HPP +#include "core/technology.hpp" + #include #include #include @@ -12,13 +14,13 @@ namespace form::experimental::config { struct PersistenceItem { - std::string product_name; // e.g. "trackStart", "trackNumberHits" - std::string file_name; // e.g. "toy.root", "output.hdf5" - int technology{}; // Technology::ROOT_TTREE, Technology::ROOT_RNTUPLE, Technology::HDF5 + std::string product_name; // e.g. "trackStart", "trackNumberHits" + std::string file_name; // e.g. "toy.root", "output.hdf5" + technology::Id technology{}; // technology::ROOT_TTREE, ROOT_RNTUPLE, HDF5 PersistenceItem() = default; - PersistenceItem(std::string product, std::string file, int tech) : + PersistenceItem(std::string product, std::string file, technology::Id tech) : product_name(std::move(product)), file_name(std::move(file)), technology(tech) { } @@ -30,7 +32,9 @@ namespace form::experimental::config { ~ItemConfig() = default; // Add a configuration item - void addItem(std::string const& product_name, std::string const& file_name, int technology); + void addItem(std::string const& product_name, + std::string const& file_name, + technology::Id technology); // Find configuration for a product+creator combination std::optional findItem(std::string const& product_name) const; @@ -44,12 +48,12 @@ namespace form::experimental::config { struct tech_setting_config { using table_t = std::vector>; - using map_t = std::map>; + using map_t = std::map>; map_t file_settings; map_t container_settings; - table_t getFileTable(int technology, std::string const& fileName) const; - table_t getContainerTable(int technology, std::string const& containerName) const; + table_t getFileTable(technology::Id technology, std::string const& fileName) const; + table_t getContainerTable(technology::Id technology, std::string const& containerName) const; }; } // namespace form::experimental::config diff --git a/form/form_module.cpp b/form/form_module.cpp index 080e96615..0a203ee43 100644 --- a/form/form_module.cpp +++ b/form/form_module.cpp @@ -4,29 +4,27 @@ // FORM headers - these need to be available via CMake configuration // need to set up the build system to find these headers +#include "core/technology.hpp" #include "form/config.hpp" #include "form/form_writer.hpp" -#include "form/technology.hpp" #include #include #include #include -#include -#include namespace { class FormOutputModule { public: FormOutputModule(std::string output_file, - int technology, + form::technology::Id technology, std::vector const& products_to_save) : m_output_file(std::move(output_file)), m_technology(technology) { std::cout << "FormOutputModule initialized\n"; std::cout << " Output file: " << m_output_file << "\n"; - std::cout << " Technology: " << m_technology << "\n"; + std::cout << " Technology: " << form::technology::to_string(m_technology) << "\n"; // Build FORM configuration form::experimental::config::ItemConfig output_cfg; @@ -104,7 +102,7 @@ namespace { // Algorithm configuration fixed at construction; intentionally immutable for object lifetime. // NOLINTBEGIN(cppcoreguidelines-avoid-const-or-ref-data-members) std::string const m_output_file; - int const m_technology; + form::technology::Id const m_technology; // NOLINTEND(cppcoreguidelines-avoid-const-or-ref-data-members) std::unique_ptr m_form_interface; }; @@ -123,18 +121,7 @@ PHLEX_REGISTER_ALGORITHMS(m, config) std::cout << " output_file: " << output_file << "\n"; std::cout << " technology: " << tech_string << "\n"; - std::unordered_map const tech_lookup = { - {"ROOT_TTREE", form::technology::ROOT_TTREE}, - {"ROOT_RNTUPLE", form::technology::ROOT_RNTUPLE}, - {"HDF5", form::technology::HDF5}}; - - auto it = tech_lookup.find(tech_string); - - if (it == tech_lookup.end()) { - throw std::runtime_error("Unknown technology: " + tech_string); - } - - int const technology = it->second; + auto const technology = form::technology::from_string(tech_string); auto products_to_save = config.get>("products"); diff --git a/form/form_source.cpp b/form/form_source.cpp index b2c84926f..60e211ff0 100644 --- a/form/form_source.cpp +++ b/form/form_source.cpp @@ -1,9 +1,9 @@ #include "phlex/source.hpp" +#include "core/technology.hpp" #include "form/config.hpp" #include "form/form_reader.hpp" #include "form/form_source_type_registry.hpp" -#include "form/technology.hpp" #include "phlex/model/data_cell_index.hpp" @@ -13,7 +13,6 @@ #include #include #include -#include #include #include @@ -176,16 +175,7 @@ PHLEX_REGISTER_SOURCE(s, config) actual_creator = *plugin + ":" + *algorithm; } - std::unordered_map const tech_lookup = { - {"ROOT_TTREE", form::technology::ROOT_TTREE}, - {"ROOT_RNTUPLE", form::technology::ROOT_RNTUPLE}, - {"HDF5", form::technology::HDF5}}; - - auto it = tech_lookup.find(tech_string); - if (it == tech_lookup.end()) { - throw std::runtime_error("Unknown technology: " + tech_string); - } - int const technology = it->second; + auto const technology = form::technology::from_string(tech_string); form::experimental::config::ItemConfig input_cfg; form::experimental::config::tech_setting_config tech_cfg; diff --git a/form/storage/storage_reader.cpp b/form/storage/storage_reader.cpp index 6f9f11b07..cedd62308 100644 --- a/form/storage/storage_reader.cpp +++ b/form/storage/storage_reader.cpp @@ -17,7 +17,7 @@ using namespace form::detail::experimental; namespace { form::experimental::config::tech_setting_config::table_t get_file_table( form::experimental::config::tech_setting_config const& settings, - int technology, + form::technology::Id technology, std::string const& file_name) { auto const per_tech = settings.file_settings.find(technology); @@ -33,7 +33,7 @@ namespace { form::experimental::config::tech_setting_config::table_t get_container_table( form::experimental::config::tech_setting_config const& settings, - int technology, + form::technology::Id technology, std::string const& container_name) { auto const per_tech = settings.container_settings.find(technology); diff --git a/form/storage/storage_writer.cpp b/form/storage/storage_writer.cpp index 0b9755977..344919c4b 100644 --- a/form/storage/storage_writer.cpp +++ b/form/storage/storage_writer.cpp @@ -12,7 +12,7 @@ using namespace form::detail::experimental; namespace { form::experimental::config::tech_setting_config::table_t get_file_table( form::experimental::config::tech_setting_config const& settings, - int technology, + form::technology::Id technology, std::string const& file_name) { auto const per_tech = settings.file_settings.find(technology); @@ -28,7 +28,7 @@ namespace { form::experimental::config::tech_setting_config::table_t get_container_table( form::experimental::config::tech_setting_config const& settings, - int technology, + form::technology::Id technology, std::string const& container_name) { auto const per_tech = settings.container_settings.find(technology); diff --git a/form/util/factories.hpp b/form/util/factories.hpp index 48b74bb9a..93a2524f4 100644 --- a/form/util/factories.hpp +++ b/form/util/factories.hpp @@ -3,7 +3,7 @@ #ifndef FORM_UTIL_FACTORIES_HPP #define FORM_UTIL_FACTORIES_HPP -#include "form/technology.hpp" +#include "core/technology.hpp" #include "storage/istorage.hpp" #include "storage/storage_file.hpp" @@ -29,33 +29,35 @@ namespace form::detail::experimental { - inline std::shared_ptr createFile(int tech, std::string const& name, char mode) + inline std::shared_ptr createFile(form::technology::Id tech, + std::string const& name, + char mode) { - if (form::technology::GetMajor(tech) == form::technology::ROOT_MAJOR) { + if (tech.family == form::technology::Family::root) { #ifdef USE_ROOT_STORAGE return std::make_shared(name, mode); #endif - } else if (form::technology::GetMajor(tech) == form::technology::HDF5_MAJOR) { + } else if (tech.family == form::technology::Family::hdf5) { // Handle HDF5 file creation when implemented // return std::make_shared(name, mode); } return std::make_shared(name, mode); } - inline std::shared_ptr createWriteAssociation(int tech, + inline std::shared_ptr createWriteAssociation(form::technology::Id tech, std::string const& name) { - if (form::technology::GetMajor(tech) == form::technology::ROOT_MAJOR) { - if (form::technology::GetMinor(tech) == form::technology::ROOT_TTREE_MINOR) { + if (tech.family == form::technology::Family::root) { + if (tech == form::technology::ROOT_TTREE) { #ifdef USE_ROOT_STORAGE return std::make_shared(name); #endif // USE_ROOT_STORAGE - } else if (form::technology::GetMinor(tech) == form::technology::ROOT_RNTUPLE_MINOR) { + } else if (tech == form::technology::ROOT_RNTUPLE) { #ifdef USE_RNTUPLE_STORAGE return std::make_shared(name); #endif // USE_RNTUPLE_STORAGE } - } else if (form::technology::GetMajor(tech) == form::technology::HDF5_MAJOR) { + } else if (tech.family == form::technology::Family::hdf5) { #ifdef USE_HDF5_STORAGE // Add HDF5 implementation when available // return std::make_shared(name); @@ -66,21 +68,20 @@ namespace form::detail::experimental { return std::make_shared(name); } - inline std::shared_ptr createReadContainer(int tech, + inline std::shared_ptr createReadContainer(form::technology::Id tech, std::string const& name) { - // Use the helper functions from Technology namespace for consistency - if (form::technology::GetMajor(tech) == form::technology::ROOT_MAJOR) { - if (form::technology::GetMinor(tech) == form::technology::ROOT_TTREE_MINOR) { + if (tech.family == form::technology::Family::root) { + if (tech == form::technology::ROOT_TTREE) { #ifdef USE_ROOT_STORAGE return std::make_shared(name); #endif // USE_ROOT_STORAGE - } else if (form::technology::GetMinor(tech) == form::technology::ROOT_RNTUPLE_MINOR) { + } else if (tech == form::technology::ROOT_RNTUPLE) { #ifdef USE_RNTUPLE_STORAGE return std::make_shared(name); #endif // USE_RNTUPLE_STORAGE } - } else if (form::technology::GetMajor(tech) == form::technology::HDF5_MAJOR) { + } else if (tech.family == form::technology::Family::hdf5) { #ifdef USE_HDF5_STORAGE // Add HDF5 implementation when available // return std::make_shared(name); @@ -91,21 +92,20 @@ namespace form::detail::experimental { return std::make_shared(name); } - inline std::shared_ptr createWriteContainer(int tech, + inline std::shared_ptr createWriteContainer(form::technology::Id tech, std::string const& name) { - // Use the helper functions from Technology namespace for consistency - if (form::technology::GetMajor(tech) == form::technology::ROOT_MAJOR) { - if (form::technology::GetMinor(tech) == form::technology::ROOT_TTREE_MINOR) { + if (tech.family == form::technology::Family::root) { + if (tech == form::technology::ROOT_TTREE) { #ifdef USE_ROOT_STORAGE return std::make_shared(name); #endif // USE_ROOT_STORAGE - } else if (form::technology::GetMinor(tech) == form::technology::ROOT_RNTUPLE_MINOR) { + } else if (tech == form::technology::ROOT_RNTUPLE) { #ifdef USE_RNTUPLE_STORAGE return std::make_shared(name); #endif // USE_RNTUPLE_STORAGE } - } else if (form::technology::GetMajor(tech) == form::technology::HDF5_MAJOR) { + } else if (tech.family == form::technology::Family::hdf5) { #ifdef USE_HDF5_STORAGE // Add HDF5 implementation when available // return std::make_shared(name); diff --git a/test/form/form_basics_test.cpp b/test/form/form_basics_test.cpp index 3087c4f8f..d76120aae 100644 --- a/test/form/form_basics_test.cpp +++ b/test/form/form_basics_test.cpp @@ -1,9 +1,9 @@ +#include "core/technology.hpp" #include "core/token.hpp" #include "form/config.hpp" #include "form/form_reader.hpp" #include "form/form_source_type_registry.hpp" #include "form/form_writer.hpp" -#include "form/technology.hpp" #include "persistence/persistence_reader.hpp" #include "persistence/persistence_writer.hpp" #include "storage/istorage.hpp" @@ -26,7 +26,7 @@ TEST_CASE("Token default constructor", "[form]") Token t; CHECK(t.fileName().empty()); CHECK(t.containerName().empty()); - CHECK(t.technology() == 0); + CHECK(t.technology() == form::technology::Id{}); // Default-constructed token must carry the -1 sentinel for id CHECK(t.id() == -1); } @@ -123,16 +123,16 @@ TEST_CASE("Storage_Associative_Write_Container basics", "[form]") TEST_CASE("Factories fallback", "[form]") { - auto f = createFile(0, "test.root", 'o'); + auto f = createFile(form::technology::Id{}, "test.root", 'o'); CHECK(dynamic_cast(f.get()) != nullptr); - auto rc = createReadContainer(0, "cont"); + auto rc = createReadContainer(form::technology::Id{}, "cont"); CHECK(dynamic_cast(rc.get()) != nullptr); - auto wa = createWriteAssociation(0, "assoc"); + auto wa = createWriteAssociation(form::technology::Id{}, "assoc"); CHECK(dynamic_cast(wa.get()) != nullptr); - auto wc = createWriteContainer(0, "cont"); + auto wc = createWriteContainer(form::technology::Id{}, "cont"); CHECK(dynamic_cast(wc.get()) != nullptr); } @@ -143,7 +143,7 @@ TEST_CASE("StorageReader basic operations", "[form]") form::experimental::config::tech_setting_config settings; - Token token("file.root", "cont", 0, 1); + Token token("file.root", "cont", form::technology::Id{}, 1); void const* read_data = nullptr; storage->readContainer(token, &read_data, typeid(int), settings); @@ -159,12 +159,12 @@ TEST_CASE("StorageWriter basic operations", "[form]") form::experimental::config::tech_setting_config settings; std::map, std::type_info const*> containers; - auto p = std::make_unique("file.root", "cont", 0); + auto p = std::make_unique("file.root", "cont", form::technology::Id{}); containers.emplace(std::move(p), &typeid(int)); storage->createContainers(containers, settings); - Placement p2("file.root", "cont", 0); + Placement p2("file.root", "cont", form::technology::Id{}); int data = 42; storage->fillContainer(p2, &data, typeid(int)); storage->commitContainers(p2); @@ -177,8 +177,8 @@ TEST_CASE("PersistenceReader basic operations", "[form]") using namespace form::experimental::config; ItemConfig out_cfg; - out_cfg.addItem("prod", "file.root", 0); - out_cfg.addItem("parent/child", "file.root", 0); + out_cfg.addItem("prod", "file.root", form::technology::Id{}); + out_cfg.addItem("parent/child", "file.root", form::technology::Id{}); p->configure(out_cfg); tech_setting_config tech_cfg; @@ -199,8 +199,8 @@ TEST_CASE("PersistenceWriter basic operations", "[form]") using namespace form::experimental::config; ItemConfig out_cfg; - out_cfg.addItem("prod", "file.root", 0); - out_cfg.addItem("parent/child", "file.root", 0); + out_cfg.addItem("prod", "file.root", form::technology::Id{}); + out_cfg.addItem("parent/child", "file.root", form::technology::Id{}); p->configure(out_cfg); tech_setting_config tech_cfg; @@ -221,7 +221,7 @@ TEST_CASE("form::experimental::config tests", "[form]") SECTION("ItemConfig") { ItemConfig cfg; - cfg.addItem("prod1", "file1.root", 1); + cfg.addItem("prod1", "file1.root", form::technology::ROOT_TTREE); auto item = cfg.findItem("prod1"); REQUIRE(item); @@ -234,15 +234,15 @@ TEST_CASE("form::experimental::config tests", "[form]") SECTION("tech_setting_config") { tech_setting_config cfg; - cfg.file_settings[1]["file1.root"] = {{"attr", "val"}}; - cfg.container_settings[1]["cont1"] = {{"cattr", "cval"}}; + cfg.file_settings[form::technology::ROOT_TTREE]["file1.root"] = {{"attr", "val"}}; + cfg.container_settings[form::technology::ROOT_TTREE]["cont1"] = {{"cattr", "cval"}}; - auto ftable = cfg.getFileTable(1, "file1.root"); + auto ftable = cfg.getFileTable(form::technology::ROOT_TTREE, "file1.root"); REQUIRE(ftable.size() == 1); CHECK(ftable[0].first == "attr"); CHECK(ftable[0].second == "val"); - auto ctable = cfg.getContainerTable(1, "cont1"); + auto ctable = cfg.getContainerTable(form::technology::ROOT_TTREE, "cont1"); REQUIRE(ctable.size() == 1); CHECK(ctable[0].first == "cattr"); CHECK(ctable[0].second == "cval"); @@ -298,7 +298,7 @@ TEST_CASE("form_reader_interface::indices exercises persistence listIndices path using namespace form::experimental::config; ItemConfig cfg; - cfg.addItem("prod", "dummy_reader_test.root", 0); + cfg.addItem("prod", "dummy_reader_test.root", form::technology::Id{}); form::experimental::form_reader_interface reader{cfg, tech_setting_config{}}; // indices() calls persistence listIndices; with tech=0 the index container is @@ -311,7 +311,7 @@ TEST_CASE("form_reader_interface::read throws for missing product config", "[for using namespace form::experimental::config; ItemConfig cfg; - cfg.addItem("prod", "dummy_reader_test.root", 0); + cfg.addItem("prod", "dummy_reader_test.root", form::technology::Id{}); form::experimental::form_reader_interface reader{cfg, tech_setting_config{}}; form::experimental::product_with_name product{"missing", nullptr, &typeid(int)}; @@ -323,7 +323,7 @@ TEST_CASE("form_writer_interface handles missing product config without crashing using namespace form::experimental::config; ItemConfig cfg; - cfg.addItem("prod", "dummy_writer_test.root", 0); + cfg.addItem("prod", "dummy_writer_test.root", form::technology::Id{}); form::experimental::form_writer_interface writer{cfg, tech_setting_config{}}; form::experimental::product_with_name product{"missing", nullptr, &typeid(int)}; diff --git a/test/form/form_root_schema_read_test.cpp b/test/form/form_root_schema_read_test.cpp index 15c27f58b..c860ccb34 100644 --- a/test/form/form_root_schema_read_test.cpp +++ b/test/form/form_root_schema_read_test.cpp @@ -14,7 +14,7 @@ int main(int const argc, char const** argv) std::string const tech_string = (argc > 1) ? argv[1] : "ROOT_TTREE"; try { - int const technology = getTechnology(tech_string); + auto const technology = getTechnology(tech_string); auto const& [prods] = read>(technology); std::ofstream outFile("form_root_schema_read_log_" + tech_string + ".txt"); diff --git a/test/form/form_root_schema_write_test.cpp b/test/form/form_root_schema_write_test.cpp index 6cca0fc8c..7e3d73614 100644 --- a/test/form/form_root_schema_write_test.cpp +++ b/test/form/form_root_schema_write_test.cpp @@ -13,7 +13,7 @@ using namespace form::test; int main(int const argc, char const** argv) { std::string const tech_string = (argc > 1) ? argv[1] : "ROOT_TTREE"; - int const technology = getTechnology(tech_string); + auto const technology = getTechnology(tech_string); ToyTracker tracker(4 * 1024); std::vector const prods = tracker(); diff --git a/test/form/form_storage_test.cpp b/test/form/form_storage_test.cpp index bad1d3bff..b5704ff47 100644 --- a/test/form/form_storage_test.cpp +++ b/test/form/form_storage_test.cpp @@ -21,7 +21,7 @@ using namespace form::detail::experimental; namespace { // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables) - int technology = form::technology::ROOT_TTREE; //Potentially overridden in main + form::technology::Id technology = form::technology::ROOT_TTREE; //Potentially overridden in main //Non-const global variable required by limitations of Catch2 } @@ -283,7 +283,8 @@ TEST_CASE("Persistence round-trip: structured index normalization and listing", { using namespace form::experimental::config; - std::string const file_name = "persistence_roundtrip_" + std::to_string(technology) + ".root"; + std::string const file_name = + "persistence_roundtrip_" + form::technology::to_string(technology) + ".root"; std::string const creator = "norm_creator"; ItemConfig cfg; @@ -332,7 +333,8 @@ TEST_CASE("Persistence round-trip: all-zero structured id fallback", "[form]") { using namespace form::experimental::config; - std::string const file_name = "persistence_zero_index_" + std::to_string(technology) + ".root"; + std::string const file_name = + "persistence_zero_index_" + form::technology::to_string(technology) + ".root"; std::string const creator = "zero_creator"; ItemConfig cfg; @@ -367,7 +369,7 @@ TEST_CASE("StorageReader getIndex: malformed ids and compatibility fallbacks", " using namespace form::experimental::config; std::string const file_name = - "storage_reader_index_branches_" + std::to_string(technology) + ".root"; + "storage_reader_index_branches_" + form::technology::to_string(technology) + ".root"; std::string const creator = "storage_reader_creator"; std::string const index_container = creator + "/index"; @@ -420,7 +422,7 @@ TEST_CASE("StorageReader getIndex: empty container and tech-table branches", "[f std::runtime_error); std::string const file_name = - "storage_reader_getindex_attr_" + std::to_string(technology) + ".root"; + "storage_reader_getindex_attr_" + form::technology::to_string(technology) + ".root"; std::string const creator = "storage_reader_getindex_attr_creator"; ItemConfig cfg; cfg.addItem("prod", file_name, technology); @@ -450,7 +452,8 @@ TEST_CASE("StorageReader prime/listIndices/readContainer: attribute and error br { using namespace form::experimental::config; - std::string const file_name = "storage_reader_misc_attr_" + std::to_string(technology) + ".root"; + std::string const file_name = + "storage_reader_misc_attr_" + form::technology::to_string(technology) + ".root"; std::string const creator = "storage_reader_misc_creator"; ItemConfig cfg; cfg.addItem("prod", file_name, technology); diff --git a/test/form/reader.cpp b/test/form/reader.cpp index 8714551c3..8710d7772 100644 --- a/test/form/reader.cpp +++ b/test/form/reader.cpp @@ -1,8 +1,8 @@ // Copyright (C) 2025 ... +#include "core/technology.hpp" #include "data_products/track_start.hpp" #include "form/form_reader.hpp" -#include "form/technology.hpp" #include "test_helpers.hpp" #include "test_utils.hpp" @@ -36,7 +36,7 @@ int main(int argc, char** argv) std::string const filename = (argc > 1) ? argv[1] : "toy.root"; std::string const checksum_filename = (argc > 2) ? argv[2] : "toy_checksums.txt"; - int const technology = form::test::getTechnology((argc > 3) ? argv[3] : "ROOT_TTREE"); + auto const technology = form::test::getTechnology((argc > 3) ? argv[3] : "ROOT_TTREE"); // Load expected checksums from file std::map, SegChecksum> expected_seg; diff --git a/test/form/test_utils.hpp b/test/form/test_utils.hpp index c33c26834..89bbbf2e3 100644 --- a/test/form/test_utils.hpp +++ b/test/form/test_utils.hpp @@ -39,7 +39,7 @@ namespace form::test { inline std::vector> doWrite( std::shared_ptr& /*file*/, - int const /*technology*/, + form::technology::Id const /*technology*/, std::shared_ptr& /*parent*/) { return {}; @@ -48,7 +48,7 @@ namespace form::test { template inline std::vector> doWrite( std::shared_ptr& file, - int const technology, + form::technology::Id const technology, std::shared_ptr& parent, PROD& prod, PRODS&... prods) @@ -69,7 +69,7 @@ namespace form::test { } template - inline void write(int const technology, PRODS&... prods) + inline void write(form::technology::Id const technology, PRODS&... prods) { auto file = createFile(technology, std::string(testFileName), 'o'); auto parent = createWriteAssociation(technology, std::string(testTreeName)); @@ -83,7 +83,7 @@ namespace form::test { template inline std::unique_ptr doRead(std::shared_ptr& file, - int const technology) + form::technology::Id const technology) { auto container = createReadContainer(technology, makeTestBranchName()); container->setFile(file); @@ -97,26 +97,16 @@ namespace form::test { } template - inline std::tuple...> read(int const technology) + inline std::tuple...> read(form::technology::Id const technology) { auto file = createFile(technology, std::string(testFileName), 'i'); return std::make_tuple(doRead(file, technology)...); } - inline int getTechnology(std::string const& tech_string) + inline form::technology::Id getTechnology(std::string const& tech_string) { - std::unordered_map const tech_lookup = { - {"ROOT_TTREE", form::technology::ROOT_TTREE}, - {"ROOT_RNTUPLE", form::technology::ROOT_RNTUPLE}, - {"HDF5", form::technology::HDF5}}; - - auto const it = tech_lookup.find(tech_string); - if (it == tech_lookup.end()) { - throw std::runtime_error("Unknown technology: " + tech_string); - } - - return it->second; + return form::technology::from_string(tech_string); } } // namespace form::test diff --git a/test/form/writer.cpp b/test/form/writer.cpp index bfe2a3e9e..e20d5e870 100644 --- a/test/form/writer.cpp +++ b/test/form/writer.cpp @@ -1,8 +1,8 @@ // Copyright (C) 2025 ... +#include "core/technology.hpp" #include "data_products/track_start.hpp" #include "form/form_writer.hpp" -#include "form/technology.hpp" #include "test_helpers.hpp" #include "test_utils.hpp" #include "toy_tracker.hpp" @@ -48,7 +48,7 @@ int main(int argc, char** argv) std::string const filename = (argc > 1) ? argv[1] : "toy.root"; std::string const checksum_filename = (argc > 2) ? argv[2] : "toy_checksums.txt"; - int const technology = form::test::getTechnology((argc > 3) ? argv[3] : "ROOT_TTREE"); + auto const technology = form::test::getTechnology((argc > 3) ? argv[3] : "ROOT_TTREE"); // TODO: Read configuration from config file instead of hardcoding form::experimental::config::ItemConfig config_items; From e8e4d1adc6e3e61961b7dbebe2e023bf012e7f42 Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Fri, 7 Aug 2026 14:28:56 -0500 Subject: [PATCH 03/11] form: replace packed-int technolgy with typed technology::Id (major/minor) --- form/core/technology.hpp | 22 +++++++++++----------- form/util/factories.hpp | 16 ++++++++-------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/form/core/technology.hpp b/form/core/technology.hpp index b66d2095f..f530346e1 100644 --- a/form/core/technology.hpp +++ b/form/core/technology.hpp @@ -6,30 +6,30 @@ #include #include -/* A storage technology, identified by a (family, variant) pair */ +/* A storage technology, identified by a (major, minor) pair */ namespace form::technology { - // Storage family (ROOT, HDF5, ...) - enum class Family : int { + // Major storage type (ROOT, HDF5, ...) + enum class Major : int { unknown = 0, root = 1, hdf5 = 2, }; - // Variant in family + // Minor variant within a Major (e.g. TTree vs RNTuple within ROOT) struct Id { - Family family{Family::unknown}; - int variant{0}; + Major major{Major::unknown}; + int minor{0}; - // Equality and ordering: ordering lets an Id be used as a std::map key + // Exact ordering over (major, minor): lets an Id be a std::map key and drives backend dispatch constexpr auto operator<=>(Id const&) const = default; }; - // Backends: valid (family, variant) pairs, stable numeric values as a future Token may persist them - inline constexpr Id ROOT_TTREE{Family::root, 1}; - inline constexpr Id ROOT_RNTUPLE{Family::root, 2}; - inline constexpr Id HDF5{Family::hdf5, 1}; + // Backends: valid (major, minor) pairs, stable numeric values as a future Token may persist them + inline constexpr Id ROOT_TTREE{Major::root, 1}; + inline constexpr Id ROOT_RNTUPLE{Major::root, 2}; + inline constexpr Id HDF5{Major::hdf5, 1}; // Canonical string -> technology mapping: the single place a technology string is parsed, replacing the copies that used to live in each module/source/test inline Id from_string(std::string_view name) diff --git a/form/util/factories.hpp b/form/util/factories.hpp index 93a2524f4..cdf3da95d 100644 --- a/form/util/factories.hpp +++ b/form/util/factories.hpp @@ -33,11 +33,11 @@ namespace form::detail::experimental { std::string const& name, char mode) { - if (tech.family == form::technology::Family::root) { + if (tech.major == form::technology::Major::root) { #ifdef USE_ROOT_STORAGE return std::make_shared(name, mode); #endif - } else if (tech.family == form::technology::Family::hdf5) { + } else if (tech.major == form::technology::Major::hdf5) { // Handle HDF5 file creation when implemented // return std::make_shared(name, mode); } @@ -47,7 +47,7 @@ namespace form::detail::experimental { inline std::shared_ptr createWriteAssociation(form::technology::Id tech, std::string const& name) { - if (tech.family == form::technology::Family::root) { + if (tech.major == form::technology::Major::root) { if (tech == form::technology::ROOT_TTREE) { #ifdef USE_ROOT_STORAGE return std::make_shared(name); @@ -57,7 +57,7 @@ namespace form::detail::experimental { return std::make_shared(name); #endif // USE_RNTUPLE_STORAGE } - } else if (tech.family == form::technology::Family::hdf5) { + } else if (tech.major == form::technology::Major::hdf5) { #ifdef USE_HDF5_STORAGE // Add HDF5 implementation when available // return std::make_shared(name); @@ -71,7 +71,7 @@ namespace form::detail::experimental { inline std::shared_ptr createReadContainer(form::technology::Id tech, std::string const& name) { - if (tech.family == form::technology::Family::root) { + if (tech.major == form::technology::Major::root) { if (tech == form::technology::ROOT_TTREE) { #ifdef USE_ROOT_STORAGE return std::make_shared(name); @@ -81,7 +81,7 @@ namespace form::detail::experimental { return std::make_shared(name); #endif // USE_RNTUPLE_STORAGE } - } else if (tech.family == form::technology::Family::hdf5) { + } else if (tech.major == form::technology::Major::hdf5) { #ifdef USE_HDF5_STORAGE // Add HDF5 implementation when available // return std::make_shared(name); @@ -95,7 +95,7 @@ namespace form::detail::experimental { inline std::shared_ptr createWriteContainer(form::technology::Id tech, std::string const& name) { - if (tech.family == form::technology::Family::root) { + if (tech.major == form::technology::Major::root) { if (tech == form::technology::ROOT_TTREE) { #ifdef USE_ROOT_STORAGE return std::make_shared(name); @@ -105,7 +105,7 @@ namespace form::detail::experimental { return std::make_shared(name); #endif // USE_RNTUPLE_STORAGE } - } else if (tech.family == form::technology::Family::hdf5) { + } else if (tech.major == form::technology::Major::hdf5) { #ifdef USE_HDF5_STORAGE // Add HDF5 implementation when available // return std::make_shared(name); From 72b0377b51700120fd26abe451474f4c059876bc Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Fri, 7 Aug 2026 15:56:08 -0500 Subject: [PATCH 04/11] add test to increase coverage and throw 'Technology 'HDF5' is recognized but not yet implemented' --- form/core/technology.hpp | 3 ++- form/util/factories.hpp | 19 ++++---------- test/form/form_basics_test.cpp | 46 ++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 15 deletions(-) diff --git a/form/core/technology.hpp b/form/core/technology.hpp index f530346e1..291cb9b25 100644 --- a/form/core/technology.hpp +++ b/form/core/technology.hpp @@ -41,7 +41,8 @@ namespace form::technology { return ROOT_RNTUPLE; } if (name == "HDF5") { - return HDF5; + // HDF5 is a reserved technology but has no backend yet: reject it at parse time + throw std::runtime_error("Technology 'HDF5' is recognized but not yet implemented"); } throw std::runtime_error("Unknown technology: " + std::string(name)); } diff --git a/form/util/factories.hpp b/form/util/factories.hpp index cdf3da95d..9b102529b 100644 --- a/form/util/factories.hpp +++ b/form/util/factories.hpp @@ -25,6 +25,7 @@ #endif #include +#include #include namespace form::detail::experimental { @@ -38,8 +39,7 @@ namespace form::detail::experimental { return std::make_shared(name, mode); #endif } else if (tech.major == form::technology::Major::hdf5) { - // Handle HDF5 file creation when implemented - // return std::make_shared(name, mode); + throw std::runtime_error("FORM: HDF5 storage is recognized but not yet implemented"); } return std::make_shared(name, mode); } @@ -58,10 +58,7 @@ namespace form::detail::experimental { #endif // USE_RNTUPLE_STORAGE } } else if (tech.major == form::technology::Major::hdf5) { -#ifdef USE_HDF5_STORAGE - // Add HDF5 implementation when available - // return std::make_shared(name); -#endif // USE_HDF5_STORAGE + throw std::runtime_error("FORM: HDF5 storage is recognized but not yet implemented"); } // Default fallback @@ -82,10 +79,7 @@ namespace form::detail::experimental { #endif // USE_RNTUPLE_STORAGE } } else if (tech.major == form::technology::Major::hdf5) { -#ifdef USE_HDF5_STORAGE - // Add HDF5 implementation when available - // return std::make_shared(name); -#endif // USE_HDF5_STORAGE + throw std::runtime_error("FORM: HDF5 storage is recognized but not yet implemented"); } // Default fallback @@ -106,10 +100,7 @@ namespace form::detail::experimental { #endif // USE_RNTUPLE_STORAGE } } else if (tech.major == form::technology::Major::hdf5) { -#ifdef USE_HDF5_STORAGE - // Add HDF5 implementation when available - // return std::make_shared(name); -#endif // USE_HDF5_STORAGE + throw std::runtime_error("FORM: HDF5 storage is recognized but not yet implemented"); } // Default fallback diff --git a/test/form/form_basics_test.cpp b/test/form/form_basics_test.cpp index d76120aae..499107448 100644 --- a/test/form/form_basics_test.cpp +++ b/test/form/form_basics_test.cpp @@ -40,6 +40,45 @@ TEST_CASE("Token basics", "[form]") CHECK(t.id() == 42); } +TEST_CASE("technology::Id string conversions", "[form]") +{ + using namespace form::technology; + + // Round-trip the implemented backends through from_string / to_string + CHECK(from_string("ROOT_TTREE") == ROOT_TTREE); + CHECK(from_string("ROOT_RNTUPLE") == ROOT_RNTUPLE); + + CHECK(to_string(ROOT_TTREE) == "ROOT_TTREE"); + CHECK(to_string(ROOT_RNTUPLE) == "ROOT_RNTUPLE"); + CHECK(to_string(HDF5) == "HDF5"); // reserved: still names itself for diagnostics + + // HDF5 is reserved but unimplemented: reject it at parse time rather than + // silently falling back to a different storage. + CHECK_THROWS_AS(from_string("HDF5"), std::runtime_error); + + // An unknown name throws; an unknown Id stringifies to the sentinel + CHECK_THROWS_AS(from_string("NOT_A_TECH"), std::runtime_error); + CHECK(to_string(Id{}) == "UNKNOWN"); +} + +TEST_CASE("technology::Id members and ordering", "[form]") +{ + using namespace form::technology; + + // (major, minor) decomposition + CHECK(ROOT_TTREE.major == Major::root); + CHECK(ROOT_TTREE.minor == 1); + CHECK(ROOT_RNTUPLE.major == Major::root); + CHECK(ROOT_RNTUPLE.minor == 2); + CHECK(HDF5.major == Major::hdf5); + CHECK(Id{}.major == Major::unknown); + + // operator<=> compares BOTH parts: same major, different minor stay distinct + CHECK(ROOT_TTREE != ROOT_RNTUPLE); + CHECK(ROOT_TTREE < ROOT_RNTUPLE); + CHECK(Id{} == Id{Major::unknown, 0}); +} + TEST_CASE("Storage_File basics", "[form]") { Storage_File f("test.root", 'o'); @@ -134,6 +173,13 @@ TEST_CASE("Factories fallback", "[form]") auto wc = createWriteContainer(form::technology::Id{}, "cont"); CHECK(dynamic_cast(wc.get()) != nullptr); + + // HDF5 is reserved but unimplemented: every factory must fail loudly on the + // hdf5 dispatch branch rather than silently return generic storage. + CHECK_THROWS_AS(createFile(form::technology::HDF5, "test.h5", 'o'), std::runtime_error); + CHECK_THROWS_AS(createReadContainer(form::technology::HDF5, "cont"), std::runtime_error); + CHECK_THROWS_AS(createWriteAssociation(form::technology::HDF5, "assoc"), std::runtime_error); + CHECK_THROWS_AS(createWriteContainer(form::technology::HDF5, "cont"), std::runtime_error); } TEST_CASE("StorageReader basic operations", "[form]") From bd3fe1cfb47f0cd03c04a50577cafd45f9402bc6 Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Fri, 7 Aug 2026 17:19:39 -0500 Subject: [PATCH 05/11] address codecov and clang-tidy fails --- form/util/factories.hpp | 6 ++++ test/form/form_basics_test.cpp | 58 ++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/form/util/factories.hpp b/form/util/factories.hpp index 9b102529b..d285dd3a5 100644 --- a/form/util/factories.hpp +++ b/form/util/factories.hpp @@ -48,6 +48,7 @@ namespace form::detail::experimental { std::string const& name) { if (tech.major == form::technology::Major::root) { +#if defined(USE_ROOT_STORAGE) || defined(USE_RNTUPLE_STORAGE) if (tech == form::technology::ROOT_TTREE) { #ifdef USE_ROOT_STORAGE return std::make_shared(name); @@ -57,6 +58,7 @@ namespace form::detail::experimental { return std::make_shared(name); #endif // USE_RNTUPLE_STORAGE } +#endif } else if (tech.major == form::technology::Major::hdf5) { throw std::runtime_error("FORM: HDF5 storage is recognized but not yet implemented"); } @@ -69,6 +71,7 @@ namespace form::detail::experimental { std::string const& name) { if (tech.major == form::technology::Major::root) { +#if defined(USE_ROOT_STORAGE) || defined(USE_RNTUPLE_STORAGE) if (tech == form::technology::ROOT_TTREE) { #ifdef USE_ROOT_STORAGE return std::make_shared(name); @@ -78,6 +81,7 @@ namespace form::detail::experimental { return std::make_shared(name); #endif // USE_RNTUPLE_STORAGE } +#endif } else if (tech.major == form::technology::Major::hdf5) { throw std::runtime_error("FORM: HDF5 storage is recognized but not yet implemented"); } @@ -90,6 +94,7 @@ namespace form::detail::experimental { std::string const& name) { if (tech.major == form::technology::Major::root) { +#if defined(USE_ROOT_STORAGE) || defined(USE_RNTUPLE_STORAGE) if (tech == form::technology::ROOT_TTREE) { #ifdef USE_ROOT_STORAGE return std::make_shared(name); @@ -99,6 +104,7 @@ namespace form::detail::experimental { return std::make_shared(name); #endif // USE_RNTUPLE_STORAGE } +#endif } else if (tech.major == form::technology::Major::hdf5) { throw std::runtime_error("FORM: HDF5 storage is recognized but not yet implemented"); } diff --git a/test/form/form_basics_test.cpp b/test/form/form_basics_test.cpp index 499107448..6877573ba 100644 --- a/test/form/form_basics_test.cpp +++ b/test/form/form_basics_test.cpp @@ -13,6 +13,16 @@ #include "storage/storage_write_association.hpp" #include "storage/storage_write_container.hpp" #include "util/factories.hpp" +#if defined(USE_ROOT_STORAGE) +#include "root_storage/root_tbranch_read_container.hpp" +#include "root_storage/root_tbranch_write_container.hpp" +#include "root_storage/root_ttree_write_container.hpp" +#endif +#if defined(USE_RNTUPLE_STORAGE) +#include "root_storage/root_rfield_read_container.hpp" +#include "root_storage/root_rfield_write_container.hpp" +#include "root_storage/root_rntuple_write_container.hpp" +#endif #include #include @@ -182,6 +192,54 @@ TEST_CASE("Factories fallback", "[form]") CHECK_THROWS_AS(createWriteContainer(form::technology::HDF5, "cont"), std::runtime_error); } +TEST_CASE("Factories ROOT storage dispatch", "[form]") +{ + auto rc_ttree = createReadContainer(form::technology::ROOT_TTREE, "cont"); +#if defined(USE_ROOT_STORAGE) + CHECK(dynamic_cast(rc_ttree.get()) != nullptr); +#else + CHECK(dynamic_cast(rc_ttree.get()) != nullptr); +#endif + + auto wa_ttree = createWriteAssociation(form::technology::ROOT_TTREE, "assoc"); +#if defined(USE_ROOT_STORAGE) + CHECK(dynamic_cast(wa_ttree.get()) != nullptr); +#else + CHECK(dynamic_cast(wa_ttree.get()) != nullptr); +#endif + + auto wc_ttree = createWriteContainer(form::technology::ROOT_TTREE, "cont"); +#if defined(USE_ROOT_STORAGE) + CHECK(dynamic_cast(wc_ttree.get()) != nullptr); +#else + CHECK(dynamic_cast(wc_ttree.get()) != nullptr); +#endif +} + +TEST_CASE("Factories RNTuple storage dispatch", "[form]") +{ + auto rc_rntuple = createReadContainer(form::technology::ROOT_RNTUPLE, "cont"); +#if defined(USE_RNTUPLE_STORAGE) + CHECK(dynamic_cast(rc_rntuple.get()) != nullptr); +#else + CHECK(dynamic_cast(rc_rntuple.get()) != nullptr); +#endif + + auto wa_rntuple = createWriteAssociation(form::technology::ROOT_RNTUPLE, "assoc"); +#if defined(USE_RNTUPLE_STORAGE) + CHECK(dynamic_cast(wa_rntuple.get()) != nullptr); +#else + CHECK(dynamic_cast(wa_rntuple.get()) != nullptr); +#endif + + auto wc_rntuple = createWriteContainer(form::technology::ROOT_RNTUPLE, "cont"); +#if defined(USE_RNTUPLE_STORAGE) + CHECK(dynamic_cast(wc_rntuple.get()) != nullptr); +#else + CHECK(dynamic_cast(wc_rntuple.get()) != nullptr); +#endif +} + TEST_CASE("StorageReader basic operations", "[form]") { auto storage = createStorageReader(); From 0fa6d489c1af3145e1ad8e77d4b550e4ef79cac5 Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Mon, 10 Aug 2026 18:50:36 -0500 Subject: [PATCH 06/11] redesign factories.hpp: move from form/util to form/storage and split into .hpp and .cpp --- form/storage/CMakeLists.txt | 1 + .../factories.hpp => storage/factories.cpp} | 62 ++++++++----------- form/storage/factories.hpp | 28 +++++++++ form/storage/storage_reader.cpp | 2 +- form/storage/storage_writer.cpp | 2 +- test/form/form_basics_test.cpp | 2 +- test/form/form_storage_test.cpp | 2 + test/form/test_utils.hpp | 2 +- 8 files changed, 61 insertions(+), 40 deletions(-) rename form/{util/factories.hpp => storage/factories.cpp} (70%) create mode 100644 form/storage/factories.hpp diff --git a/form/storage/CMakeLists.txt b/form/storage/CMakeLists.txt index b53ea66ac..8348daf65 100644 --- a/form/storage/CMakeLists.txt +++ b/form/storage/CMakeLists.txt @@ -3,6 +3,7 @@ # Component(s) in the package: add_library( storage + factories.cpp storage_reader.cpp storage_writer.cpp storage_file.cpp diff --git a/form/util/factories.hpp b/form/storage/factories.cpp similarity index 70% rename from form/util/factories.hpp rename to form/storage/factories.cpp index d285dd3a5..09a754d93 100644 --- a/form/util/factories.hpp +++ b/form/storage/factories.cpp @@ -1,11 +1,7 @@ -// Copyright (C) 2025 ..... +// Copyright (C) 2025 ... -#ifndef FORM_UTIL_FACTORIES_HPP -#define FORM_UTIL_FACTORIES_HPP +#include "storage/factories.hpp" -#include "core/technology.hpp" - -#include "storage/istorage.hpp" #include "storage/storage_file.hpp" #include "storage/storage_read_container.hpp" #include "storage/storage_write_association.hpp" @@ -24,15 +20,13 @@ #include "root_storage/root_rntuple_write_container.hpp" #endif -#include #include -#include namespace form::detail::experimental { - inline std::shared_ptr createFile(form::technology::Id tech, - std::string const& name, - char mode) + std::shared_ptr createFile(form::technology::Id tech, + std::string const& name, + char mode) { if (tech.major == form::technology::Major::root) { #ifdef USE_ROOT_STORAGE @@ -44,21 +38,20 @@ namespace form::detail::experimental { return std::make_shared(name, mode); } - inline std::shared_ptr createWriteAssociation(form::technology::Id tech, - std::string const& name) + std::shared_ptr createWriteAssociation(form::technology::Id tech, + std::string const& name) { if (tech.major == form::technology::Major::root) { -#if defined(USE_ROOT_STORAGE) || defined(USE_RNTUPLE_STORAGE) - if (tech == form::technology::ROOT_TTREE) { #ifdef USE_ROOT_STORAGE + if (tech == form::technology::ROOT_TTREE) { return std::make_shared(name); -#endif // USE_ROOT_STORAGE - } else if (tech == form::technology::ROOT_RNTUPLE) { + } #ifdef USE_RNTUPLE_STORAGE + else if (tech == form::technology::ROOT_RNTUPLE) { return std::make_shared(name); -#endif // USE_RNTUPLE_STORAGE } -#endif +#endif // USE_RNTUPLE_STORAGE +#endif // USE_ROOT_STORAGE } else if (tech.major == form::technology::Major::hdf5) { throw std::runtime_error("FORM: HDF5 storage is recognized but not yet implemented"); } @@ -67,21 +60,20 @@ namespace form::detail::experimental { return std::make_shared(name); } - inline std::shared_ptr createReadContainer(form::technology::Id tech, - std::string const& name) + std::shared_ptr createReadContainer(form::technology::Id tech, + std::string const& name) { if (tech.major == form::technology::Major::root) { -#if defined(USE_ROOT_STORAGE) || defined(USE_RNTUPLE_STORAGE) - if (tech == form::technology::ROOT_TTREE) { #ifdef USE_ROOT_STORAGE + if (tech == form::technology::ROOT_TTREE) { return std::make_shared(name); -#endif // USE_ROOT_STORAGE - } else if (tech == form::technology::ROOT_RNTUPLE) { + } #ifdef USE_RNTUPLE_STORAGE + else if (tech == form::technology::ROOT_RNTUPLE) { return std::make_shared(name); -#endif // USE_RNTUPLE_STORAGE } -#endif +#endif // USE_RNTUPLE_STORAGE +#endif // USE_ROOT_STORAGE } else if (tech.major == form::technology::Major::hdf5) { throw std::runtime_error("FORM: HDF5 storage is recognized but not yet implemented"); } @@ -90,21 +82,20 @@ namespace form::detail::experimental { return std::make_shared(name); } - inline std::shared_ptr createWriteContainer(form::technology::Id tech, - std::string const& name) + std::shared_ptr createWriteContainer(form::technology::Id tech, + std::string const& name) { if (tech.major == form::technology::Major::root) { -#if defined(USE_ROOT_STORAGE) || defined(USE_RNTUPLE_STORAGE) - if (tech == form::technology::ROOT_TTREE) { #ifdef USE_ROOT_STORAGE + if (tech == form::technology::ROOT_TTREE) { return std::make_shared(name); -#endif // USE_ROOT_STORAGE - } else if (tech == form::technology::ROOT_RNTUPLE) { + } #ifdef USE_RNTUPLE_STORAGE + else if (tech == form::technology::ROOT_RNTUPLE) { return std::make_shared(name); -#endif // USE_RNTUPLE_STORAGE } -#endif +#endif // USE_RNTUPLE_STORAGE +#endif // USE_ROOT_STORAGE } else if (tech.major == form::technology::Major::hdf5) { throw std::runtime_error("FORM: HDF5 storage is recognized but not yet implemented"); } @@ -114,4 +105,3 @@ namespace form::detail::experimental { } } // namespace form::detail::experimental -#endif // FORM_UTIL_FACTORIES_HPP diff --git a/form/storage/factories.hpp b/form/storage/factories.hpp new file mode 100644 index 000000000..73875cd40 --- /dev/null +++ b/form/storage/factories.hpp @@ -0,0 +1,28 @@ +// Copyright (C) 2025 ... + +#ifndef FORM_STORAGE_FACTORIES_HPP +#define FORM_STORAGE_FACTORIES_HPP + +#include "core/technology.hpp" +#include "storage/istorage.hpp" + +#include +#include + +namespace form::detail::experimental { + + std::shared_ptr createFile(form::technology::Id tech, + std::string const& name, + char mode); + + std::shared_ptr createWriteAssociation(form::technology::Id tech, + std::string const& name); + + std::shared_ptr createReadContainer(form::technology::Id tech, + std::string const& name); + + std::shared_ptr createWriteContainer(form::technology::Id tech, + std::string const& name); + +} // namespace form::detail::experimental +#endif // FORM_STORAGE_FACTORIES_HPP diff --git a/form/storage/storage_reader.cpp b/form/storage/storage_reader.cpp index cedd62308..cb053cb3c 100644 --- a/form/storage/storage_reader.cpp +++ b/form/storage/storage_reader.cpp @@ -4,7 +4,7 @@ #include "storage_file.hpp" #include "storage_read_container.hpp" -#include "util/factories.hpp" +#include "storage/factories.hpp" #include #include diff --git a/form/storage/storage_writer.cpp b/form/storage/storage_writer.cpp index 344919c4b..a6b081ee7 100644 --- a/form/storage/storage_writer.cpp +++ b/form/storage/storage_writer.cpp @@ -5,7 +5,7 @@ #include "storage_file.hpp" #include "storage_write_association.hpp" -#include "util/factories.hpp" +#include "storage/factories.hpp" using namespace form::detail::experimental; diff --git a/test/form/form_basics_test.cpp b/test/form/form_basics_test.cpp index 6877573ba..3b9a951ca 100644 --- a/test/form/form_basics_test.cpp +++ b/test/form/form_basics_test.cpp @@ -12,7 +12,7 @@ #include "storage/storage_read_container.hpp" #include "storage/storage_write_association.hpp" #include "storage/storage_write_container.hpp" -#include "util/factories.hpp" +#include "storage/factories.hpp" #if defined(USE_ROOT_STORAGE) #include "root_storage/root_tbranch_read_container.hpp" #include "root_storage/root_tbranch_write_container.hpp" diff --git a/test/form/form_storage_test.cpp b/test/form/form_storage_test.cpp index b5704ff47..a41050a25 100644 --- a/test/form/form_storage_test.cpp +++ b/test/form/form_storage_test.cpp @@ -5,7 +5,9 @@ #include "form/config.hpp" #include "persistence/persistence_reader.hpp" #include "persistence/persistence_writer.hpp" +#include "storage/storage_file.hpp" #include "storage/storage_reader.hpp" +#include "storage/storage_write_container.hpp" #include "TFile.h" #include "TTree.h" diff --git a/test/form/test_utils.hpp b/test/form/test_utils.hpp index 89bbbf2e3..2464ad6f8 100644 --- a/test/form/test_utils.hpp +++ b/test/form/test_utils.hpp @@ -7,7 +7,7 @@ #include "storage/istorage.hpp" #include "storage/storage_associative_write_container.hpp" #include "storage/storage_read_container.hpp" -#include "util/factories.hpp" +#include "storage/factories.hpp" #include #include From cf7de50eeb180ea3acfff3d11834e12858bd22d1 Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Mon, 10 Aug 2026 18:58:01 -0500 Subject: [PATCH 07/11] applying clang-format --- test/form/form_basics_test.cpp | 2 +- test/form/test_utils.hpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/form/form_basics_test.cpp b/test/form/form_basics_test.cpp index 3b9a951ca..daa7d4618 100644 --- a/test/form/form_basics_test.cpp +++ b/test/form/form_basics_test.cpp @@ -6,13 +6,13 @@ #include "form/form_writer.hpp" #include "persistence/persistence_reader.hpp" #include "persistence/persistence_writer.hpp" +#include "storage/factories.hpp" #include "storage/istorage.hpp" #include "storage/storage_associative_write_container.hpp" #include "storage/storage_file.hpp" #include "storage/storage_read_container.hpp" #include "storage/storage_write_association.hpp" #include "storage/storage_write_container.hpp" -#include "storage/factories.hpp" #if defined(USE_ROOT_STORAGE) #include "root_storage/root_tbranch_read_container.hpp" #include "root_storage/root_tbranch_write_container.hpp" diff --git a/test/form/test_utils.hpp b/test/form/test_utils.hpp index 2464ad6f8..059aa6b39 100644 --- a/test/form/test_utils.hpp +++ b/test/form/test_utils.hpp @@ -4,10 +4,10 @@ #define TEST_FORM_TEST_UTILS_HPP #include "root_storage/demangle_name.hpp" +#include "storage/factories.hpp" #include "storage/istorage.hpp" #include "storage/storage_associative_write_container.hpp" #include "storage/storage_read_container.hpp" -#include "storage/factories.hpp" #include #include From 5258b5ec136bd727417319a60f46b2be4e53b9d3 Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Tue, 11 Aug 2026 12:35:03 -0500 Subject: [PATCH 08/11] FORM: throw on unavailable/unknown storage technologies instead of silently returning the no-op generic backend; enforce RNTuple-requires-ROOT; fix form_basics_test macro visibility and add throw-path tests --- form/CMakeLists.txt | 5 ++ form/storage/factories.cpp | 89 ++++++++++++++++++++++------------ test/form/CMakeLists.txt | 12 ++++- test/form/form_basics_test.cpp | 39 ++++++++------- 4 files changed, 94 insertions(+), 51 deletions(-) diff --git a/form/CMakeLists.txt b/form/CMakeLists.txt index 4ccd8281a..81f1653ce 100644 --- a/form/CMakeLists.txt +++ b/form/CMakeLists.txt @@ -16,6 +16,11 @@ include_directories(${PROJECT_SOURCE_DIR}/form) option(FORM_USE_ROOT_STORAGE "Enable ROOT Storage" ON) option(FORM_USE_RNTUPLE_STORAGE "Enable RNTuple Storage" OFF) +# RNTuple is a ROOT sub-technology and cannot be built without ROOT. +if(FORM_USE_RNTUPLE_STORAGE AND NOT FORM_USE_ROOT_STORAGE) + message(FATAL_ERROR "FORM_USE_RNTUPLE_STORAGE requires FORM_USE_ROOT_STORAGE=ON (RNTuple is part of ROOT).") +endif() + # Add sub directories add_subdirectory(form) add_subdirectory(core) diff --git a/form/storage/factories.cpp b/form/storage/factories.cpp index 09a754d93..832e4996a 100644 --- a/form/storage/factories.cpp +++ b/form/storage/factories.cpp @@ -24,84 +24,113 @@ namespace form::detail::experimental { + using Major = form::technology::Major; + std::shared_ptr createFile(form::technology::Id tech, std::string const& name, char mode) { - if (tech.major == form::technology::Major::root) { + switch (tech.major) { + case Major::unknown: + // No technology specified: generic storage. + return std::make_shared(name, mode); + case Major::root: #ifdef USE_ROOT_STORAGE return std::make_shared(name, mode); +#else + throw std::runtime_error("FORM: ROOT support is not compiled into this build"); #endif - } else if (tech.major == form::technology::Major::hdf5) { + case Major::hdf5: throw std::runtime_error("FORM: HDF5 storage is recognized but not yet implemented"); } - return std::make_shared(name, mode); + throw std::runtime_error("FORM: unsupported storage technology requested"); } std::shared_ptr createWriteAssociation(form::technology::Id tech, std::string const& name) { - if (tech.major == form::technology::Major::root) { + switch (tech.major) { + case Major::unknown: + // No technology specified: generic storage. + return std::make_shared(name); + case Major::root: #ifdef USE_ROOT_STORAGE if (tech == form::technology::ROOT_TTREE) { return std::make_shared(name); - } + } else if (tech == form::technology::ROOT_RNTUPLE) { #ifdef USE_RNTUPLE_STORAGE - else if (tech == form::technology::ROOT_RNTUPLE) { return std::make_shared(name); +#else + throw std::runtime_error("FORM: ROOT RNTUPLE support is not compiled into this build"); +#endif } -#endif // USE_RNTUPLE_STORAGE -#endif // USE_ROOT_STORAGE - } else if (tech.major == form::technology::Major::hdf5) { + // Recognized ROOT major, but an unsupported subtype/minor. + throw std::runtime_error("FORM: requested ROOT write-association backend is not available"); +#else + throw std::runtime_error("FORM: ROOT support is not compiled into this build"); +#endif + case Major::hdf5: throw std::runtime_error("FORM: HDF5 storage is recognized but not yet implemented"); } - - // Default fallback - return std::make_shared(name); + throw std::runtime_error("FORM: unsupported storage technology requested"); } std::shared_ptr createReadContainer(form::technology::Id tech, std::string const& name) { - if (tech.major == form::technology::Major::root) { + switch (tech.major) { + case Major::unknown: + // No technology specified: generic storage. + return std::make_shared(name); + case Major::root: #ifdef USE_ROOT_STORAGE if (tech == form::technology::ROOT_TTREE) { return std::make_shared(name); - } + } else if (tech == form::technology::ROOT_RNTUPLE) { #ifdef USE_RNTUPLE_STORAGE - else if (tech == form::technology::ROOT_RNTUPLE) { return std::make_shared(name); +#else + throw std::runtime_error("FORM: ROOT RNTUPLE support is not compiled into this build"); +#endif } -#endif // USE_RNTUPLE_STORAGE -#endif // USE_ROOT_STORAGE - } else if (tech.major == form::technology::Major::hdf5) { + // Recognized ROOT major, but an unsupported subtype/minor. + throw std::runtime_error("FORM: requested ROOT read-container backend is not available"); +#else + throw std::runtime_error("FORM: ROOT support is not compiled into this build"); +#endif + case Major::hdf5: throw std::runtime_error("FORM: HDF5 storage is recognized but not yet implemented"); } - - // Default fallback - return std::make_shared(name); + throw std::runtime_error("FORM: unsupported storage technology requested"); } std::shared_ptr createWriteContainer(form::technology::Id tech, std::string const& name) { - if (tech.major == form::technology::Major::root) { + switch (tech.major) { + case Major::unknown: + // No technology specified: generic storage. + return std::make_shared(name); + case Major::root: #ifdef USE_ROOT_STORAGE if (tech == form::technology::ROOT_TTREE) { return std::make_shared(name); - } + } else if (tech == form::technology::ROOT_RNTUPLE) { #ifdef USE_RNTUPLE_STORAGE - else if (tech == form::technology::ROOT_RNTUPLE) { return std::make_shared(name); +#else + throw std::runtime_error("FORM: ROOT RNTUPLE support is not compiled into this build"); +#endif } -#endif // USE_RNTUPLE_STORAGE -#endif // USE_ROOT_STORAGE - } else if (tech.major == form::technology::Major::hdf5) { + // Recognized ROOT major, but an unsupported subtype/minor. + throw std::runtime_error("FORM: requested ROOT write-container backend is not available"); +#else + throw std::runtime_error("FORM: ROOT support is not compiled into this build"); +#endif + case Major::hdf5: throw std::runtime_error("FORM: HDF5 storage is recognized but not yet implemented"); } - - // Default fallback - return std::make_shared(name); + throw std::runtime_error("FORM: unsupported storage technology requested"); } } // namespace form::detail::experimental diff --git a/test/form/CMakeLists.txt b/test/form/CMakeLists.txt index 0289f6e3d..a1735f6ae 100644 --- a/test/form/CMakeLists.txt +++ b/test/form/CMakeLists.txt @@ -153,10 +153,20 @@ if(FORM_USE_ROOT_STORAGE AND FORM_USE_RNTUPLE_STORAGE) ) endif() +set(form_basics_test_libraries form) +if(FORM_USE_ROOT_STORAGE) + list(APPEND form_basics_test_libraries root_storage) +endif() cet_test(form_basics_test USE_CATCH2_MAIN SOURCE form_basics_test.cpp LIBRARIES - form + ${form_basics_test_libraries} ) target_include_directories(form_basics_test PRIVATE ${PROJECT_SOURCE_DIR}/form) +if(FORM_USE_ROOT_STORAGE) + target_compile_definitions(form_basics_test PRIVATE USE_ROOT_STORAGE) + if(FORM_USE_RNTUPLE_STORAGE) + target_compile_definitions(form_basics_test PRIVATE USE_RNTUPLE_STORAGE) + endif() +endif() add_library(generate_vector MODULE generate_vector.cpp) target_link_libraries(generate_vector PRIVATE phlex::module) diff --git a/test/form/form_basics_test.cpp b/test/form/form_basics_test.cpp index daa7d4618..24c84a678 100644 --- a/test/form/form_basics_test.cpp +++ b/test/form/form_basics_test.cpp @@ -190,53 +190,52 @@ TEST_CASE("Factories fallback", "[form]") CHECK_THROWS_AS(createReadContainer(form::technology::HDF5, "cont"), std::runtime_error); CHECK_THROWS_AS(createWriteAssociation(form::technology::HDF5, "assoc"), std::runtime_error); CHECK_THROWS_AS(createWriteContainer(form::technology::HDF5, "cont"), std::runtime_error); + + // A major FORM doesn't recognize at all must also fail loudly + // Major has a fixed underlying type, so an out-of-range value is legal at runtime + // NOLINTNEXTLINE(clang-analyzer-optin.core.EnumCastOutOfRange) + auto const unknown_major = form::technology::Id{static_cast(99), 0}; + CHECK_THROWS_AS(createFile(unknown_major, "test.dat", 'o'), std::runtime_error); + CHECK_THROWS_AS(createReadContainer(unknown_major, "cont"), std::runtime_error); + CHECK_THROWS_AS(createWriteAssociation(unknown_major, "assoc"), std::runtime_error); + CHECK_THROWS_AS(createWriteContainer(unknown_major, "cont"), std::runtime_error); } TEST_CASE("Factories ROOT storage dispatch", "[form]") { - auto rc_ttree = createReadContainer(form::technology::ROOT_TTREE, "cont"); #if defined(USE_ROOT_STORAGE) + auto rc_ttree = createReadContainer(form::technology::ROOT_TTREE, "cont"); CHECK(dynamic_cast(rc_ttree.get()) != nullptr); -#else - CHECK(dynamic_cast(rc_ttree.get()) != nullptr); -#endif auto wa_ttree = createWriteAssociation(form::technology::ROOT_TTREE, "assoc"); -#if defined(USE_ROOT_STORAGE) CHECK(dynamic_cast(wa_ttree.get()) != nullptr); -#else - CHECK(dynamic_cast(wa_ttree.get()) != nullptr); -#endif auto wc_ttree = createWriteContainer(form::technology::ROOT_TTREE, "cont"); -#if defined(USE_ROOT_STORAGE) CHECK(dynamic_cast(wc_ttree.get()) != nullptr); #else - CHECK(dynamic_cast(wc_ttree.get()) != nullptr); + CHECK_THROWS_AS(createReadContainer(form::technology::ROOT_TTREE, "cont"), std::runtime_error); + CHECK_THROWS_AS(createWriteAssociation(form::technology::ROOT_TTREE, "assoc"), + std::runtime_error); + CHECK_THROWS_AS(createWriteContainer(form::technology::ROOT_TTREE, "cont"), std::runtime_error); #endif } TEST_CASE("Factories RNTuple storage dispatch", "[form]") { - auto rc_rntuple = createReadContainer(form::technology::ROOT_RNTUPLE, "cont"); #if defined(USE_RNTUPLE_STORAGE) + auto rc_rntuple = createReadContainer(form::technology::ROOT_RNTUPLE, "cont"); CHECK(dynamic_cast(rc_rntuple.get()) != nullptr); -#else - CHECK(dynamic_cast(rc_rntuple.get()) != nullptr); -#endif auto wa_rntuple = createWriteAssociation(form::technology::ROOT_RNTUPLE, "assoc"); -#if defined(USE_RNTUPLE_STORAGE) CHECK(dynamic_cast(wa_rntuple.get()) != nullptr); -#else - CHECK(dynamic_cast(wa_rntuple.get()) != nullptr); -#endif auto wc_rntuple = createWriteContainer(form::technology::ROOT_RNTUPLE, "cont"); -#if defined(USE_RNTUPLE_STORAGE) CHECK(dynamic_cast(wc_rntuple.get()) != nullptr); #else - CHECK(dynamic_cast(wc_rntuple.get()) != nullptr); + CHECK_THROWS_AS(createReadContainer(form::technology::ROOT_RNTUPLE, "cont"), std::runtime_error); + CHECK_THROWS_AS(createWriteAssociation(form::technology::ROOT_RNTUPLE, "assoc"), + std::runtime_error); + CHECK_THROWS_AS(createWriteContainer(form::technology::ROOT_RNTUPLE, "cont"), std::runtime_error); #endif } From 2bf48d570e7b56b6800f0c320a934be0f81f591c Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Tue, 11 Aug 2026 12:48:09 -0500 Subject: [PATCH 09/11] applied cmake format --- form/CMakeLists.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/form/CMakeLists.txt b/form/CMakeLists.txt index 81f1653ce..ef111766d 100644 --- a/form/CMakeLists.txt +++ b/form/CMakeLists.txt @@ -18,7 +18,10 @@ option(FORM_USE_RNTUPLE_STORAGE "Enable RNTuple Storage" OFF) # RNTuple is a ROOT sub-technology and cannot be built without ROOT. if(FORM_USE_RNTUPLE_STORAGE AND NOT FORM_USE_ROOT_STORAGE) - message(FATAL_ERROR "FORM_USE_RNTUPLE_STORAGE requires FORM_USE_ROOT_STORAGE=ON (RNTuple is part of ROOT).") + message( + FATAL_ERROR + "FORM_USE_RNTUPLE_STORAGE requires FORM_USE_ROOT_STORAGE=ON (RNTuple is part of ROOT)." + ) endif() # Add sub directories From 7e977f55bb5f4da6c8751e6abec44819616e5117 Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Wed, 12 Aug 2026 21:21:48 -0500 Subject: [PATCH 10/11] renaming 'unknown' to 'generic' and clean up 'int' in enum as it's the default --- form/core/technology.hpp | 6 +++--- form/storage/factories.cpp | 8 ++++---- test/form/form_basics_test.cpp | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/form/core/technology.hpp b/form/core/technology.hpp index 291cb9b25..582c73183 100644 --- a/form/core/technology.hpp +++ b/form/core/technology.hpp @@ -11,15 +11,15 @@ namespace form::technology { // Major storage type (ROOT, HDF5, ...) - enum class Major : int { - unknown = 0, + enum class Major { + generic = 0, // no specific technology requested root = 1, hdf5 = 2, }; // Minor variant within a Major (e.g. TTree vs RNTuple within ROOT) struct Id { - Major major{Major::unknown}; + Major major{Major::generic}; int minor{0}; // Exact ordering over (major, minor): lets an Id be a std::map key and drives backend dispatch diff --git a/form/storage/factories.cpp b/form/storage/factories.cpp index 832e4996a..321c3933f 100644 --- a/form/storage/factories.cpp +++ b/form/storage/factories.cpp @@ -31,7 +31,7 @@ namespace form::detail::experimental { char mode) { switch (tech.major) { - case Major::unknown: + case Major::generic: // No technology specified: generic storage. return std::make_shared(name, mode); case Major::root: @@ -50,7 +50,7 @@ namespace form::detail::experimental { std::string const& name) { switch (tech.major) { - case Major::unknown: + case Major::generic: // No technology specified: generic storage. return std::make_shared(name); case Major::root: @@ -79,7 +79,7 @@ namespace form::detail::experimental { std::string const& name) { switch (tech.major) { - case Major::unknown: + case Major::generic: // No technology specified: generic storage. return std::make_shared(name); case Major::root: @@ -108,7 +108,7 @@ namespace form::detail::experimental { std::string const& name) { switch (tech.major) { - case Major::unknown: + case Major::generic: // No technology specified: generic storage. return std::make_shared(name); case Major::root: diff --git a/test/form/form_basics_test.cpp b/test/form/form_basics_test.cpp index 24c84a678..19127bcd1 100644 --- a/test/form/form_basics_test.cpp +++ b/test/form/form_basics_test.cpp @@ -81,12 +81,12 @@ TEST_CASE("technology::Id members and ordering", "[form]") CHECK(ROOT_RNTUPLE.major == Major::root); CHECK(ROOT_RNTUPLE.minor == 2); CHECK(HDF5.major == Major::hdf5); - CHECK(Id{}.major == Major::unknown); + CHECK(Id{}.major == Major::generic); // operator<=> compares BOTH parts: same major, different minor stay distinct CHECK(ROOT_TTREE != ROOT_RNTUPLE); CHECK(ROOT_TTREE < ROOT_RNTUPLE); - CHECK(Id{} == Id{Major::unknown, 0}); + CHECK(Id{} == Id{Major::generic, 0}); } TEST_CASE("Storage_File basics", "[form]") From f1299a8f823d064eff6002fa2f2086aaf6490689 Mon Sep 17 00:00:00 2001 From: wwuoneway Date: Fri, 14 Aug 2026 11:13:46 -0500 Subject: [PATCH 11/11] add tests for recognized ROOT major but unsupported minor --- test/form/form_basics_test.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/form/form_basics_test.cpp b/test/form/form_basics_test.cpp index 19127bcd1..c1bed680d 100644 --- a/test/form/form_basics_test.cpp +++ b/test/form/form_basics_test.cpp @@ -212,6 +212,11 @@ TEST_CASE("Factories ROOT storage dispatch", "[form]") auto wc_ttree = createWriteContainer(form::technology::ROOT_TTREE, "cont"); CHECK(dynamic_cast(wc_ttree.get()) != nullptr); + + auto const unsupported_root = form::technology::Id{form::technology::Major::root, 99}; + CHECK_THROWS_AS(createReadContainer(unsupported_root, "cont"), std::runtime_error); + CHECK_THROWS_AS(createWriteAssociation(unsupported_root, "assoc"), std::runtime_error); + CHECK_THROWS_AS(createWriteContainer(unsupported_root, "cont"), std::runtime_error); #else CHECK_THROWS_AS(createReadContainer(form::technology::ROOT_TTREE, "cont"), std::runtime_error); CHECK_THROWS_AS(createWriteAssociation(form::technology::ROOT_TTREE, "assoc"),