Use own class for MovableUniquePtr

Author: franzpoeschel

include/openPMD/auxiliary/Memory.hpp

@@ -22,16 +22,13 @@
 
 #include "openPMD/Dataset.hpp"
 #include "openPMD/Datatype.hpp"
+#include "openPMD/Error.hpp"
 #include "openPMD/auxiliary/UniquePtr.hpp"
 
 #include <any>
-#include <complex>
 #include <functional>
-#include <iostream>
 #include <memory>
-#include <type_traits>
 #include <utility>
-#include <variant>
 
 namespace openPMD
 {
@@ -52,8 +49,41 @@ namespace auxiliary
         /*
          * Sic. Have to put the unique_ptr behind a shared_ptr because
          * std::variant does not want immovable types.
+         * Use a separate class to avoid mistakes in double dereference.
          */
-        using UniquePtr = std::shared_ptr<UniquePtrWithLambda<void>>;
+        struct MovableUniquePtr
+            : private std::shared_ptr<UniquePtrWithLambda<void>>
+        {
+        private:
+            using parent_t = std::shared_ptr<UniquePtrWithLambda<void>>;
+
+        public:
+            MovableUniquePtr() = default;
+            MovableUniquePtr(UniquePtrWithLambda<void> ptr_in)
+                : parent_t{std::make_shared<UniquePtrWithLambda<void>>(
+                      std::move(ptr_in))}
+            {}
+            auto get() -> void *
+            {
+                return (**this).get();
+            }
+            [[nodiscard]] auto get() const -> void const *
+            {
+                return (**this).get();
+            }
+            [[nodiscard]] auto release() -> UniquePtrWithLambda<void>
+            {
+                if (parent_t::use_count() > 1)
+                {
+                    throw error::Internal(
+                        "Control flow error: UniquePtr variant of WriteBuffer "
+                        "has been copied.");
+                }
+                UniquePtrWithLambda<void> res = std::move(**this);
+                this->reset();
+                return res;
+            }
+        };
         using SharedPtr = std::shared_ptr<void const>;
         /*
          * Use std::any publically since some compilers have trouble with

include/openPMD/auxiliary/Memory_internal.hpp

@@ -21,12 +21,10 @@
 #pragma once
 
 #include "openPMD/auxiliary/Memory.hpp"
-#include "openPMD/auxiliary/UniquePtr.hpp"
-#include <memory>
 
 namespace openPMD::auxiliary
 {
 // cannot use a unique_ptr inside a std::variant, so we represent it with this
 using WriteBufferTypes =
-    std::variant<WriteBuffer::UniquePtr, WriteBuffer::SharedPtr>;
+    std::variant<WriteBuffer::MovableUniquePtr, WriteBuffer::SharedPtr>;
 } // namespace openPMD::auxiliary

src/IO/ADIOS/ADIOS2File.cpp

@@ -26,6 +26,7 @@
 #include "openPMD/IO/AbstractIOHandler.hpp"
 #include "openPMD/IterationEncoding.hpp"
 #include "openPMD/auxiliary/Environment.hpp"
+#include "openPMD/auxiliary/Memory.hpp"
 #include "openPMD/auxiliary/Memory_internal.hpp"
 #include "openPMD/auxiliary/StringManip.hpp"
 
@@ -115,22 +116,13 @@ void WriteDataset::call(ADIOS2File &ba, detail::BufferedPut &bp)
             }
             else if constexpr (std::is_same_v<
                                    ptr_type,
-                                   std::shared_ptr<UniquePtrWithLambda<void>>>)
+                                   auxiliary::WriteBuffer::MovableUniquePtr>)
             {
                 BufferedUniquePtrPut bput;
                 bput.name = std::move(bp.name);
                 bput.offset = std::move(bp.param.offset);
                 bput.extent = std::move(bp.param.extent);
-                /*
-                 * Note: Moving is required here since it's a unique_ptr.
-                 * std::forward<>() would theoretically work, but it
-                 * requires the type parameter and we don't have that
-                 * inside the lambda.
-                 * (ptr_type does not work for this case).
-                 */
-                // clang-format off
-                bput.data = std::move(*arg); // NOLINT(bugprone-move-forwarding-reference)
-                // clang-format on
+                bput.data = arg.release();
                 bput.dtype = bp.param.dtype;
                 ba.m_uniquePtrPuts.push_back(std::move(bput));
             }

src/auxiliary/Memory.cpp

@@ -161,7 +161,7 @@ allocatePtr(Datatype dtype, Extent const &e)
     return allocatePtr(dtype, numPoints);
 }
 
-WriteBuffer::WriteBuffer() : m_buffer(std::make_any<UniquePtr>())
+WriteBuffer::WriteBuffer() : m_buffer(std::make_any<MovableUniquePtr>())
 {}
 
 WriteBuffer::WriteBuffer(std::shared_ptr<void const> ptr)
@@ -173,8 +173,7 @@ WriteBuffer::WriteBuffer(std::shared_ptr<void const> ptr)
 
 WriteBuffer::WriteBuffer(UniquePtrWithLambda<void> ptr)
     : m_buffer(
-          std::make_any<WriteBufferTypes>(
-              std::make_shared<UniquePtrWithLambda<void>>(std::move(ptr))))
+          std::make_any<WriteBufferTypes>(MovableUniquePtr(std::move(ptr))))
 {}
 
 WriteBuffer::WriteBuffer(WriteBuffer &&) noexcept = default;
@@ -193,8 +192,8 @@ WriteBuffer const &WriteBuffer::operator=(std::shared_ptr<void const> ptr)
 
 WriteBuffer const &WriteBuffer::operator=(UniquePtrWithLambda<void> ptr)
 {
-    m_buffer = std::make_any<WriteBufferTypes>(
-        std::make_shared<UniquePtrWithLambda<void>>(std::move(ptr)));
+    m_buffer =
+        std::make_any<WriteBufferTypes>(MovableUniquePtr(std::move(ptr)));
     return *this;
 }