From 2da55c7612fa880d5553aa9afbec91dd6b95dcdb Mon Sep 17 00:00:00 2001 From: Marc Paterno Date: Wed, 21 Oct 2026 09:11:38 -0500 Subject: [PATCH 1/4] feat(model): store concept translator registrations Add result storage metadata and type-erased conversion functions to data product concepts, with validation for concrete types, identical types, and duplicate ordered pairs. Add tests covering registration lookup, conversion round-tripping, storage semantics, and validation failures. --- phlex/model/data_product_concept.cpp | 60 +++++++++ phlex/model/data_product_concept.hpp | 59 ++++++++- test/data_product_concept_test.cpp | 180 ++++++++++++++++++++++++--- 3 files changed, 284 insertions(+), 15 deletions(-) diff --git a/phlex/model/data_product_concept.cpp b/phlex/model/data_product_concept.cpp index c9daaed95..a5a715b8b 100644 --- a/phlex/model/data_product_concept.cpp +++ b/phlex/model/data_product_concept.cpp @@ -1,5 +1,8 @@ #include "phlex/model/data_product_concept.hpp" +#include "boost/core/demangle.hpp" +#include "fmt/format.h" + #include #include #include @@ -41,4 +44,61 @@ namespace phlex::experimental { return concrete_types_.contains(type); } + void data_product_concept::add_translator(std::string translator_name, + concrete_product_id source, + concrete_product_id target, + std::any function, + result_storage storage) + { + if (source == target) { + throw std::invalid_argument{ + fmt::format("Cannot register translator '{}' with concept '{}': the source and target " + "types are both '{}'.", + translator_name, + name_, + boost::core::demangle(source.name()))}; + } + + for (auto type : {source, target}) { + if (!has_concrete_type(type)) { + throw std::invalid_argument{ + fmt::format("Cannot register translator '{}' with concept '{}': type '{}' is not a " + "concrete type of that concept.", + translator_name, + name_, + boost::core::demangle(type.name()))}; + } + } + + translator_key const key{source, target}; + if (auto it = translators_.find(key); it != translators_.end()) { + throw std::invalid_argument{ + fmt::format("Cannot register translator '{}' with concept '{}': translator '{}' is " + "already registered for the conversion from '{}' to '{}'.", + translator_name, + name_, + it->second.name, + boost::core::demangle(source.name()), + boost::core::demangle(target.name()))}; + } + + translators_.emplace( + key, translator_registration{std::move(translator_name), std::move(function), storage}); + } + + translator_registration const* data_product_concept::find_translator( + concrete_product_id source, concrete_product_id target) const + { + auto it = translators_.find(translator_key{source, target}); + if (it == translators_.end()) { + return nullptr; + } + return &it->second; + } + + bool data_product_concept::operator==(data_product_concept const& other) const + { + return name_ == other.name_ && concrete_types_ == other.concrete_types_; + } + } // namespace phlex::experimental diff --git a/phlex/model/data_product_concept.hpp b/phlex/model/data_product_concept.hpp index e23220339..e5bd77414 100644 --- a/phlex/model/data_product_concept.hpp +++ b/phlex/model/data_product_concept.hpp @@ -4,10 +4,13 @@ #include "phlex/model/identifier.hpp" #include "phlex/phlex_model_export.hpp" +#include +#include #include #include #include #include +#include #include namespace phlex::experimental { @@ -15,6 +18,31 @@ namespace phlex::experimental { // Type alias for std::type_index, representing a concrete data product type. using concrete_product_id = std::type_index; + // Declares the relationship between the result of a conversion function and + // its storage. + // + // A conversion is 'owned' when its result owns its storage, and 'borrowed' + // when its result refers to storage owned by the input product. A borrowed + // result dangles if its source is released, so the caller that uses a + // borrowed conversion must keep the source alive. This cannot be deduced and + // must be declared at registration. This declaration is recorded but not + // enforced by this class. + enum class result_storage { owned, borrowed }; + + // A conversion function registered with a data product concept, together + // with the information a translator node needs in order to use it. + struct translator_registration { + // Name of the translator algorithm, as supplied at registration. + std::string name; + + // The conversion function, type-erased. The stored type is not checked + // against the registered source and target types. + std::any function; + + // Whether the conversion's result owns or borrows its storage. + result_storage storage; + }; + // Represents a data product concept. // // A data product concept is a named category that groups related concrete @@ -24,6 +52,10 @@ namespace phlex::experimental { // Each data_product_concept has: // - A unique name, which can not be modified // - A set of concrete types that model this concept + // - A set of conversion functions between pairs of those concrete types + // + // add_translator checks the concrete-type and ordered-pair invariants for + // registrations made through it. It does not check the stored function's type. class PHLEX_MODEL_EXPORT data_product_concept { public: explicit data_product_concept(std::string name); @@ -46,13 +78,38 @@ namespace phlex::experimental { bool has_concrete_type(concrete_product_id type) const; - bool operator==(data_product_concept const&) const = default; + // Register a conversion function from the source to the target concrete + // type, under the given translator name. + // + // Throws std::invalid_argument if: + // - source and target are the same type, + // - either type is not already a concrete type of this concept, or + // - a translator is already registered for that ordered pair. + void add_translator(std::string translator_name, + concrete_product_id source, + concrete_product_id target, + std::any function, + result_storage storage); + + // Get the translator registered for an ordered pair of concrete types, or + // nullptr if there is none. The returned pointer is invalidated by the + // destruction of this concept. + translator_registration const* find_translator(concrete_product_id source, + concrete_product_id target) const; + + // Compares the name and the concrete types. Registered translators do not + // participate: a std::any holding a conversion function is not + // equality-comparable. + bool operator==(data_product_concept const& other) const; private: + using translator_key = std::pair; + // The name must remain consistent with the registry key. // NOLINTNEXTLINE(cppcoreguidelines-avoid-const-or-ref-data-members) std::string const name_; std::unordered_set concrete_types_; + std::map translators_; }; } // namespace phlex::experimental diff --git a/test/data_product_concept_test.cpp b/test/data_product_concept_test.cpp index a663dc02e..3b6a06111 100644 --- a/test/data_product_concept_test.cpp +++ b/test/data_product_concept_test.cpp @@ -1,6 +1,8 @@ #include "catch2/catch_test_macros.hpp" #include "phlex/model/data_product_concept.hpp" +#include +#include #include #include #include @@ -8,15 +10,28 @@ using namespace phlex::experimental; namespace { - struct test_type_1 { + struct soa_vertices { int value; - auto operator<=>(test_type_1 const&) const = default; + auto operator<=>(soa_vertices const&) const = default; }; - struct test_type_2 { + struct aos_vertices { std::string value; - auto operator<=>(test_type_2 const&) const = default; + auto operator<=>(aos_vertices const&) const = default; }; + + using conversion_to_soa = std::function; + + soa_vertices to_soa(aos_vertices const& in) { return soa_vertices{std::stoi(in.value)}; } + + // A concept modeled by both test types, ready for translator registration. + data_product_concept concept_with_both_types() + { + data_product_concept con{"VertexCollection"}; + con.add_concrete_type(typeid(soa_vertices)); + con.add_concrete_type(typeid(aos_vertices)); + return con; + } } TEST_CASE("data_product_concept construction records the right name", "[data_product_concept]") @@ -42,10 +57,10 @@ TEST_CASE("data_product_concept starts with no concrete types", "[data_product_c TEST_CASE("data_product_concept add and check concrete types", "[data_product_concept]") { - data_product_concept con{"test_concept"}; + data_product_concept con{"VertexCollection"}; - auto const& type1 = typeid(test_type_1); - auto const& type2 = typeid(test_type_2); + auto const& type1 = typeid(soa_vertices); + auto const& type2 = typeid(aos_vertices); con.add_concrete_type(type1); con.add_concrete_type(type2); @@ -85,9 +100,9 @@ TEST_CASE("data_product_concept with multiple primitive types", "[data_product_c TEST_CASE("adding a concrete type is idempotent", "[data_product_concept]") { - data_product_concept con{"concept_A"}; + data_product_concept con{"VertexCollection"}; - auto const& type1 = typeid(test_type_1); + auto const& type1 = typeid(soa_vertices); con.add_concrete_type(type1); con.add_concrete_type(type1); // Add again @@ -103,14 +118,151 @@ TEST_CASE("data_product_concept construction with an empty name throws", "[data_ TEST_CASE("add_concrete_types adds the union of an overlapping set", "[data_product_concept]") { - data_product_concept con{"concept_A"}; - con.add_concrete_type(typeid(test_type_1)); + data_product_concept con{"VertexCollection"}; + con.add_concrete_type(typeid(soa_vertices)); - con.add_concrete_types({typeid(test_type_1), typeid(test_type_2)}); + con.add_concrete_types({typeid(soa_vertices), typeid(aos_vertices)}); CHECK(con.concrete_types().size() == 2ull); - CHECK(con.has_concrete_type(typeid(test_type_1))); - CHECK(con.has_concrete_type(typeid(test_type_2))); + CHECK(con.has_concrete_type(typeid(soa_vertices))); + CHECK(con.has_concrete_type(typeid(aos_vertices))); +} + +TEST_CASE("a concept starts with no registered translators", "[data_product_concept]") +{ + auto con = concept_with_both_types(); + CHECK(con.find_translator(typeid(soa_vertices), typeid(aos_vertices)) == nullptr); +} + +TEST_CASE("a registered translator round-trips through the accessor", "[data_product_concept]") +{ + auto con = concept_with_both_types(); + + con.add_translator("one_to_two", + typeid(soa_vertices), + typeid(aos_vertices), + conversion_to_soa{to_soa}, + result_storage::owned); + + auto const* registration = con.find_translator(typeid(soa_vertices), typeid(aos_vertices)); + REQUIRE(registration != nullptr); + CHECK(registration->name == "one_to_two"); + CHECK(registration->storage == result_storage::owned); + + // The stored callable converts aos_vertices to soa_vertices, which is the + // reverse of the pair it is registered under. Registration does not verify + // the callable's signature, so this checks only that the stored value is + // retrievable and invocable, not that it is the correct direction. + auto const* conversion = std::any_cast(®istration->function); + REQUIRE(conversion != nullptr); + aos_vertices input{"7"}; + soa_vertices const result = (*conversion)(input); + CHECK(result == soa_vertices{7}); +} + +TEST_CASE("a borrowed result storage declaration is preserved", "[data_product_concept]") +{ + auto con = concept_with_both_types(); + + con.add_translator("one_to_two", + typeid(soa_vertices), + typeid(aos_vertices), + conversion_to_soa{to_soa}, + result_storage::borrowed); + + auto const* registration = con.find_translator(typeid(soa_vertices), typeid(aos_vertices)); + REQUIRE(registration != nullptr); + CHECK(registration->storage == result_storage::borrowed); +} + +TEST_CASE("translators are keyed by the ordered type pair", "[data_product_concept]") +{ + auto con = concept_with_both_types(); + + con.add_translator("one_to_two", + typeid(soa_vertices), + typeid(aos_vertices), + conversion_to_soa{to_soa}, + result_storage::owned); + + // The reverse conversion is a different translator, and is not registered. + CHECK(con.find_translator(typeid(aos_vertices), typeid(soa_vertices)) == nullptr); + + // The reverse pair is accepted independently, and each direction keeps its + // own registration. + con.add_translator("two_to_one", + typeid(aos_vertices), + typeid(soa_vertices), + std::function{}, + result_storage::owned); + + auto const* forward = con.find_translator(typeid(soa_vertices), typeid(aos_vertices)); + auto const* reverse = con.find_translator(typeid(aos_vertices), typeid(soa_vertices)); + REQUIRE(forward != nullptr); + REQUIRE(reverse != nullptr); + CHECK(forward->name == "one_to_two"); + CHECK(reverse->name == "two_to_one"); +} + +TEST_CASE("registering a translator with identical source and target types throws", + "[data_product_concept]") +{ + auto con = concept_with_both_types(); + + CHECK_THROWS_AS(con.add_translator("one_to_one", + typeid(soa_vertices), + typeid(soa_vertices), + std::function{}, + result_storage::owned), + std::invalid_argument); +} + +TEST_CASE("registering a translator for a type outside the concept throws", + "[data_product_concept]") +{ + data_product_concept con{"VertexCollection"}; + con.add_concrete_type(typeid(soa_vertices)); + + // The target type does not model the concept. + CHECK_THROWS_AS(con.add_translator("one_to_two", + typeid(soa_vertices), + typeid(aos_vertices), + conversion_to_soa{to_soa}, + result_storage::owned), + std::invalid_argument); + + // Nor does the source type, once the roles are reversed. + CHECK_THROWS_AS(con.add_translator("two_to_one", + typeid(aos_vertices), + typeid(soa_vertices), + std::function{}, + result_storage::owned), + std::invalid_argument); + + CHECK(con.find_translator(typeid(soa_vertices), typeid(aos_vertices)) == nullptr); +} + +TEST_CASE("registering a second translator for the same type pair throws", "[data_product_concept]") +{ + auto con = concept_with_both_types(); + + con.add_translator("one_to_two", + typeid(soa_vertices), + typeid(aos_vertices), + conversion_to_soa{to_soa}, + result_storage::owned); + + CHECK_THROWS_AS(con.add_translator("another_one_to_two", + typeid(soa_vertices), + typeid(aos_vertices), + conversion_to_soa{to_soa}, + result_storage::owned), + std::invalid_argument); + + // The original registration is intact. + auto const* registration = con.find_translator(typeid(soa_vertices), typeid(aos_vertices)); + REQUIRE(registration != nullptr); + CHECK(registration->name == "one_to_two"); } TEST_CASE("add_concrete_types accepts its own set", "[data_product_concept]") From a015d4f83fbad2a5bd78f374e6c8726fb0cb714b Mon Sep 17 00:00:00 2001 From: Marc Paterno Date: Tue, 3 Nov 2026 15:44:37 -0600 Subject: [PATCH 2/4] style: apply clang-tidy fixes to data product concepts - Add missing and includes - Set std::uint8_t as the underlying type of result_storage - Use designated initializers for translator_registration - Separate the registry source's own header include group --- phlex/model/data_product_concept.cpp | 7 +++++-- phlex/model/data_product_concept.hpp | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/phlex/model/data_product_concept.cpp b/phlex/model/data_product_concept.cpp index a5a715b8b..521f105bd 100644 --- a/phlex/model/data_product_concept.cpp +++ b/phlex/model/data_product_concept.cpp @@ -3,6 +3,7 @@ #include "boost/core/demangle.hpp" #include "fmt/format.h" +#include #include #include #include @@ -82,8 +83,10 @@ namespace phlex::experimental { boost::core::demangle(target.name()))}; } - translators_.emplace( - key, translator_registration{std::move(translator_name), std::move(function), storage}); + translators_.emplace(key, + translator_registration{.name = std::move(translator_name), + .function = std::move(function), + .storage = storage}); } translator_registration const* data_product_concept::find_translator( diff --git a/phlex/model/data_product_concept.hpp b/phlex/model/data_product_concept.hpp index e5bd77414..a2ec320ff 100644 --- a/phlex/model/data_product_concept.hpp +++ b/phlex/model/data_product_concept.hpp @@ -5,6 +5,7 @@ #include "phlex/phlex_model_export.hpp" #include +#include #include #include #include @@ -27,7 +28,7 @@ namespace phlex::experimental { // borrowed conversion must keep the source alive. This cannot be deduced and // must be declared at registration. This declaration is recorded but not // enforced by this class. - enum class result_storage { owned, borrowed }; + enum class result_storage : std::uint8_t { owned, borrowed }; // A conversion function registered with a data product concept, together // with the information a translator node needs in order to use it. From b82ac36d0ff177912328b587b21ab11d86940454 Mon Sep 17 00:00:00 2001 From: Marc Paterno Date: Tue, 3 Nov 2026 15:45:16 -0600 Subject: [PATCH 3/4] test: adapt data product concept tests to non-copyable concept Replace the concept factory helper with add_both_types, which populates a caller-owned concept because data_product_concept is neither movable nor assignable. Use the soa_vertices and aos_vertices types in the add_concrete_types self-insertion test, and make the translator input const. --- test/data_product_concept_test.cpp | 37 ++++++++++++++++++------------ 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/test/data_product_concept_test.cpp b/test/data_product_concept_test.cpp index 3b6a06111..bac3c354c 100644 --- a/test/data_product_concept_test.cpp +++ b/test/data_product_concept_test.cpp @@ -24,13 +24,14 @@ namespace { soa_vertices to_soa(aos_vertices const& in) { return soa_vertices{std::stoi(in.value)}; } - // A concept modeled by both test types, ready for translator registration. - data_product_concept concept_with_both_types() + // Make a concept modeled by both test types, ready for translator + // registration. data_product_concept is neither movable nor assignable, so + // it cannot be returned from a function that populates a named local; + // instead the caller constructs the concept and this function populates it. + void add_both_types(data_product_concept& con) { - data_product_concept con{"VertexCollection"}; con.add_concrete_type(typeid(soa_vertices)); con.add_concrete_type(typeid(aos_vertices)); - return con; } } @@ -130,13 +131,15 @@ TEST_CASE("add_concrete_types adds the union of an overlapping set", "[data_prod TEST_CASE("a concept starts with no registered translators", "[data_product_concept]") { - auto con = concept_with_both_types(); + data_product_concept con{"VertexCollection"}; + add_both_types(con); CHECK(con.find_translator(typeid(soa_vertices), typeid(aos_vertices)) == nullptr); } TEST_CASE("a registered translator round-trips through the accessor", "[data_product_concept]") { - auto con = concept_with_both_types(); + data_product_concept con{"VertexCollection"}; + add_both_types(con); con.add_translator("one_to_two", typeid(soa_vertices), @@ -155,14 +158,15 @@ TEST_CASE("a registered translator round-trips through the accessor", "[data_pro // retrievable and invocable, not that it is the correct direction. auto const* conversion = std::any_cast(®istration->function); REQUIRE(conversion != nullptr); - aos_vertices input{"7"}; + aos_vertices const input{"7"}; soa_vertices const result = (*conversion)(input); CHECK(result == soa_vertices{7}); } TEST_CASE("a borrowed result storage declaration is preserved", "[data_product_concept]") { - auto con = concept_with_both_types(); + data_product_concept con{"VertexCollection"}; + add_both_types(con); con.add_translator("one_to_two", typeid(soa_vertices), @@ -177,7 +181,8 @@ TEST_CASE("a borrowed result storage declaration is preserved", "[data_product_c TEST_CASE("translators are keyed by the ordered type pair", "[data_product_concept]") { - auto con = concept_with_both_types(); + data_product_concept con{"VertexCollection"}; + add_both_types(con); con.add_translator("one_to_two", typeid(soa_vertices), @@ -207,7 +212,8 @@ TEST_CASE("translators are keyed by the ordered type pair", "[data_product_conce TEST_CASE("registering a translator with identical source and target types throws", "[data_product_concept]") { - auto con = concept_with_both_types(); + data_product_concept con{"VertexCollection"}; + add_both_types(con); CHECK_THROWS_AS(con.add_translator("one_to_one", typeid(soa_vertices), @@ -244,7 +250,8 @@ TEST_CASE("registering a translator for a type outside the concept throws", TEST_CASE("registering a second translator for the same type pair throws", "[data_product_concept]") { - auto con = concept_with_both_types(); + data_product_concept con{"VertexCollection"}; + add_both_types(con); con.add_translator("one_to_two", typeid(soa_vertices), @@ -271,11 +278,11 @@ TEST_CASE("add_concrete_types accepts its own set", "[data_product_concept]") con.add_concrete_types(con.concrete_types()); CHECK(con.concrete_types().empty()); - con.add_concrete_type(typeid(test_type_1)); - con.add_concrete_type(typeid(test_type_2)); + con.add_concrete_type(typeid(soa_vertices)); + con.add_concrete_type(typeid(aos_vertices)); con.add_concrete_types(con.concrete_types()); CHECK(con.concrete_types().size() == 2ull); - CHECK(con.has_concrete_type(typeid(test_type_1))); - CHECK(con.has_concrete_type(typeid(test_type_2))); + CHECK(con.has_concrete_type(typeid(soa_vertices))); + CHECK(con.has_concrete_type(typeid(aos_vertices))); } From cb2f7aa62f50120b5e8b65449d8e6c192d186f25 Mon Sep 17 00:00:00 2001 From: Marc Paterno Date: Tue, 3 Nov 2026 20:58:50 -0600 Subject: [PATCH 4/4] test: cover unknown-type and multi-concept concept lookups - Split the unknown-type case into sections for an empty registry and a registry whose concepts all lack the queried type. - Add a non-matching distractor concept to the multi-concept lookup test so an implementation that returns an arbitrary concept fails. Require a non-null result and check that it contains the queried type and is not the distractor. --- test/data_product_concept_registry_test.cpp | 28 ++++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/test/data_product_concept_registry_test.cpp b/test/data_product_concept_registry_test.cpp index 5ded812cd..9dc7d876b 100644 --- a/test/data_product_concept_registry_test.cpp +++ b/test/data_product_concept_registry_test.cpp @@ -270,11 +270,23 @@ TEST_CASE("get_concept_for_type returns the associated concept", "[data_product_ TEST_CASE("get_concept_for_type returns nullptr for an unknown type", "[data_product_concept_registry]") { - data_product_concept_registry const registry; - CHECK(registry.get_concept_for_type(typeid(int)) == nullptr); + SECTION("empty registry") + { + data_product_concept_registry const registry; + CHECK(registry.get_concept_for_type(typeid(int)) == nullptr); + } + + SECTION("registry whose concepts all lack the type") + { + data_product_concept_registry registry; + registry.add_concrete_type("concept_a", typeid(test_type_1)); + registry.add_concrete_type("concept_b", typeid(test_type_2)); + + CHECK(registry.get_concept_for_type(typeid(int)) == nullptr); + } } -TEST_CASE("get_concept_for_type returns a non-null match when a type belongs to multiple " +TEST_CASE("get_concept_for_type returns a matching concept when a type belongs to multiple " "concepts", "[data_product_concept_registry]") { @@ -282,8 +294,16 @@ TEST_CASE("get_concept_for_type returns a non-null match when a type belongs to registry.add_concrete_type("concept_a", typeid(test_type_1)); registry.add_concrete_type("concept_b", typeid(test_type_1)); + // "concept_other" is a distractor: it does not contain test_type_1, so it must never be + // returned for a test_type_1 query. Without it, every concept in the registry would match, + // and an implementation that returned an arbitrary concept without checking the type would + // still pass. Which of "concept_a" and "concept_b" is returned is unspecified (it depends + // on the registry's iteration order), so the checks below do not assume either one. + registry.add_concrete_type("concept_other", typeid(test_type_2)); auto const* found = registry.get_concept_for_type(typeid(test_type_1)); - CHECK(found != nullptr); + REQUIRE(found != nullptr); CHECK(found->has_concrete_type(typeid(test_type_1))); + CHECK_FALSE(found->has_concrete_type(typeid(test_type_2))); + CHECK(found->name() != "concept_other"); }