Merge pull request #71 from bluescarni/pr/better_invalid

bluescarni · web-flow · commit 1fd935db49dc · 2025-12-20T21:25:50.000+01:00
Better semantics for the invalid state
diff --git a/README.md b/README.md
@@ -10,7 +10,7 @@ tanuki
 <br />
 <p align="center">
   <p align="center">
-    A type-erasure toolkit for C++20
+    A type-erasure toolkit for C++20/23
     <br />
     <a href="https://bluescarni.github.io/tanuki/index.html"><strong>Explore the docs »</strong></a>
     <br />
diff --git a/doc/config.rst b/doc/config.rst
@@ -49,21 +49,27 @@ Configuration options
 
       This option selects the semantics for the :cpp:class:`wrap` class.
 
-   .. cpp:var:: bool copyable = true
+   .. cpp:var:: bool copy_constructible = true
 
-      This option selects whether or not the :cpp:class:`wrap` class is copy constructible/assignable.
+      This option selects whether or not the :cpp:class:`wrap` class is copy-constructible.
 
       This option is ignored when employing :ref:`reference semantics <ref_semantics>`.
 
-   .. cpp:var:: bool movable = true
+   .. cpp:var:: bool copy_assignable = true
 
-      This option selects whether or not the :cpp:class:`wrap` class is move constructible/assignable.
+      This option selects whether or not the :cpp:class:`wrap` class is copy-assignable.
 
       This option is ignored when employing :ref:`reference semantics <ref_semantics>`.
 
-   .. cpp:var:: bool swappable = true
+   .. cpp:var:: bool move_constructible = true
 
-      This option selects whether or not the :cpp:class:`wrap` class is swappable.
+      This option selects whether or not the :cpp:class:`wrap` class is move-constructible.
+
+      This option is ignored when employing :ref:`reference semantics <ref_semantics>`.
+
+   .. cpp:var:: bool move_assignable = true
+
+      This option selects whether or not the :cpp:class:`wrap` class is move-assignable.
 
       This option is ignored when employing :ref:`reference semantics <ref_semantics>`.
 
@@ -132,7 +138,7 @@ Configuration options
    - :cpp:var:`config::static_align` is a power of two,
    - :cpp:var:`config::explicit_ctor` is one of the enumerators defined in :cpp:enum:`wrap_ctor`,
    - :cpp:var:`config::semantics` is one of the enumerators defined in :cpp:enum:`wrap_semantics`,
-   - if :cpp:var:`config::copyable` is set to ``true``, so is :cpp:var:`config::movable` (that is,
-     a copyable :cpp:class:`wrap` must also be movable),
-   - if :cpp:var:`config::movable` is set to ``true``, so is :cpp:var:`config::swappable` (that is,
-     a movable :cpp:class:`wrap` must also be swappable).
+   - if :cpp:var:`config::copy_assignable` is set to ``true``, so is :cpp:var:`config::copy_constructible` (that is,
+     copy-assignability requires copy-constructibility),
+   - if :cpp:var:`config::move_assignable` is set to ``true``, so is :cpp:var:`config::move_constructible` (that is,
+     move-assignability requires move-constructibility).
diff --git a/doc/custom_construct.rst b/doc/custom_construct.rst
@@ -49,9 +49,8 @@ A :cpp:class:`wrap` object can become invalid in a variety of circumstances:
 
 The only allowed operations on an invalid :cpp:class:`wrap` are:
 
-- destruction,
+- copy/move construction/assignment (including via :cpp:func:`wrap::copy()`), swapping and destruction,
 - the invocation of :cpp:func:`~wrap::is_invalid()` and :cpp:func:`is_valid()`,
-- copy/move assignment from, and swapping with, a valid :cpp:class:`wrap`,
 - :ref:`emplacement <emplacement>`,
 - generic assignment.
 
diff --git a/doc/wrap.rst b/doc/wrap.rst
@@ -17,7 +17,7 @@ The ``wrap`` class
       - a non-``void`` default-initialisable :cpp:type:`~config::DefaultValueType` with a valid, default-initialisable
         interface implementation for :cpp:type:`IFace` has been specified as first template argument in :cpp:var:`Cfg`.
         In this case, the default constructor value-initialises an instance of :cpp:type:`~config::DefaultValueType` as
-        the internal type-erased value. When employing value semantics, the copyability, movability and swappability of
+        the internal type-erased value. When employing value semantics, the copyability and movability of
         :cpp:type:`~config::DefaultValueType` must be consistent with the corresponding settings
         in :cpp:var:`Cfg` in order for this constructor to be enabled.
 
