Skip to content
Merged
2 changes: 1 addition & 1 deletion docs/dev/clang-tidy-fixes-2026-04.md
Comment thread
knoepfel marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -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)
27 changes: 13 additions & 14 deletions form/root_storage/root_tbranch_write_container.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,6 @@

#include <unordered_map>

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<std::string, std::string> 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) :
Expand Down Expand Up @@ -69,6 +55,19 @@ void ROOT_TBranch_Write_ContainerImp::setParent(std::shared_ptr<IStorage_Write_C

void ROOT_TBranch_Write_ContainerImp::setupWrite(std::type_info const& type)
{
//Type name conversion based on https://root.cern.ch/doc/master/classTTree.html#ac1fa9466ce018d4aa739b357f981c615
//An empty leaf list (i.e. for a type not in this map) defaults to Float_t; this is intentional.
static std::unordered_map<std::string, std::string> 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"}};
Comment thread
greenc-FNAL marked this conversation as resolved.

if (m_tree == nullptr) {
throw std::runtime_error("ROOT_TBranch_Write_ContainerImp::setupWrite no tree found");
}
Expand Down
12 changes: 8 additions & 4 deletions phlex/core/detail/filter_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@
#include <string>

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 {
Expand Down Expand Up @@ -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)
{
Expand Down
7 changes: 5 additions & 2 deletions phlex/model/data_layer_hierarchy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
28 changes: 23 additions & 5 deletions test/data_cell_counting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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]")
Expand All @@ -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);
}

Expand Down Expand Up @@ -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};
Expand All @@ -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);

Expand Down
18 changes: 18 additions & 0 deletions test/filter_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
12 changes: 6 additions & 6 deletions test/form/test_utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <class PROD>
inline std::string getTypeName()
Expand All @@ -29,7 +29,7 @@ namespace form::test {
template <class PROD>
inline std::string makeTestBranchName()
{
return testTreeName + "/" + getTypeName<PROD>();
return std::string(testTreeName) + "/" + getTypeName<PROD>();
}

inline std::vector<std::shared_ptr<IStorage_Write_Container>> doWrite(
Expand Down Expand Up @@ -66,8 +66,8 @@ namespace form::test {
template <class... PRODS>
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();

Expand All @@ -94,7 +94,7 @@ namespace form::test {
template <class... PRODS>
inline std::tuple<std::unique_ptr<PRODS const>...> read(int const technology)
{
auto file = createFile(technology, testFileName, 'i');
auto file = createFile(technology, std::string(testFileName), 'i');

return std::make_tuple(doRead<PRODS>(file, technology)...);
}
Expand Down
Loading