Constrain dyn constructors based on storage

Author: patrickroberts

include/simply/dyn.hpp

@@ -31,25 +31,24 @@ class dyn
 
   // TODO add default constructor for nullable mixin
 
-  // TODO generalize member constraints based on storage
   dyn(const dyn &other) = delete;
   constexpr dyn(const dyn &other)
-    requires simply::copy_mixin<Mixin>
+    requires simply::copy_constructible<storage_base>
   = default;
 
   dyn(dyn &&other) = delete;
-  constexpr dyn(dyn &&other) noexcept
-    requires simply::move_mixin<Mixin>
+  constexpr dyn(dyn &&other)
+    requires simply::move_constructible<storage_base>
   = default;
 
-  template <typename Alloc, simply::has_mixin<Mixin> T, typename... Args>
+  template <typename Alloc, simply::has_mixin<mixin_type> T, typename... Args>
     requires simply::constructible_from<T, Args...>
   constexpr explicit dyn(std::allocator_arg_t alloc_tag, const Alloc &alloc,
                          std::in_place_type_t<T> obj_tag, Args &&...args)
       : storage_base(alloc_tag, alloc, obj_tag, std::forward<Args>(args)...),
         dispatch_base(obj_tag) {}
 
-  template <simply::has_mixin<Mixin> T, typename... Args>
+  template <simply::has_mixin<mixin_type> T, typename... Args>
     requires simply::constructible_from<T, Args...>
   constexpr explicit dyn(std::in_place_type_t<T> tag, Args &&...args)
       : storage_base(tag, std::forward<Args>(args)...), dispatch_base(tag) {}
@@ -58,7 +57,7 @@ class dyn
     requires(not std::same_as<std::remove_cvref_t<T>, dyn>) and
                 (not simply::specialization_of<std::remove_cvref_t<T>,
                                                std::in_place_type_t>) and
-                simply::has_mixin<std::decay_t<T>, Mixin> and
+                simply::has_mixin<std::decay_t<T>, mixin_type> and
                 simply::constructible_from<std::decay_t<T>, T>
   constexpr explicit dyn(T &&value)
       : storage_base(std::in_place_type<std::decay_t<T>>,
@@ -67,7 +66,8 @@ class dyn
 
   auto operator=(const dyn &other) -> dyn & = delete;
   constexpr auto operator=(const dyn &other) -> dyn &
-    requires simply::destroy_mixin<Mixin> and simply::copy_mixin<Mixin>
+    requires simply::destroy_mixin<mixin_type> and
+             simply::copy_constructible<storage_base>
   {
     // prevent self-assignment
     if (this == std::addressof(other)) {
@@ -81,7 +81,8 @@ class dyn
 
   auto operator=(dyn &&other) -> dyn & = delete;
   constexpr auto operator=(dyn &&other) noexcept -> dyn &
-    requires simply::destroy_mixin<Mixin> and simply::move_mixin<Mixin>
+    requires simply::destroy_mixin<mixin_type> and
+             simply::move_constructible<storage_base>
   {
     std::destroy_at(this);
     std::construct_at(this, std::move(other));
@@ -92,9 +93,9 @@ class dyn
 
   ~dyn() = delete;
   constexpr ~dyn()
-    requires simply::destroy_mixin<Mixin>
+    requires simply::destroy_mixin<mixin_type>
   {
-    using destroy = simply::destroy_mixin_t<Mixin>;
+    using destroy = simply::destroy_mixin_t<mixin_type>;
     simply::fn<destroy, dyn>(*this);
   }
 };

include/simply/impl.hpp

@@ -19,15 +19,17 @@ inline constexpr const auto &fn = simply::impl<Mixin, T>::fn;
 
 template <typename Mixin>
 struct _deduce_t {
+
+// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
+#define SIMPLY_PERFECT_FORWARD(...)                                            \
+  noexcept(noexcept(__VA_ARGS__))->decltype(__VA_ARGS__) { return __VA_ARGS__; }
+
   template <typename T, typename... Args>
-  static constexpr auto operator()(T &&self, Args &&...args) noexcept(noexcept(
-      simply::fn<Mixin, std::remove_cvref_t<T>>(std::forward<T>(self),
-                                                std::forward<Args>(args)...)))
-      -> decltype(simply::fn<Mixin, std::remove_cvref_t<T>>(
-          std::forward<T>(self), std::forward<Args>(args)...)) {
-    return simply::fn<Mixin, std::remove_cvref_t<T>>(
-        std::forward<T>(self), std::forward<Args>(args)...);
-  }
+  static constexpr auto operator()(T &&self, Args &&...args)
+      SIMPLY_PERFECT_FORWARD(simply::fn<Mixin, std::remove_cvref_t<T>>(
+          std::forward<T>(self), std::forward<Args>(args)...))
+
+#undef SIMPLY_PERFECT_FORWARD
 };
 
 template <typename Mixin>

include/simply/storage.hpp

@@ -101,12 +101,13 @@ struct impl<simply::impl<Copy, T>, simply::iface<Storage, Dyn>,
   }
 
 private:
-  impl() noexcept = default;
+  impl() = default;
 };
 
 template <simply::specialization_of<simply::allocator_storage> Storage,
           typename Self>
   requires simply::mixin<Storage>
+// NOLINTNEXTLINE(cppcoreguidelines-special-member-functions)
 struct iface<Storage, Self> {
 private:
   using traits = Storage::traits;
@@ -127,6 +128,13 @@ struct iface<Storage, Self> {
     requires std::default_initializable<void_pointer>
       : alloc(alloc), object_ptr() {}
 
+  // copy constructor overload resolution needs to consider the template below
+  iface(const iface &)
+    requires false;
+
+  // defer instantiation until Self::mixin_type is accessible
+  template <typename T = Self>
+    requires simply::copy_mixin<typename T::mixin_type>
   constexpr iface(const iface &other) {
     using copy = simply::copy_mixin_t<typename Self::mixin_type>;
 
@@ -138,6 +146,7 @@ struct iface<Storage, Self> {
                       }));
   }
 
+  // allocator_storage can implement move construction without type erasure
   constexpr iface(iface &&other) noexcept
       : alloc(other.alloc), object_ptr(other._release()) {}
 
@@ -159,7 +168,7 @@ struct iface<Storage, Self> {
                                            std::forward<Args>(args)...)) {}
 
   auto operator=(const iface &other) -> iface & = default;