@@ -63,7 +63,7 @@ The ``wrap`` class
       - the interface :cpp:type:`IFace` has a valid, default-initialisable implementation for the value type :cpp:type:`T`
         (see the :cpp:concept:`iface_with_impl` concept);
       - :cpp:var:`x` can be perfectly-forwarded to construct an instance of the value type;
-      - when employing value semantics, the copyability, movability and swappability of the value type
+      - when employing value semantics, the copyability and movability of the value type
         are consistent with the corresponding settings in :cpp:var:`Cfg`.
 
       This constructor is marked ``explicit`` if either:
@@ -98,7 +98,7 @@ The ``wrap`` class
       - the interface :cpp:type:`IFace` has a valid, default-initialisable implementation for the value type :cpp:type:`T`
         (see the :cpp:concept:`iface_with_impl` concept);
       - :cpp:var:`args` can be perfectly-forwarded to construct an instance of the value type :cpp:type:`T`;
-      - when employing value semantics, the copyability, movability and swappability of the value type :cpp:type:`T`
+      - when employing value semantics, the copyability and movability of the value type :cpp:type:`T`
         are consistent with the corresponding settings in :cpp:var:`Cfg`.
 
       :param args: the input construction arguments.
@@ -167,7 +167,7 @@ The ``wrap`` class
       - an instance of :cpp:type:`T` can be constructed from :cpp:type:`Args`;
       - the interface :cpp:type:`IFace` has a valid, default-initialisable implementation for the value type :cpp:type:`T`
         (see the :cpp:concept:`iface_with_impl` concept);
-      - when employing value semantics, the copyability, movability and swappability of the value type :cpp:type:`T`
+      - when employing value semantics, the copyability and movability of the value type :cpp:type:`T`
         are consistent with the corresponding settings in :cpp:var:`Cfg`.
 
       Passing :cpp:var:`w` as an argument in :cpp:var:`args` (e.g., attempting to emplace :cpp:var:`w` into itself) will lead to
diff --git a/include/tanuki/tanuki.hpp b/include/tanuki/tanuki.hpp
@@ -1527,21 +1527,27 @@ class TANUKI_VISIBLE wrap : private detail::wrap_storage<IFace, Cfg.static_size,
     }
 
 private:
