Handles are valid upon construction

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 <cassert>

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 <cstddef>
 #include <string>
@@ -13,7 +14,7 @@
 namespace phlex::experimental {
   template <typename T, std::size_t JoinNodePort>
   struct retriever {
-    using handle_arg_t = typename handle_for<T>::value_type;
+    using handle_arg_t = detail::handle_value_type<T>;
     specified_label label;
     auto retrieve(auto const& messages) const
     {

phlex/core/message.hpp

@@ -95,14 +95,6 @@ namespace phlex::experimental {
       return join;
     }
   }
-
-  template <typename T>
-  auto get_handle_for(message const& msg, std::string const& product_label)
-  {
-    using handle_arg_t = typename handle_for<T>::value_type;
-    return msg.store->get_handle<handle_arg_t>(product_label);
-  }
-
 }
 
 #endif // PHLEX_CORE_MESSAGE_HPP

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 <string>
 #include <type_traits>
 #include <utility>
 #include <variant>
 
 namespace phlex::experimental {
+  template <typename T>
+  class handle;
 
   namespace detail {
     template <typename T>
-    using handle_type = std::remove_cvref_t<T>;
+    struct handle_value_type_impl {
+      using type = std::remove_const_t<T>;
+    };
+
+    template <typename T>
+    struct handle_value_type_impl<T&> {
+      static_assert(std::is_const_v<T>,
+                    "If template argument to handle_for is a reference, it must be const.");
+      using type = std::remove_const_t<T>;
+    };
+
+    template <typename T>
+    struct handle_value_type_impl<T*> {
+      static_assert(std::is_const_v<T>,
+                    "If template argument to handle_for is a pointer, the pointee must be const.");
+      using type = std::remove_const_t<T>;
+    };
+
+    // Users are allowed to specify handle<T> as a parameter type to their algorithm
+    template <typename T>
+    struct handle_value_type_impl<handle<T>> {
+      using type = typename handle_value_type_impl<T>::type;
+    };
 
-    template <typename T, typename U>
-    concept same_handle_type = std::same_as<handle_type<T>, handle_type<U>>;
+    template <typename T>
+    using handle_value_type = typename handle_value_type_impl<T>::type;
   }
 
+  // ==============================================================================================
   template <typename T>
   class handle {
-    using err_t = std::string;
-
   public:
-    using value_type = detail::handle_type<T>;
+    static_assert(std::same_as<T, detail::handle_value_type<T>>,
+                  "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 <typename U>
-    explicit handle(product<U> const& prod, level_id const& id = level_id::base())
-      requires detail::same_handle_type<T, U>
-      : rep_{&prod.obj}, id_{&id}
+    // The 'product' parameter is not 'const_reference' to avoid avoid implicit type conversions.
+    explicit handle(std::same_as<T> auto const& product, level_id const& id) :
+      product_{&product}, id_{&id}
     {
     }
 
-    explicit handle(std::variant<const_pointer, err_t> 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<err_t>(&rep_)) {
-        throw std::runtime_error(*err);
-      }
-      return get<const_pointer>(rep_);
-    }
-    [[nodiscard]] const_reference operator*() const { return *operator->(); }
-    explicit operator bool() const noexcept { return get_if<const_pointer>(&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 <typename U>
     friend class handle;
 
-    template <typename U>
-    bool operator==(handle<U> rhs) const noexcept
-      requires detail::same_handle_type<T, U>
+    bool operator==(handle other) const noexcept
     {
-      return rep_ == rhs.rep_;
+      return product_ == other.product_ and id_ == other.id_;
     }
 
   private:
-    std::variant<const_pointer, err_t> rep_{"Cannot dereference empty handle of type '" +
-                                            boost::core::demangle(typeid(T).name()) + "'."};
-    class level_id const* id_;
-  };
-
-  template <typename T>
-  handle(product<T> const&) -> handle<T>;
-
-  template <typename T>
-  struct handle_ {
-    using type = handle<std::remove_const_t<T>>;
-  };
-
-  template <typename T>
-  struct handle_<T&> {
-    static_assert(std::is_const_v<T>,
-                  "If template argument to handle_for is a reference, it must be const.");
-    using type = handle<std::remove_const_t<T>>;
-  };
-
-  template <typename T>
-  struct handle_<T*> {
-    static_assert(std::is_const_v<T>,
-                  "If template argument to handle_for is a pointer, the pointee must be const.");
-    using type = handle<std::remove_const_t<T>>;
-  };
-
-  template <typename T>
-  struct handle_<handle<T>> {
-    using type = handle<T>;
+    const_pointer product_;    // Non-null, by construction
+    class level_id const* id_; // Non-null, by construction
   };
 
   template <typename T>
-  using handle_for = typename handle_<T>::type;
+  handle(T const&, level_id const&) -> handle<T>;
 }
 
 #endif // PHLEX_MODEL_HANDLE_HPP

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) + "'.");
   }
 }

