Bug fix in allocator_storage, refactor dispatch

patrickroberts · patrickroberts · commit d81eba476c3b · 2025-08-18T23:27:58.000-05:00
diff --git a/include/simply/dyn.hpp b/include/simply/dyn.hpp
@@ -3,7 +3,6 @@
 
 #include <simply/concepts.hpp>
 #include <simply/dispatch.hpp>
-#include <simply/elide.hpp>
 #include <simply/iface.hpp>
 #include <simply/impl.hpp>
 #include <simply/storage.hpp>
@@ -106,6 +105,7 @@ struct affordance_type<simply::dyn<Affordance, Storage, Dispatch>> {
   using type = Affordance;
 };
 
+// dynamic dispatch for member affordances of dyn
 template <simply::member_affordance Affordance,
           simply::specialization_of<simply::dyn> Dyn, typename R, typename Self,
           typename... Args, bool NoExcept>
@@ -116,17 +116,19 @@ struct impl<Affordance, Dyn, R(Self, Args...) noexcept(NoExcept)> {
   }
 };
 
-// TODO refactor impl specializations below to not assume allocator_storage
+// TODO refactor specializations below to not assume allocator_storage
 template <typename Dyn>
 concept _allocator_storage_dyn =
     simply::specialization_of<Dyn, simply::dyn> and
     simply::specialization_of<typename Dyn::storage_type,
                               simply::allocator_storage>;
 
-// TODO resolve storage type from choices by checking each compatibility with T
-template <typename T, typename Dyn>
-inline constexpr const auto &_storage_fn =
-    simply::fn<typename Dyn::storage_type, T>;
+// only copy the storage subobject of Dyn
+template <simply::_allocator_storage_dyn Dyn>
+struct affordance_traits<simply::copyable, Dyn> {
+  using function_type =
+      simply::iface<typename Dyn::storage_type, Dyn>(const Dyn &);
+};
 
 template <simply::member_affordance Affordance, typename T,
           simply::_allocator_storage_dyn Dyn, typename R, typename Self,
@@ -136,48 +138,28 @@ struct impl<simply::impl<Affordance, T>, Dyn,
   static constexpr auto fn(Self dyn, Args... args) noexcept(NoExcept) -> R {
     using self_type = simply::apply_cvref_t<Self, T>;
 
-    const auto pointer = simply::_storage_fn<T, Dyn>(dyn.get());
+    const auto pointer = simply::_dispatch_fn<T, Dyn>(dyn);
     return simply::fn<Affordance, T>(std::forward<self_type>(*pointer),
                                      std::forward<Args>(args)...);
   }
 };
 
