diff --git a/docs/dev/clang-tidy-fixes-2026-04.md b/docs/dev/clang-tidy-fixes-2026-04.md index 39358f82..b4e41de7 100644 --- a/docs/dev/clang-tidy-fixes-2026-04.md +++ b/docs/dev/clang-tidy-fixes-2026-04.md @@ -11,5 +11,5 @@ - [x] [bugprone-random-generator-seed](https://github.com/Framework-R-D/phlex/pull/490) (1) - [x] [bugprone-reserved-identifier](https://github.com/Framework-R-D/phlex/pull/489) (1) - [ ] bugprone-return-const-ref-from-parameter (163) - - [ ] bugprone-throwing-static-initialization (11) + - [x] [bugprone-throwing-static-initialization](https://github.com/Framework-R-D/phlex/pull/472) (11) - [ ] bugprone-unchecked-optional-access (15) diff --git a/form/root_storage/root_tbranch_write_container.cpp b/form/root_storage/root_tbranch_write_container.cpp index 4a0132b0..9d524bde 100644 --- a/form/root_storage/root_tbranch_write_container.cpp +++ b/form/root_storage/root_tbranch_write_container.cpp @@ -12,20 +12,6 @@ #include -namespace { - //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 typeNameToLeafList = {{"int", "/I"}, - {"unsigned int", "/i"}, - {"float", "/F"}, - {"double", "/D"}, - {"short int", "/S"}, - {"unsigned short", "/s"}, - {"long int", "/L"}, - {"unsigned long int", "/l"}, - {"bool", "/O"}}; -} - using namespace form::detail::experimental; ROOT_TBranch_Write_ContainerImp::ROOT_TBranch_Write_ContainerImp(std::string const& name) : @@ -69,6 +55,19 @@ void ROOT_TBranch_Write_ContainerImp::setParent(std::shared_ptr typeNameToLeafList = { + {"int", "/I"}, + {"unsigned int", "/i"}, + {"float", "/F"}, + {"double", "/D"}, + {"short int", "/S"}, + {"unsigned short", "/s"}, + {"long int", "/L"}, + {"unsigned long int", "/l"}, + {"bool", "/O"}}; + if (m_tree == nullptr) { throw std::runtime_error("ROOT_TBranch_Write_ContainerImp::setupWrite no tree found"); } diff --git a/phlex/core/detail/filter_impl.cpp b/phlex/core/detail/filter_impl.cpp index 5a7cf673..dfb11e89 100644 --- a/phlex/core/detail/filter_impl.cpp +++ b/phlex/core/detail/filter_impl.cpp @@ -5,9 +5,13 @@ #include namespace { - phlex::product_query const output_dummy = phlex::product_query{ - .creator = "for_output_only"_id, .layer = "dummy_layer"_id, .suffix = "for_output_only"_id}; - phlex::product_queries const for_output_only{output_dummy}; + phlex::product_queries const& for_output_only() + { + static phlex::product_query const output_dummy = phlex::product_query{ + .creator = "for_output_only"_id, .layer = "dummy_layer"_id, .suffix = "for_output_only"_id}; + static phlex::product_queries const for_output_only_queries{output_dummy}; + return for_output_only_queries; + } } namespace phlex::experimental { @@ -57,7 +61,7 @@ namespace phlex::experimental { assert(nargs_ > 0); } - data_map::data_map(for_output_t) : data_map{for_output_only} {} + data_map::data_map(for_output_t) : data_map{for_output_only()} {} void data_map::update(std::size_t const msg_id, product_store_const_ptr const& store) { diff --git a/phlex/model/data_layer_hierarchy.cpp b/phlex/model/data_layer_hierarchy.cpp index 1de22aad..8a2e15a6 100644 --- a/phlex/model/data_layer_hierarchy.cpp +++ b/phlex/model/data_layer_hierarchy.cpp @@ -6,8 +6,11 @@ #include "spdlog/spdlog.h" namespace { - std::string const unnamed{"(unnamed)"}; - std::string const& maybe_name(std::string const& name) { return empty(name) ? unnamed : name; } + std::string const& maybe_name(std::string const& name) + { + static std::string const unnamed{"(unnamed)"}; + return empty(name) ? unnamed : name; + } } namespace phlex::experimental { diff --git a/test/data_cell_counting.cpp b/test/data_cell_counting.cpp index 0f0b9027..2548557e 100644 --- a/test/data_cell_counting.cpp +++ b/test/data_cell_counting.cpp @@ -10,7 +10,13 @@ using namespace phlex::experimental::literals; using phlex::data_cell_index; namespace { - auto const job_hash_value = "job"_idq.hash; + struct job_hash_fixture { + std::size_t job_hash_value() const + { + static std::size_t const cached_hash = "job"_idq.hash; + return cached_hash; + } + }; } TEST_CASE("Counter with nothing processed", "[data model]") @@ -19,13 +25,13 @@ TEST_CASE("Counter with nothing processed", "[data model]") CHECK(job_counter.result().empty()); } -TEST_CASE("Counter one layer deep", "[data model]") +TEST_CASE_METHOD(job_hash_fixture, "Counter one layer deep", "[data model]") { data_cell_counter job_counter{}; for (std::size_t i = 0; i != 10; ++i) { job_counter.make_child("event"_id); } - auto const event_hash_value = hash(job_hash_value, "event"_idq.hash); + auto const event_hash_value = hash(job_hash_value(), "event"_idq.hash); CHECK(job_counter.result().count_for(event_hash_value) == 10); } @@ -56,7 +62,19 @@ TEST_CASE("Data layer hierarchy with ambiguous layer names", "[data model]") CHECK(h.count_for("/job/run/spill") == 2); } -TEST_CASE("Counter multiple layers deep", "[data model]") +TEST_CASE("Data layer hierarchy with unnamed layer", "[data model]") +{ + // Exercises the maybe_name() fallback to "(unnamed)" for layers with empty names. + // Invoke print() explicitly so the test body, rather than teardown side effects, + // traverses the hierarchy and reaches maybe_name("") for the unnamed child. + data_layer_hierarchy h; + auto job_index = data_cell_index::job(); + CHECK_NOTHROW(h.increment_count(job_index)); + CHECK_NOTHROW(h.increment_count(job_index->make_child("", 0))); + CHECK_NOTHROW(h.print()); +} + +TEST_CASE_METHOD(job_hash_fixture, "Counter multiple layers deep", "[data model]") { constexpr std::size_t nruns{2ull}; constexpr std::size_t nsubruns_per_run{3ull}; @@ -78,7 +96,7 @@ TEST_CASE("Counter multiple layers deep", "[data model]") CHECK(h.count_for("event", true) == processed_events); }; - auto const run_hash_value = hash(job_hash_value, "run"_idq.hash); + auto const run_hash_value = hash(job_hash_value(), "run"_idq.hash); auto const subrun_hash_value = hash(run_hash_value, "subrun"_idq.hash); auto const event_hash_value = hash(subrun_hash_value, "event"_idq.hash); diff --git a/test/filter_impl.cpp b/test/filter_impl.cpp index 4504ff4d..1197e871 100644 --- a/test/filter_impl.cpp +++ b/test/filter_impl.cpp @@ -66,3 +66,21 @@ TEST_CASE("Filter data map", "[filtering]") data.update(msg_id, store_with_b); CHECK(data.is_complete(msg_id)); } + +TEST_CASE("Data map for output only", "[filtering]") +{ + // Exercises data_map(for_output_t) which uses the output-only product query + data_map data{data_map::for_output}; + + std::size_t const msg_id{1}; + CHECK(not data.is_complete(msg_id)); + + // Output-only data maps accept any store (no product lookup required) + auto store = product_store::base(algorithm_name::create("output_only")); + data.update(msg_id, store); + CHECK(data.is_complete(msg_id)); + + auto result = data.release_data(msg_id); + CHECK(result.size() == 1); + CHECK(not data.is_complete(msg_id)); +} diff --git a/test/form/test_utils.hpp b/test/form/test_utils.hpp index 99e51ee6..6f9a2c78 100644 --- a/test/form/test_utils.hpp +++ b/test/form/test_utils.hpp @@ -17,8 +17,8 @@ using namespace form::detail::experimental; namespace form::test { - inline std::string const testTreeName = "FORMTestTree"; - inline std::string const testFileName = "FORMTestFile.root"; + inline constexpr char const* testTreeName = "FORMTestTree"; + inline constexpr char const* testFileName = "FORMTestFile.root"; template inline std::string getTypeName() @@ -29,7 +29,7 @@ namespace form::test { template inline std::string makeTestBranchName() { - return testTreeName + "/" + getTypeName(); + return std::string(testTreeName) + "/" + getTypeName(); } inline std::vector> doWrite( @@ -66,8 +66,8 @@ namespace form::test { template inline void write(int const technology, PRODS&... prods) { - auto file = createFile(technology, testFileName.c_str(), 'o'); - auto parent = createWriteAssociation(technology, testTreeName); + auto file = createFile(technology, std::string(testFileName), 'o'); + auto parent = createWriteAssociation(technology, std::string(testTreeName)); parent->setFile(file); parent->setupWrite(); @@ -94,7 +94,7 @@ namespace form::test { template inline std::tuple...> read(int const technology) { - auto file = createFile(technology, testFileName, 'i'); + auto file = createFile(technology, std::string(testFileName), 'i'); return std::make_tuple(doRead(file, technology)...); }