diff --git a/phlex/core/glue.hpp b/phlex/core/glue.hpp index be8ef98c0..7f0cfdbb6 100644 --- a/phlex/core/glue.hpp +++ b/phlex/core/glue.hpp @@ -8,6 +8,7 @@ #include "phlex/metaprogramming/delegate.hpp" #include "phlex/utilities/stripped_name.hpp" +#include "boost/core/demangle.hpp" #include "oneapi/tbb/flow_graph.h" #include diff --git a/phlex/core/input_arguments.hpp b/phlex/core/input_arguments.hpp index 4572f94f8..864364a7b 100644 --- a/phlex/core/input_arguments.hpp +++ b/phlex/core/input_arguments.hpp @@ -3,6 +3,7 @@ #include "phlex/core/message.hpp" #include "phlex/core/specified_label.hpp" +#include "phlex/model/handle.hpp" #include #include @@ -13,7 +14,7 @@ namespace phlex::experimental { template struct retriever { - using handle_arg_t = typename handle_for::value_type; + using handle_arg_t = detail::handle_value_type; specified_label label; auto retrieve(auto const& messages) const { diff --git a/phlex/core/message.hpp b/phlex/core/message.hpp index 7f05c5391..6e7d3a3c5 100644 --- a/phlex/core/message.hpp +++ b/phlex/core/message.hpp @@ -95,14 +95,6 @@ namespace phlex::experimental { return join; } } - - template - auto get_handle_for(message const& msg, std::string const& product_label) - { - using handle_arg_t = typename handle_for::value_type; - return msg.store->get_handle(product_label); - } - } #endif // PHLEX_CORE_MESSAGE_HPP diff --git a/phlex/model/handle.hpp b/phlex/model/handle.hpp index 10f43af22..21987c2e5 100644 --- a/phlex/model/handle.hpp +++ b/phlex/model/handle.hpp @@ -2,59 +2,75 @@ #define PHLEX_MODEL_HANDLE_HPP #include "phlex/model/level_id.hpp" -#include "phlex/model/products.hpp" -#include "boost/core/demangle.hpp" - -#include #include #include #include namespace phlex::experimental { + template + class handle; namespace detail { template - using handle_type = std::remove_cvref_t; + struct handle_value_type_impl { + using type = std::remove_const_t; + }; + + template + struct handle_value_type_impl { + static_assert(std::is_const_v, + "If template argument to handle_for is a reference, it must be const."); + using type = std::remove_const_t; + }; + + template + struct handle_value_type_impl { + static_assert(std::is_const_v, + "If template argument to handle_for is a pointer, the pointee must be const."); + using type = std::remove_const_t; + }; + + // Users are allowed to specify handle as a parameter type to their algorithm + template + struct handle_value_type_impl> { + using type = typename handle_value_type_impl::type; + }; - template - concept same_handle_type = std::same_as, handle_type>; + template + using handle_value_type = typename handle_value_type_impl::type; } + // ============================================================================================== template class handle { - using err_t = std::string; - public: - using value_type = detail::handle_type; + static_assert(std::same_as>, + "Cannot create a handle with a template argument that is const-qualified, a " + "reference type, or a pointer type."); + using value_type = T; using const_reference = value_type const&; using const_pointer = value_type const*; - handle() = default; - - template - explicit handle(product const& prod, level_id const& id = level_id::base()) - requires detail::same_handle_type - : rep_{&prod.obj}, id_{&id} + // The 'product' parameter is not 'const_reference' to avoid avoid implicit type conversions. + explicit handle(std::same_as auto const& product, level_id const& id) : + product_{&product}, id_{&id} { } - explicit handle(std::variant maybe_product, level_id const& id) : - rep_{std::move(maybe_product)}, id_{&id} - { - } + // Handles cannot be invalid + handle() = delete; - explicit handle(std::string err_msg, level_id const& id) : rep_{std::move(err_msg)}, id_{&id} {} + // Copy operations + handle(handle const&) noexcept = default; + handle& operator=(handle const&) noexcept = default; - const_pointer operator->() const - { - if (auto const* err = get_if(&rep_)) { - throw std::runtime_error(*err); - } - return get(rep_); - } - [[nodiscard]] const_reference operator*() const { return *operator->(); } - explicit operator bool() const noexcept { return get_if(&rep_) != nullptr; } + // Move operations + handle(handle&&) noexcept = default; + handle& operator=(handle&&) noexcept = default; + + const_pointer operator->() const noexcept { return product_; } + [[nodiscard]] const_reference operator*() const noexcept { return *operator->(); } operator const_reference() const noexcept { return operator*(); } operator const_pointer() const noexcept { return operator->(); } @@ -63,48 +79,18 @@ namespace phlex::experimental { template friend class handle; - template - bool operator==(handle rhs) const noexcept - requires detail::same_handle_type + bool operator==(handle other) const noexcept { - return rep_ == rhs.rep_; + return product_ == other.product_ and id_ == other.id_; } private: - std::variant rep_{"Cannot dereference empty handle of type '" + - boost::core::demangle(typeid(T).name()) + "'."}; - class level_id const* id_; - }; - - template - handle(product const&) -> handle; - - template - struct handle_ { - using type = handle>; - }; - - template - struct handle_ { - static_assert(std::is_const_v, - "If template argument to handle_for is a reference, it must be const."); - using type = handle>; - }; - - template - struct handle_ { - static_assert(std::is_const_v, - "If template argument to handle_for is a pointer, the pointee must be const."); - using type = handle>; - }; - - template - struct handle_> { - using type = handle; + const_pointer product_; // Non-null, by construction + class level_id const* id_; // Non-null, by construction }; template - using handle_for = typename handle_::type; + handle(T const&, level_id const&) -> handle; } #endif // PHLEX_MODEL_HANDLE_HPP diff --git a/phlex/model/products.cpp b/phlex/model/products.cpp index 6a3eea86f..5127f5e22 100644 --- a/phlex/model/products.cpp +++ b/phlex/model/products.cpp @@ -13,12 +13,12 @@ namespace phlex::experimental { products::const_iterator products::begin() const noexcept { return products_.begin(); } products::const_iterator products::end() const noexcept { return products_.end(); } - std::string products::error_message(std::string const& product_name, - char const* requested_type, - char const* available_type) + void products::throw_mismatched_type(std::string const& product_name, + char const* requested_type, + char const* available_type) { - return "Cannot get product '" + product_name + "' with type '" + - boost::core::demangle(requested_type) + "' -- must specify type '" + - boost::core::demangle(available_type) + "'."; + throw std::runtime_error("Cannot get product '" + product_name + "' with type '" + + boost::core::demangle(requested_type) + "' -- must specify type '" + + boost::core::demangle(available_type) + "'."); } } diff --git a/phlex/model/products.hpp b/phlex/model/products.hpp index a66d6b7e8..f57d1513a 100644 --- a/phlex/model/products.hpp +++ b/phlex/model/products.hpp @@ -69,17 +69,17 @@ namespace phlex::experimental { } template - std::variant get(std::string const& product_name) const + T const& get(std::string const& product_name) const { auto it = products_.find(product_name); if (it == cend(products_)) { - return "No product exists with the name '" + product_name + "'."; + throw std::runtime_error("No product exists with the name '" + product_name + "'."); } // Should be able to use dynamic_cast a la: // // if (auto t = dynamic_cast const*>(it->second.get())) { - // return &t->obj; + // return t->obj; // } // // Unfortunately, this doesn't work well whenever products are inserted across @@ -87,9 +87,10 @@ namespace phlex::experimental { auto available_product = it->second.get(); if (std::strcmp(typeid(T).name(), available_product->type().name()) == 0) { - return &reinterpret_cast const*>(available_product)->obj; + return reinterpret_cast const*>(available_product)->obj; } - return error_message(product_name, typeid(T).name(), available_product->type().name()); + + throw_mismatched_type(product_name, typeid(T).name(), available_product->type().name()); } bool contains(std::string const& product_name) const; @@ -97,9 +98,9 @@ namespace phlex::experimental { const_iterator end() const noexcept; private: - static std::string error_message(std::string const& product_name, - char const* requested_type, - char const* available_type); + static void throw_mismatched_type [[noreturn]] (std::string const& product_name, + char const* requested_type, + char const* available_type); collection_t products_; }; diff --git a/test/product_handle.cpp b/test/product_handle.cpp index 97d72078b..403591a35 100644 --- a/test/product_handle.cpp +++ b/test/product_handle.cpp @@ -2,7 +2,7 @@ #include "phlex/model/level_id.hpp" #include "phlex/model/product_store.hpp" -#include "catch2/catch_all.hpp" +#include "catch2/catch_test_macros.hpp" #include #include @@ -18,39 +18,89 @@ namespace { TEST_CASE("Handle type conversions (compile-time checks)", "[data model]") { - static_assert(std::same_as, handle>); - static_assert(std::same_as, handle>); - static_assert(std::same_as, handle>); - static_assert(std::same_as, handle>); + using detail::handle_value_type; + static_assert(std::same_as, int>); + static_assert(std::same_as, int>); + static_assert(std::same_as, int>); + static_assert(std::same_as, int>); + static_assert(std::same_as>, int>); } -TEST_CASE("Can only create handles with compatible types", "[data model]") +TEST_CASE("Can only construct handles with compatible types (compile-time checks)", "[data model]") { - static_assert(std::constructible_from, product>); - static_assert(std::constructible_from, product, level_id>); - static_assert(not std::constructible_from, product>); - static_assert(not std::constructible_from, product, level_id>); + static_assert(std::constructible_from, handle const&>); // Copies + static_assert(std::constructible_from, handle&&>); // Moves + static_assert(not std::constructible_from, handle>); + + static_assert(std::constructible_from, int, level_id>); + static_assert(std::constructible_from, int const, level_id>); + static_assert(std::constructible_from, int const&, level_id>); + static_assert(not std::constructible_from, double, level_id>); +} + +TEST_CASE("Can only assign handles with compatible types (compile-time checks)", "[data model]") +{ + static_assert(std::assignable_from&, handle const&>); // Copies + static_assert(std::assignable_from&, handle&&>); // Moves + static_assert(not std::assignable_from&, handle const&>); +} + +TEST_CASE("Handle copies and moves", "[data model]") +{ + int const two{2}; + int const four{4}; + + auto job_data_cell = level_id::base_ptr(); + auto subrun_6_data_cell = job_data_cell->make_child(6, "subrun"); + + handle h2{two, *job_data_cell}; + handle h4{four, *subrun_6_data_cell}; + CHECK(h2 != h4); + + CHECK(handle{h2} == h2); + h2 = h4; + CHECK(h2 == h4); + CHECK(*h2 == 4); + + handle h3 = std::move(h4); + CHECK(*h3 == 4); + + h4 = h2; + CHECK(h2 == h4); + CHECK(*h4 == 4); + + h4 = std::move(h3); + CHECK(*h4 == 4); +} + +TEST_CASE("Handle comparisons", "[data model]") +{ + int const seventeen{17}; + int const eighteen{18}; + handle const h17{seventeen, level_id::base()}; + handle const h18{eighteen, level_id::base()}; + CHECK(h17 == h17); + CHECK(h17 != h18); + + auto subrun_6_data_cell = level_id::base_ptr()->make_child(6, "subrun"); + handle const h17sr{seventeen, *subrun_6_data_cell}; + CHECK(*h17 == *h17sr); // Products are the same + CHECK(h17.level_id() != h17sr.level_id()); // Levels are not the same + CHECK(h17 != h17sr); // Therefore handles are not the same } TEST_CASE("Handle type conversions (run-time checks)", "[data model]") { - handle empty; - CHECK(not empty); - CHECK_THROWS_WITH( - *empty, - Catch::Matchers::ContainsSubstring("Cannot dereference empty handle of type 'double'.")); - - product const number{3}; - handle const h{number}; - CHECK(handle{number} == h); + int const number{3}; + handle const h{number, level_id::base()}; CHECK(h.level_id() == level_id::base()); int const& num_ref = h; int const* num_ptr = h; CHECK(static_cast(h)); - CHECK(num_ref == 3); - CHECK(*num_ptr == 3); + CHECK(num_ref == number); + CHECK(*num_ptr == number); - product const composer{{"Elgar"}}; - CHECK(handle{composer}->name == "Elgar"); + Composer const composer{"Elgar"}; + CHECK(handle{composer, level_id::base()}->name == "Elgar"); } diff --git a/test/product_store.cpp b/test/product_store.cpp index 804645fcf..72623126f 100644 --- a/test/product_store.cpp +++ b/test/product_store.cpp @@ -21,12 +21,9 @@ TEST_CASE("Product store insertion", "[data model]") Catch::Matchers::ContainsSubstring( "Cannot get product 'number' with type 'double' -- must specify type 'int'.")); - auto invalid_handle = store->get_handle("wrong_key"); - CHECK(!invalid_handle); auto const matcher = Catch::Matchers::ContainsSubstring("No product exists with the name 'wrong_key'."); - CHECK_THROWS_WITH(*invalid_handle, matcher); - CHECK_THROWS_WITH(invalid_handle.operator->(), matcher); + CHECK_THROWS_WITH(store->get_handle("wrong_key"), matcher); CHECK(store->get_product("number") == number);