phlex/model/products.hpp

@@ -69,37 +69,38 @@ namespace phlex::experimental {
     }
 
     template <typename T>
-    std::variant<T const*, std::string> 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<product<T> const*>(it->second.get())) {
-      //     return &t->obj;
+      //     return t->obj;
       //   }
       //
       // Unfortunately, this doesn't work well whenever products are inserted across
       // modules and shared object libraries.
 
       auto available_product = it->second.get();
       if (std::strcmp(typeid(T).name(), available_product->type().name()) == 0) {
-        return &reinterpret_cast<product<T> const*>(available_product)->obj;
+        return reinterpret_cast<product<T> 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;
     const_iterator begin() const noexcept;
     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_;
   };

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 <concepts>
 #include <string>
@@ -18,39 +18,89 @@ namespace {
 
 TEST_CASE("Handle type conversions (compile-time checks)", "[data model]")
 {
-  static_assert(std::same_as<handle_for<int>, handle<int>>);
-  static_assert(std::same_as<handle_for<int const>, handle<int>>);
-  static_assert(std::same_as<handle_for<int const&>, handle<int>>);
-  static_assert(std::same_as<handle_for<int const*>, handle<int>>);
+  using detail::handle_value_type;
+  static_assert(std::same_as<handle_value_type<int>, int>);
+  static_assert(std::same_as<handle_value_type<int const>, int>);
+  static_assert(std::same_as<handle_value_type<int const&>, int>);
+  static_assert(std::same_as<handle_value_type<int const*>, int>);
+  static_assert(std::same_as<handle_value_type<handle<int>>, 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<handle<int>, product<int>>);
-  static_assert(std::constructible_from<handle<int>, product<int>, level_id>);
-  static_assert(not std::constructible_from<handle<int>, product<double>>);
-  static_assert(not std::constructible_from<handle<int>, product<double>, level_id>);
+  static_assert(std::constructible_from<handle<int>, handle<int> const&>); // Copies
+  static_assert(std::constructible_from<handle<int>, handle<int>&&>);      // Moves
+  static_assert(not std::constructible_from<handle<int>, handle<double>>);
+
+  static_assert(std::constructible_from<handle<int>, int, level_id>);
+  static_assert(std::constructible_from<handle<int>, int const, level_id>);
+  static_assert(std::constructible_from<handle<int>, int const&, level_id>);
+  static_assert(not std::constructible_from<handle<int>, double, level_id>);
+}
+
+TEST_CASE("Can only assign handles with compatible types (compile-time checks)", "[data model]")
+{
+  static_assert(std::assignable_from<handle<int>&, handle<int> const&>); // Copies
+  static_assert(std::assignable_from<handle<int>&, handle<int>&&>);      // Moves
+  static_assert(not std::assignable_from<handle<int>&, handle<double> 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<double> empty;
-  CHECK(not empty);
-  CHECK_THROWS_WITH(
-    *empty,
-    Catch::Matchers::ContainsSubstring("Cannot dereference empty handle of type 'double'."));
-
-  product<int> const number{3};
-  handle const h{number};
-  CHECK(handle<int const>{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<bool>(h));
-  CHECK(num_ref == 3);
-  CHECK(*num_ptr == 3);
+  CHECK(num_ref == number);
+  CHECK(*num_ptr == number);
 
-  product<Composer> const composer{{"Elgar"}};
-  CHECK(handle{composer}->name == "Elgar");
+  Composer const composer{"Elgar"};
+  CHECK(handle{composer, level_id::base()}->name == "Elgar");
 }