-  auto operator=(iface &&other) noexcept -> iface & = default;
+  auto operator=(iface &&other) -> iface & = default;
 
   ~iface() = default;
 

tests/concepts.cpp

@@ -62,7 +62,8 @@ static_assert(constructible_from_in_place<unusable, immovable>);
 static_assert(constructible_from_in_place<unusable, indestructible>);
 
 static_assert(not simply::copy_constructible<unusable>);
-static_assert(not simply::move_constructible<unusable>);
+// move constructible because allocator_storage
+static_assert(simply::move_constructible<unusable>);
 static_assert(not std::is_destructible_v<unusable>);
 
 static_assert(constructible_from_in_place<copyable, semiregular>);
@@ -75,7 +76,8 @@ static_assert(constructible_from_in_place<copyable, indestructible>);
 #endif
 
 static_assert(simply::copy_constructible<copyable>);
-static_assert(not simply::move_constructible<copyable>);
+// move constructible because allocator_storage
+static_assert(simply::move_constructible<copyable>);
 static_assert(not std::is_destructible_v<copyable>);
 
 static_assert(constructible_from_in_place<movable, semiregular>);
@@ -97,5 +99,6 @@ static_assert(constructible_from_in_place<destructible, immovable>);
 static_assert(not constructible_from_in_place<destructible, indestructible>);
 
 static_assert(not simply::copy_constructible<destructible>);
-static_assert(not simply::move_constructible<destructible>);
+// move constructible because allocator_storage
+static_assert(simply::move_constructible<destructible>);
 static_assert(std::is_destructible_v<destructible>);

tests/counters.cpp

@@ -1,7 +1,6 @@
 #include <simply/copyable.hpp>
 #include <simply/destructible.hpp>
 #include <simply/dyn.hpp>
-#include <simply/movable.hpp>
 
 #include <algorithm>
 #include <cassert>
@@ -12,7 +11,7 @@ template <char Label>
 struct counter {
   counter() = default;
   constexpr counter(const counter &other) : copied(other.copied + 1) {}
-  counter(counter &&other) noexcept = default;
+  counter(counter &&other) = delete;
 
   auto operator=(const counter &) -> counter & = delete;
   auto operator=(counter &&) -> counter & = delete;
@@ -61,9 +60,9 @@ struct simply::iface<labeled<LabelT>, Self> {
 
 // test dyn<countable> at compile-time
 static_assert([] {
-  struct countable
-      : simply::composes<copy_countable<int>, labeled<char>, simply::copyable,
-                         simply::movable, simply::destructible> {};
+  struct countable : simply::composes<copy_countable<int>, labeled<char>,
+                                      simply::copyable, simply::destructible> {
+  };
 
   // initialize a vector of dyn<countable> with three different counter types
   std::vector<simply::dyn<countable>> v;

tests/stream.cpp

@@ -1,7 +1,6 @@
 #include <simply/destructible.hpp>
 #include <simply/dyn.hpp>
 #include <simply/insertable.hpp>
-#include <simply/movable.hpp>
 
 #include <gtest/gtest.h>
 
@@ -11,7 +10,7 @@ TEST(Stream, TypeErasedInsertion) {
   using namespace std::string_literals;
 
   struct mixins : simply::composes<simply::insertable<std::ostream>,
-                                   simply::movable, simply::destructible> {};
+                                   simply::destructible> {};
 
   std::vector<simply::dyn<mixins>> values;
   values.emplace_back("Hello, world!"s);