+    // Implementation of copy-initialisation for value semantics.
     void copy_init_from(const wrap &other)
     {
         static_assert(Cfg.semantics == wrap_semantics::value);
 
-        if constexpr (Cfg.static_size == 0u) {
-            // Static storage disabled.
-            this->m_pv_iface = other.m_pv_iface->_tanuki_clone_holder();
-        } else {
-            if (other.stype()) {
-                // Other has static storage.
-                this->m_pv_iface = other.m_pv_iface->_tanuki_copy_init_holder(this->static_storage);
-            } else {
-                // Other has dynamic storage.
+        if (is_valid(other)) {
+            if constexpr (Cfg.static_size == 0u) {
+                // Static storage disabled.
                 this->m_pv_iface = other.m_pv_iface->_tanuki_clone_holder();
+            } else {
+                if (other.stype()) {
+                    // Other has static storage.
+                    this->m_pv_iface = other.m_pv_iface->_tanuki_copy_init_holder(this->static_storage);
+                } else {
+                    // Other has dynamic storage.
+                    this->m_pv_iface = other.m_pv_iface->_tanuki_clone_holder();
+                }
             }
+        } else {
+            // Handle initialisation from an invalid wrap.
+            this->m_pv_iface = nullptr;
         }
     }
 
@@ -1560,34 +1566,42 @@ class TANUKI_VISIBLE wrap : private detail::wrap_storage<IFace, Cfg.static_size,
     }
 
 private:
+    // Implementation of move-initialisation for value semantics.
+    //
     // NOLINTNEXTLINE(cppcoreguidelines-rvalue-reference-param-not-moved)
     void move_init_from(wrap &&other) noexcept
     {
         static_assert(Cfg.semantics == wrap_semantics::value);
 
-        if constexpr (Cfg.static_size == 0u) {
-            // Static storage disabled. Shallow copy the pointer.
-            this->m_pv_iface = other.m_pv_iface;
-
-            // Invalidate other.
-            other.m_pv_iface = nullptr;
-        } else {
-            auto *pv_iface = other.m_pv_iface;
-
-            if (other.stype()) {
-                // Other has static storage.
-                this->m_pv_iface = std::move(*pv_iface)._tanuki_move_init_holder(this->static_storage);
-            } else {
-                // Other has dynamic storage.
-                this->m_pv_iface = pv_iface;
+        if (is_valid(other)) {
+            if constexpr (Cfg.static_size == 0u) {
+                // Static storage disabled. Shallow copy the pointer.
+                this->m_pv_iface = other.m_pv_iface;
 
                 // Invalidate other.
                 other.m_pv_iface = nullptr;
+            } else {
+                auto *pv_iface = other.m_pv_iface;
+
+                if (other.stype()) {
+                    // Other has static storage.
+                    this->m_pv_iface = std::move(*pv_iface)._tanuki_move_init_holder(this->static_storage);
+                } else {
+                    // Other has dynamic storage.
+                    this->m_pv_iface = pv_iface;
+
+                    // Invalidate other.
+                    other.m_pv_iface = nullptr;
+                }
             }
+        } else {
+            // Handle initialisation from an invalid wrap.
+            this->m_pv_iface = nullptr;
         }
     }
 
 public:
+    // Move constructor.
     wrap(wrap &&other) noexcept
         requires(Cfg.move_constructible || Cfg.semantics == wrap_semantics::reference)
                 && std::default_initializable<ref_iface_t>
@@ -1635,13 +1649,22 @@ class TANUKI_VISIBLE wrap : private detail::wrap_storage<IFace, Cfg.static_size,
                 return *this;
             }
 
-            // Handle invalid object.
+            // Handle invalid this.
             if (is_invalid(*this)) {
                 // No need to destroy, just move-init from other is sufficient.
+                //
+                // NOTE: move_init_from() will work fine if other is invalid.
                 move_init_from(std::move(other));
                 return *this;
             }
 
+            // this is valid, handle invalid other.
+            if (is_invalid(other)) {
+                destroy();
+                this->m_pv_iface = nullptr;
+                return *this;
+            }
+
             // Helper to implement move-assignment via destruction + move-initialisation.
             const auto destroy_and_move_init = [this, &other]() noexcept {
                 destroy();
@@ -1696,15 +1719,24 @@ class TANUKI_VISIBLE wrap : private detail::wrap_storage<IFace, Cfg.static_size,
                 return *this;
             }
 
-            // Handle invalid object.
+            // Handle invalid this.
             if (is_invalid(*this)) {
                 // No need to destroy, just copy-init from other is sufficient.
                 //
                 // NOTE: copy_init_from() either succeeds or fails, no intermediate state is possible.
+                //
+                // NOTE: copy_init_from() will work fine if other is invalid.
                 copy_init_from(other);
                 return *this;
             }
 
+            // this is valid, handle invalid other.
+            if (is_invalid(other)) {
+                destroy();
+                this->m_pv_iface = nullptr;
+                return *this;
+            }
+
             // NOTE: the idea here is as follows:
             //
             // - if the internal types are the same and copy-assignment is possible, employ it; otherwise,
@@ -1734,7 +1766,7 @@ class TANUKI_VISIBLE wrap : private detail::wrap_storage<IFace, Cfg.static_size,
         return *this;
     }
 
-    // Assignment to the invalid state.
+    // Assignment from the invalid state.
     wrap &operator=(invalid_wrap_t) noexcept
     {
         if constexpr (Cfg.semantics == wrap_semantics::value) {
@@ -2036,7 +2068,12 @@ class TANUKI_VISIBLE wrap : private detail::wrap_storage<IFace, Cfg.static_size,
         requires(Cfg.semantics == wrap_semantics::reference)
     {
         wrap retval(invalid_wrap);
-        retval.m_pv_iface = w.m_pv_iface->_tanuki_shared_clone_holder();
+        // NOTE: perform the deep copy only if w is valid. Otherwise, return an invalid wrap.
+        if (is_valid(w)) {
+            retval.m_pv_iface = w.m_pv_iface->_tanuki_shared_clone_holder();
+        } else {
+            ;
+        }
         return retval;
     }
 
diff --git a/test/test_basics.cpp b/test/test_basics.cpp
@@ -182,6 +182,13 @@ TEST_CASE("basics")
     // Large one.
     auto w2copy = w2;
     REQUIRE(value_ref<large>(w2copy).buffer[0] == 2);
+    // Construction from invalid wrap.
+    {
+        const wrap_t inv_wrap(tanuki::invalid_wrap);
+        // NOLINTNEXTLINE
+        const auto inv_wrap_copy(inv_wrap);
+        REQUIRE(!is_valid(inv_wrap_copy));
+    }
 
     // Move constructing.
     // Small one.
@@ -197,6 +204,8 @@ TEST_CASE("basics")
     REQUIRE(is_invalid(w2));
     // NOLINTEND
     REQUIRE(has_dynamic_storage(w2));
+    // Construction from invalid wrap.
+    REQUIRE(!is_valid(wrap_t(wrap_t(tanuki::invalid_wrap))));
 
     // Emplace test with class which is not copyable/movable/swappable.
     wrap<any_iface, tanuki::config<>{.copy_constructible = false,
@@ -370,6 +379,36 @@ TEST_CASE("assignment")
         // NOLINTNEXTLINE
         REQUIRE(!is_invalid(w2));
     }
+
+    // Move assignment between invalid objects.
+    {
+        wrap_t w1{tanuki::invalid_wrap}, w2{tanuki::invalid_wrap};
+        w1 = std::move(w2);
+        REQUIRE(is_invalid(w1));
+        // NOLINTNEXTLINE
+        REQUIRE(is_invalid(w2));
+    }
+    {
+        wrap_t w1{large{.str = "blif"}}, w2{tanuki::invalid_wrap};
+        w1 = std::move(w2);
+        REQUIRE(is_invalid(w1));
+        // NOLINTNEXTLINE
+        REQUIRE(is_invalid(w2));
+    }
+
+    // Copy assignment between invalid objects.
+    {
+        wrap_t w1{tanuki::invalid_wrap}, w2{tanuki::invalid_wrap};
+        w1 = w2;
+        REQUIRE(is_invalid(w1));
+        REQUIRE(is_invalid(w2));
+    }
+    {
+        wrap_t w1{large{.str = "blif"}}, w2{tanuki::invalid_wrap};
+        w1 = w2;
+        REQUIRE(is_invalid(w1));
+        REQUIRE(is_invalid(w2));
+    }
 }
 
 // NOTE: a small test to check the cleanup logic in the copy assignment operator of wrap when the assignment is
diff --git a/test/test_ref_semantics.cpp b/test/test_ref_semantics.cpp
@@ -115,6 +115,9 @@ TEST_CASE("basics")
     REQUIRE(value_isa<int>(w6));
     REQUIRE(value_ref<int>(w6) == 11);
     REQUIRE(tanuki::value_ptr<int>(w5) == tanuki::value_ptr<int>(w6));
+    // NOLINTNEXTLINE
+    auto w6a(w1);
+    REQUIRE(is_invalid(w6a));
 
     // Move construction.
     REQUIRE(std::is_nothrow_copy_constructible_v<wrap2_t>);
@@ -123,6 +126,7 @@ TEST_CASE("basics")
     REQUIRE(value_isa<int>(w7));
     REQUIRE(value_ref<int>(w7) == 11);
     REQUIRE(tanuki::value_ptr<int>(w5) == tanuki::value_ptr<int>(w7));
+    REQUIRE(is_invalid(wrap1_t(wrap1_t(tanuki::invalid_wrap))));
 
     // Move assignment.
     REQUIRE(std::is_nothrow_move_assignable_v<wrap2_t>);
@@ -132,6 +136,14 @@ TEST_CASE("basics")
     REQUIRE(value_isa<int>(w8));
     REQUIRE(value_ref<int>(w8) == 11);
     REQUIRE(tanuki::value_ptr<int>(w5) == tanuki::value_ptr<int>(w8));
+    // Move assignment between invalid objects.
+    {
+        wrap2_t winv1{tanuki::invalid_wrap}, winv2{tanuki::invalid_wrap};
+        winv1 = std::move(winv2);
+        REQUIRE(is_invalid(winv1));
+        // NOLINTNEXTLINE
+        REQUIRE(is_invalid(winv2));
+    }
 
     // Copy assignment.
     REQUIRE(std::is_nothrow_copy_assignable_v<wrap2_t>);
@@ -141,6 +153,13 @@ TEST_CASE("basics")
     REQUIRE(value_isa<int>(w9));
     REQUIRE(value_ref<int>(w9) == 11);
     REQUIRE(tanuki::value_ptr<int>(w5) == tanuki::value_ptr<int>(w9));
+    // Copy assignment between invalid objects.
+    {
+        wrap2_t winv1{tanuki::invalid_wrap}, winv2{tanuki::invalid_wrap};
+        winv1 = winv2;
+        REQUIRE(is_invalid(winv1));
+        REQUIRE(is_invalid(winv2));
+    }
 
     // Generic assignment.
     wrap2_t w10;
@@ -222,6 +241,10 @@ TEST_CASE("basics")
 
     const wrap2_t w13(noncopyable{});
     REQUIRE_THROWS_MATCHES(copy(w13), std::invalid_argument, Message("Attempting to clone a non-copyable value type"));
+
+    // Deep copy of invalid wrap.
+    const wrap2_t w14(tanuki::invalid_wrap);
+    REQUIRE(is_invalid(copy(w14)));
 }
 
 #if defined(TANUKI_WITH_BOOST_S11N)