-template <simply::constructor_affordance Affordance, typename T,
-          simply::_allocator_storage_dyn Dyn, typename R, typename Self,
-          typename... Args, bool NoExcept>
-struct impl<simply::impl<Affordance, T>, Dyn,
-            R(Self, Args...) noexcept(NoExcept)> {
-  static_assert(std::same_as<R, Dyn>);
-
-  static constexpr auto fn(Self dyn, Args... args) -> Dyn {
-    using self_type = simply::apply_cvref_t<Self, T>;
-    return Dyn{
-        std::allocator_arg,
-        dyn.get_allocator(),
-        std::in_place_type<T>,
-        simply::elide([&] -> T {
-          const auto pointer = simply::_storage_fn<T, Dyn>(dyn.get());
-          return simply::fn<Affordance, T>(std::forward<self_type>(*pointer),
-                                           std::forward<Args>(args)...);
-        }),
-    };
-  }
-};
-
+// TODO should this be specialized for simply::destructible only?
 template <simply::fundamental_destroy_affordance Affordance, typename T,
           simply::_allocator_storage_dyn Dyn, typename R, typename Self,
           typename... Args, bool NoExcept>
 struct impl<simply::impl<Affordance, T>, Dyn,
             R(Self, Args...) noexcept(NoExcept)> {
-  static_assert(std::same_as<R(Self, Args...), void(Dyn &)> and NoExcept);
+  static_assert(
+      std::same_as<R(Self, Args...) noexcept(NoExcept), void(Dyn &) noexcept>);
 
   static constexpr void fn(Dyn &dyn) noexcept {
     if (dyn.valueless_after_move()) {
       return;
     }
 
     auto alloc = simply::_rebind_alloc<T>(dyn.get_allocator());
-    const auto pointer = simply::_storage_fn<T, Dyn>(dyn.get());
+    const auto pointer = simply::_dispatch_fn<T, Dyn>(dyn);
 
     using traits = std::allocator_traits<decltype(alloc)>;
 
diff --git a/include/simply/storage.hpp b/include/simply/storage.hpp
@@ -1,6 +1,8 @@
 #ifndef SIMPLY_STORAGE_HPP
 #define SIMPLY_STORAGE_HPP
 
+#include <simply/copyable.hpp>
+#include <simply/elide.hpp>
 #include <simply/iface.hpp>
 #include <simply/impl.hpp>
 #include <simply/scope.hpp>
@@ -15,30 +17,91 @@ template <typename T, typename Alloc>
   return allocator_type(alloc);
 }
 
+template <typename Rebound, typename... Args>
+[[nodiscard]] static constexpr auto _construct_with(Rebound rebound,
+                                                    Args &&...args) {
+  using rebound_traits = std::allocator_traits<Rebound>;
+  auto pointer = rebound_traits::allocate(rebound, 1);
+  // defer deallocation if construct throws
+  auto dealloc = simply::scope_exit{
+      [&] { rebound_traits::deallocate(rebound, pointer, 1); },
+  };
+
+  rebound_traits::construct(rebound, std::to_address(pointer),
+                            std::forward<Args>(args)...);
+  // cancel deallocation if construct returns
+  dealloc.release();
+  return pointer;
+}
+
 template <typename Allocator = std::allocator<std::byte>>
 struct allocator_storage : simply::storage_affordance_base {
   using traits = std::allocator_traits<Allocator>;
   using void_pointer = traits::void_pointer;
   using const_void_pointer = traits::const_void_pointer;
 
-  // TODO express this by dispatching directly from iface to reduce boilerplate
   template <typename T>
-  struct fn_t {
+  struct dispatch : simply::member_affordance_base {
     using rebound = traits::template rebind_traits<T>;
     using pointer = rebound::pointer;
     using const_pointer = rebound::const_pointer;
 
-    static constexpr auto operator()(void_pointer ptr) noexcept {
-      return static_cast<pointer>(ptr);
-    }
+    template <typename Self>
+    struct fn_t {
+      static constexpr auto operator()(Self &self) noexcept {
+        return static_cast<pointer>(self.get());
+      }
 
-    static constexpr auto operator()(const_void_pointer ptr) noexcept {
-      return static_cast<const_pointer>(ptr);
-    }
+      static constexpr auto operator()(const Self &self) noexcept {
+        return static_cast<const_pointer>(self.get());
+      }
+    };
+
+    template <typename Self>
+    static constexpr fn_t<Self> fn{};
   };
+};
 
-  template <typename T>
-  static constexpr fn_t<T> fn{};
+// TODO resolve storage type from choices by checking each compatibility with T
+template <typename T, typename Dyn>
+inline constexpr const auto &_dispatch_fn =
+    simply::fn<typename Dyn::storage_type::template dispatch<T>>;
+
+template <simply::fundamental_copy_affordance Copy, typename T,
+          simply::specialization_of<simply::allocator_storage> Storage,
+          typename Dyn, typename R, typename Self, typename... Args,
+          bool NoExcept>
+struct impl<simply::impl<Copy, T>, simply::iface<Storage, Dyn>,
+            R(Self, Args...) noexcept(NoExcept)> {
+  using iface_type = simply::iface<Storage, Dyn>;
+  static_assert(std::same_as<R(Self, Args...) noexcept(NoExcept),
+                             iface_type(const iface_type &)>);
+
+  // static dispatch for copy affordance of dyn with allocator storage
+  static constexpr auto fn(Self self) -> R {
+    using type = typename simply::iface<Storage, Dyn>::allocator_type;
+    using traits = std::allocator_traits<type>;
+
+    const auto alloc =
+        traits::select_on_container_copy_construction(self.get_allocator());
+
+    if (self.valueless_after_move()) {
+      return R{std::allocator_arg, alloc, impl{}};
+    }
+
+    return R{
+        std::allocator_arg,
+        alloc,
+        std::in_place_type<T>,
+        simply::elide([&] -> T {
+          const auto pointer = simply::_dispatch_fn<T, Dyn>(self);
+          return simply::fn<Copy, T>(*pointer);
+        }),
+    };
+  }
+
+private:
+  impl() noexcept = default;
 };
 
 template <simply::specialization_of<simply::allocator_storage> Storage,
@@ -48,23 +111,6 @@ struct iface<Storage, Self> {
 private:
   using traits = Storage::traits;
 
-  template <typename Rebound, typename... Args>
-  [[nodiscard]] static constexpr auto _construct_with(Rebound rebound,
-                                                      Args &&...args) {
-    using rebound_traits = std::allocator_traits<Rebound>;
-    auto pointer = rebound_traits::allocate(rebound, 1);
-    // defer deallocation if construct throws
-    auto dealloc = simply::scope_exit{
-        [&] { rebound_traits::deallocate(rebound, pointer, 1); },
-    };
-
-    rebound_traits::construct(rebound, std::to_address(pointer),
-                              std::forward<Args>(args)...);
-    // cancel deallocation if construct returns
-    dealloc.release();
-    return pointer;
-  }
-
   [[nodiscard]] constexpr auto _release() noexcept {
     return std::exchange(object_ptr, nullptr);
   }
@@ -74,16 +120,24 @@ struct iface<Storage, Self> {
   using void_pointer = traits::void_pointer;
   using const_void_pointer = traits::const_void_pointer;
 
-  // BUG use this->alloc instead of other.alloc to copy other->object_ptr
-  constexpr iface(const iface &other)
-      : alloc(traits::select_on_container_copy_construction(other.alloc)),
-        object_ptr(
-            other.valueless_after_move()
-                ? nullptr
-                : simply::fn<
-                      simply::copy_affordance_t<typename Self::affordance_type>,
-                      Self>(static_cast<const Self &>(other))
-                      ._release()) {}
+  // effectively private "valueless" constructor
+  template <typename Alloc, simply::fundamental_copy_affordance Copy,
+            typename T>
+  constexpr iface(std::allocator_arg_t alloc_tag, const Alloc &alloc,
+                  simply::impl<simply::impl<Copy, T>, iface> tag) noexcept
+    requires std::default_initializable<void_pointer>
+      : alloc(alloc), object_ptr() {}
+
+  constexpr iface(const iface &other) {
+    using copy = simply::copy_affordance_t<typename Self::affordance_type>;
+
+    std::construct_at(this, simply::elide([&] {
+                        // TODO template storage on dispatch to access
+                        // get_member() without downcasting
+                        return simply::fn<copy>(
+                            static_cast<const Self &>(other));
+                      }));
+  }
 
   constexpr iface(iface &&other) noexcept
       : alloc(other.alloc), object_ptr(other._release()) {}
@@ -93,16 +147,17 @@ struct iface<Storage, Self> {
                            const Alloc &alloc,
                            [[maybe_unused]] std::in_place_type_t<T> obj_tag,
                            Args &&...args)
-      : alloc(alloc),
-        object_ptr(_construct_with(simply::_rebind_alloc<T>(this->alloc),
-                                   std::forward<Args>(args)...)) {}
+      : alloc(alloc), object_ptr(simply::_construct_with(
+                          simply::_rebind_alloc<T>(this->alloc),
+                          std::forward<Args>(args)...)) {}
 
   template <typename T, typename... Args>
   constexpr explicit iface([[maybe_unused]] std::in_place_type_t<T> tag,
                            Args &&...args)
     requires std::default_initializable<allocator_type>
-      : alloc(), object_ptr(_construct_with(simply::_rebind_alloc<T>(alloc),
-                                            std::forward<Args>(args)...)) {}
+      : alloc(),
+        object_ptr(simply::_construct_with(simply::_rebind_alloc<T>(alloc),
+                                           std::forward<Args>(args)...)) {}
 
   auto operator=(const iface &other) -> iface & = default;
   auto operator=(iface &&other) noexcept -> iface & = default;
diff --git a/include/simply/type_traits.hpp b/include/simply/type_traits.hpp
@@ -27,14 +27,17 @@ struct dispatch_affordance_base : simply::affordance_base {};
 template <typename Affordance, typename T>
 struct affordance_traits;
 
+template <typename Affordance, typename T>
+using function_type_t = simply::affordance_traits<Affordance, T>::function_type;
+
 template <typename T>
 struct affordance_type;
 
 template <typename T>
-using affordance_type_t = affordance_type<T>::type;
+using affordance_type_t = simply::affordance_type<T>::type;
 
 template <typename Affordance, typename T,
-          typename Fn = affordance_traits<Affordance, T>::function_type>
+          typename Fn = simply::function_type_t<Affordance, T>>
 struct impl;
 
 template <typename Affordance, typename Tag>
diff --git a/include/simply/vtable.hpp b/include/simply/vtable.hpp
@@ -1,6 +1,7 @@
 #ifndef SIMPLY_VTABLE_HPP
 #define SIMPLY_VTABLE_HPP
 
+#include <simply/iface.hpp>
 #include <simply/impl.hpp>
 
 namespace simply {
@@ -19,7 +20,14 @@ struct vtable<Affordance, Self> {};
 
 template <simply::member_affordance Affordance, typename Self>
 struct vtable<Affordance, Self> {
-  simply::affordance_traits<Affordance, Self>::function_type *fn;
+  simply::function_type_t<Affordance, Self> *fn;
+};
+
+// TODO specialize affordance_traits so this is unnecessary
+template <simply::constructor_affordance Affordance, typename Self>
+struct vtable<Affordance, Self> {
+  using iface_type = simply::iface<typename Self::storage_type, Self>;
+  simply::function_type_t<Affordance, iface_type> *fn;
 };
 
 template <simply::affordance Affordance, typename Self, typename T>
@@ -45,6 +53,14 @@ inline constexpr simply::vtable<Affordance, Self>
         .fn = &simply::fn<simply::impl<Affordance, T>, Self>,
 };
 
+// TODO specialize impl so this is unnecessary
+template <simply::constructor_affordance Affordance, typename Self, typename T>
+inline constexpr simply::vtable<Affordance, Self>
+    vtable_for<Affordance, Self, T> = {
+        .fn = &simply::fn<simply::impl<Affordance, T>,
+                          simply::iface<typename Self::storage_type, Self>>,
+};
+
 } // namespace simply
 
 #endif // SIMPLY_VTABLE_HPP
