diff --git a/CMakeLists.txt b/CMakeLists.txt index c71fc9af1..332669d09 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -22,12 +22,12 @@ FetchContent_Declare( ) # ... and Microsoft's C++ Guideline Support Library FetchContent_Declare( - GSL + Microsoft.GSL GIT_REPOSITORY https://github.com/microsoft/GSL GIT_TAG v4.2.0 GIT_SHALLOW ON EXCLUDE_FROM_ALL # Do not install - FIND_PACKAGE_ARGS NAMES Microsoft.GSL + FIND_PACKAGE_ARGS ) # ... and the Mimic C++ mocking framework FetchContent_Declare( @@ -90,7 +90,7 @@ set(DART_TESTING_TIMEOUT 90 CACHE STRING "Timeout (s) for Dart/CTest runs") set(CTEST_TEST_TIMEOUT 90 CACHE STRING "Per-test timeout (s) for CTest") # Make tools available -FetchContent_MakeAvailable(Catch2 GSL mimicpp) +FetchContent_MakeAvailable(Catch2 Microsoft.GSL mimicpp) # Avoid use of __COUNTER__ to prevent -pedantic warnings in Catch2's TEST_CASE macro set(CATCH_CONFIG_COUNTER OFF) @@ -163,6 +163,7 @@ find_package(Boost REQUIRED COMPONENTS json EXPORT) find_package(TBB REQUIRED EXPORT) find_package(fmt REQUIRED EXPORT) find_package(spdlog REQUIRED EXPORT) +find_package(Microsoft.GSL REQUIRED EXPORT) # Dependencies required by internal implementation find_package(Boost REQUIRED COMPONENTS program_options) diff --git a/phlex/driver.hpp b/phlex/driver.hpp index 64c9d8c33..2e15673ae 100644 --- a/phlex/driver.hpp +++ b/phlex/driver.hpp @@ -12,7 +12,8 @@ #include "boost/mp11/algorithm.hpp" #include "fmt/format.h" -#include +#include + #include #include #include @@ -35,13 +36,13 @@ namespace phlex::detail { using driver_shim_t = void(driver_proxy const&, configuration const&, driver_bundle*); template - std::remove_cvref_t const& as_driver_source(source const* src, std::size_t index) + std::remove_cvref_t const& as_driver_source(gsl::not_null src, + std::size_t index) { - assert(src != nullptr); - using expected_source_t = std::remove_cvref_t; - if (auto const* casted = dynamic_cast(src)) { + auto const* src_ptr = src.get(); + if (auto const* casted = dynamic_cast(src_ptr)) { return *casted; } @@ -49,7 +50,7 @@ namespace phlex::detail { fmt::format("Driver source type mismatch at source index {}: expected '{}' but got '{}'.", index, boost::core::demangle(typeid(expected_source_t).name()), - boost::core::demangle(typeid(*src).name()))); + boost::core::demangle(typeid(*src_ptr).name()))); } template diff --git a/phlex/model/CMakeLists.txt b/phlex/model/CMakeLists.txt index e9eb1ad5f..76d1fd776 100644 --- a/phlex/model/CMakeLists.txt +++ b/phlex/model/CMakeLists.txt @@ -20,6 +20,7 @@ cet_make_library( Boost::boost spdlog::spdlog fmt::fmt + Microsoft.GSL::GSL PRIVATE phlex::utilities TBB::tbb diff --git a/phlex/model/fixed_hierarchy.hpp b/phlex/model/fixed_hierarchy.hpp index 805d773be..24b6b8f47 100644 --- a/phlex/model/fixed_hierarchy.hpp +++ b/phlex/model/fixed_hierarchy.hpp @@ -6,6 +6,8 @@ #include "phlex/model/fwd.hpp" #include "phlex/model/layer_path.hpp" +#include + #include #include #include @@ -37,8 +39,8 @@ namespace phlex { data_cell_index_ptr index_; // Non-owning pointers to the enclosing hierarchy and driver; data_cell_cursor is a // short-lived, freely copyable view and does not manage their lifetimes. - fixed_hierarchy const* hierarchy_; - detail::framework_driver* driver_; + gsl::not_null hierarchy_; + gsl::not_null driver_; }; /// @brief Callable object that yields data cells to the framework driver. diff --git a/phlex/model/handle.hpp b/phlex/model/handle.hpp index b807b54a0..551950cf0 100644 --- a/phlex/model/handle.hpp +++ b/phlex/model/handle.hpp @@ -5,6 +5,8 @@ #include "phlex/model/fwd.hpp" #include "phlex/model/product_specification.hpp" +#include + #include #include #include @@ -83,7 +85,7 @@ namespace phlex { handle(handle&&) noexcept = default; handle& operator=(handle&&) noexcept = default; - const_pointer operator->() const noexcept { return product_; } + const_pointer operator->() const noexcept { return product_.get(); } [[nodiscard]] const_reference operator*() const noexcept { return *operator->(); } // NOLINTBEGIN(google-explicit-constructor) - Implicit conversion is intentional operator const_reference() const noexcept { return operator*(); } @@ -116,8 +118,10 @@ namespace phlex { } private: - const_pointer product_; // Non-null, by construction - class data_cell_index const* id_; // Non-null, by construction + // Non-owning pointers to the product and its data-cell index; both are non-null by + // construction, as enforced by gsl::not_null. + gsl::not_null product_; + gsl::not_null id_; experimental::identifier creator_plugin_; experimental::identifier creator_algorithm_; experimental::identifier suffix_; diff --git a/phlex/model/products.cpp b/phlex/model/products.cpp index 12953f894..b10ba158e 100644 --- a/phlex/model/products.cpp +++ b/phlex/model/products.cpp @@ -17,7 +17,7 @@ namespace phlex::detail { products::size_type products::size() const noexcept { return products_.size(); } bool products::empty() const noexcept { return products_.empty(); } - product_base const* products::find_product(product_specification const& spec) const + gsl::not_null products::find_product(product_specification const& spec) const { auto it = std::ranges::find(products_, spec, [](auto const& p) -> auto const& { return p.first; }); diff --git a/phlex/model/products.hpp b/phlex/model/products.hpp index 2732a09f7..fd664686d 100644 --- a/phlex/model/products.hpp +++ b/phlex/model/products.hpp @@ -5,9 +5,12 @@ #include "phlex/model/product_specification.hpp" +#include + #include #include #include +#include #include #include #include @@ -46,7 +49,10 @@ namespace phlex::detail { product_ptr product_for(T&& t) { if constexpr (std::convertible_to) { - return std::forward(t); + if (auto product = product_ptr{std::forward(t)}) { + return product; + } + throw std::runtime_error("Cannot store a null product pointer."); } else { return std::make_unique>>(std::forward(t)); } @@ -87,9 +93,9 @@ namespace phlex::detail { template T const& get(product_specification const& spec) const { - auto const* available_product = find_product(spec); + auto const available_product = find_product(spec); - if (auto const* desired_product = dynamic_cast const*>(available_product)) { + if (auto const* desired_product = dynamic_cast const*>(available_product.get())) { return desired_product->obj; } @@ -102,7 +108,9 @@ namespace phlex::detail { bool empty() const noexcept; private: - product_base const* find_product(product_specification const& spec) const; + // Throws if no product matches the specification; the returned pointer is therefore + // never null. + gsl::not_null find_product(product_specification const& spec) const; static void throw_mismatched_type [[noreturn]] (product_specification const& spec, char const* requested_type, char const* available_type); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index bb5197188..1ac190098 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -191,6 +191,9 @@ cet_test(product_handle USE_CATCH2_MAIN SOURCE product_handle_test.cpp LIBRARIES cet_test(product_store USE_CATCH2_MAIN SOURCE product_store_test.cpp LIBRARIES phlex::core_internal ) +cet_test(products USE_CATCH2_MAIN SOURCE products_test.cpp LIBRARIES + phlex::model_internal +) cet_test( fold USE_CATCH2_MAIN diff --git a/test/products_test.cpp b/test/products_test.cpp new file mode 100644 index 000000000..ee9c7b34d --- /dev/null +++ b/test/products_test.cpp @@ -0,0 +1,11 @@ +#include "phlex/model/products.hpp" + +#include "catch2/catch_test_macros.hpp" +#include "catch2/matchers/catch_matchers.hpp" + +TEST_CASE("Reject null product pointers", "[data model]") +{ + using namespace phlex::detail; + CHECK_THROWS_WITH(product_for(product_ptr{}), "Cannot store a null product pointer."); + CHECK_THROWS_WITH(product_for(nullptr), "Cannot store a null product pointer."); +}