Implementing the updated version unique_resource.

include/unique_resource.h

@@ -26,141 +26,96 @@
 namespace sr
 {
 
-    enum class invoke_it
-    {
-        once,
-        again
-    };
+    template<class T, class TT>
+    using is_ntmocp_constructible = std::conditional_t<std::is_reference<TT>::value || !std::is_nothrow_move_constructible<TT>::value,
+                                                    typename std::is_constructible<T, TT const &>::type,
+                                                    typename std::is_constructible<T, TT>::type>;
+
+    template<class T, class TT>
+    constexpr auto is_nothrow_move_or_copy_constructible_from_v = is_ntmocp_constructible<T, TT>::value;
 
 
-    template<class Ressource, class Deleter>
-    class unique_resource_t
+    template<class R, class D>
+    class unique_resource
     {
     public:
 
-        explicit unique_resource_t(Ressource&& res, Deleter&& deleter, bool shouldrun = true) noexcept : m_resource(std::move(res)),
-                                                                                                    m_deleter(std::move(deleter)),
-                                                                                                    m_execute_on_destruction(shouldrun)
+        template<class RR, class DD,
+                std::enable_if_t<(!std::is_lvalue_reference<RR>::value)
+                                    && std::is_nothrow_constructible<R, RR>::value, int> = 0,
+                std::enable_if_t<(!std::is_lvalue_reference<DD>::value)
+                                    && std::is_nothrow_constructible<D, DD>::value, int> = 0,
+
+                std::enable_if_t<(std::is_copy_constructible<R>::value || std::is_nothrow_move_constructible<R>::value)
+                                && (std::is_copy_constructible<D>::value || std::is_nothrow_move_constructible<D>::value), int> = 0,
+                std::enable_if_t<is_nothrow_move_or_copy_constructible_from_v<R, RR>, int> = 0,
+                std::enable_if_t<is_nothrow_move_or_copy_constructible_from_v<D, DD>, int> = 0
+                >
+        explicit unique_resource(RR&& r, DD& d) : m_resource(std::move(r)), m_deleter(std::move(d))
         {
         }
 
-        unique_resource_t(const unique_resource_t&) = delete;
+        template<class RR, class DD,
+                std::enable_if_t<std::is_lvalue_reference<RR>::value || std::is_lvalue_reference<DD>::value, int> = 0,
 
-        unique_resource_t(unique_resource_t&& other) noexcept : m_resource(std::move(other.m_resource)),
-                                                            m_deleter(std::move(other.m_deleter)),
-                                                            m_execute_on_destruction(other.m_execute_on_destruction)
+                std::enable_if_t<(std::is_copy_constructible<R>::value || std::is_nothrow_move_constructible<R>::value)
+                                && (std::is_copy_constructible<D>::value || std::is_nothrow_move_constructible<D>::value), int> = 0,
+                std::enable_if_t<is_nothrow_move_or_copy_constructible_from_v<R, RR>, int> = 0,
+                std::enable_if_t<is_nothrow_move_or_copy_constructible_from_v<D, DD>, int> = 0
+                >
+        explicit unique_resource(RR&& r, DD& d) try : m_resource(r), m_deleter(d)
         {
-            other.release();
         }
-
-        ~unique_resource_t()
+        catch( ... )
         {
-            invoke(invoke_it::once);
+            d(r);
+            throw;
         }
 
-
-        void invoke(const invoke_it strategy = invoke_it::once) noexcept
+        template<class TR = R, class TD = D,
+                std::enable_if_t<(std::is_nothrow_move_constructible<TR>::value
+                                && std::is_nothrow_move_constructible<TD>::value), int> = 0
+                >
+        unique_resource(unique_resource&& other) : m_resource(std::forward<R>(other.m_resource)),
+                                                m_deleter(std::forward<D>(other.m_deleter)),
+                                                m_execute_on_destruction(std::exchange(other.m_execute_on_destruction, false))
         {
-            if( m_execute_on_destruction == true )
-            {
-                call_deleter_safe();
-            }
-
-            m_execute_on_destruction = ( strategy == invoke_it::again );
         }
 
-        Ressource release() noexcept
+        template<class TR = R, class TD = D,
+                std::enable_if_t<(!std::is_nothrow_move_constructible<TR>::value
+                                || !std::is_nothrow_move_constructible<TD>::value), int> = 0
+                >
+        unique_resource(unique_resource&& other) : m_resource(other.m_resource),
+                                                m_deleter(other.m_deleter),
+                                                m_execute_on_destruction(std::exchange(other.m_execute_on_destruction, false))
         {
-            m_execute_on_destruction = false;
-            return m_resource;
         }
 
-        void reset(Ressource&& res) noexcept
-        {
-            invoke(invoke_it::again);
-            m_resource = std::move(res);
-        }
+        unique_resource(const unique_resource&) = delete;
 
-        const Ressource& get() const noexcept
-        {
-            return m_resource;
-        }
-
-        operator const Ressource&() const noexcept
-        {
-            return m_resource;
-        }
 
-        template<class R = Ressource,
-            std::enable_if_t<std::is_pointer<R>::value &&
-                ( std::is_class<std::remove_pointer_t<R>>::value
-                    || std::is_union<std::remove_pointer_t<R>>::value ), int> = 0>
-        R operator->() const noexcept
-        {
-            return m_resource;
-        }
 
-        template<class R = Ressource,
-            std::enable_if_t<std::is_pointer<R>::value, int> = 0>
-        std::add_lvalue_reference_t<std::remove_pointer_t<Ressource>> operator*() const noexcept
+        unique_resource& operator=(unique_resource&& other)
         {
-            return *get();
         }
-
-        const Deleter& get_deleter() const noexcept
-        {
-            return m_deleter;
-        }
-
-        Deleter& get_deleter() noexcept
-        {
-            return m_deleter;
-        }
-
-        unique_resource_t& operator=(unique_resource_t&& other) noexcept
-        {
-            invoke(invoke_it::once);
-            m_resource = std::move(other.m_resource);
-            m_deleter = std::move(other.m_deleter);
-            m_execute_on_destruction = other.m_execute_on_destruction;
-            other.release();
-
-            return *this;
-        }
-
-
-        unique_resource_t& operator=(const unique_resource_t&) = delete;
+        unique_resource& operator=(const unique_resource&) = delete;
 
 
     private:
 
-        void call_deleter_safe() noexcept
-        {
-            try
-            {
-                get_deleter()(m_resource);
-            }
-            catch( ... ) { /* Empty */ }
-        }
-
-        Ressource m_resource;
-        Deleter m_deleter;
+        R m_resource;
+        D m_deleter;
         bool m_execute_on_destruction;
     };
 
 
-
-    template<class Ressource, class Deleter>
-    unique_resource_t<Ressource, Deleter> unique_resource(Ressource&& res, Deleter d) noexcept
+    template<class Resource, class Deleter>
+    unique_resource<std::decay_t<Resource>, std::decay_t<Deleter>> make_unique_resource(Resource&& r, Deleter&& d)
     {
-        return unique_resource_t<Ressource, Deleter>{std::move(res), std::move(d), true};
+        return unique_resource<std::decay_t<Resource>, std::decay_t<Deleter>>{std::forward<Resource>(r), std::forward<Deleter>(d)};
     }
 
-    template<class Ressource, class Deleter>
-    unique_resource_t<Ressource, Deleter> unique_resource_checked(Ressource res, Ressource invalid, Deleter d) noexcept
-    {
-        return unique_resource_t<Ressource, Deleter>{std::move(res), std::move(d), (res != invalid)};
-    }
 
 }
 

test/UniqueResourceTest.cpp

@@ -19,10 +19,11 @@
  */
 
 #include "unique_resource.h"
-#include <functional>
 #include <catch.hpp>
 #include <trompeloeil.hpp>
 
+#include <type_traits>
+
 using namespace trompeloeil;
 
 namespace
@@ -42,205 +43,88 @@ namespace
     {
         m.deleter(h);
     }
-}
 
 
-TEST_CASE("deleter called on destruction", "[UniqueResource]")
-{
-    REQUIRE_CALL(m, deleter(3));
+    struct ThrowOnCopyMock
+    {
+        ThrowOnCopyMock() {  }
 
-    auto guard = sr::unique_resource(Handle{3}, deleter);
-    static_cast<void>(guard);
-}
+        ThrowOnCopyMock(const ThrowOnCopyMock&)
+        {
+            throw std::exception{};
+        }
 
-TEST_CASE("deleter is not called if released", "[UniqueResource]")
-{
-    REQUIRE_CALL(m, deleter(3)).TIMES(0);
-    auto guard = sr::unique_resource(Handle{3}, deleter);
-    guard.release();
-}
+        MAKE_CONST_MOCK1(deleter, void(ThrowOnCopyMock));
 
-TEST_CASE("deleter called if checked valid", "[UniqueResource]")
-{
-    REQUIRE_CALL(m, deleter(3));
-    auto guard = sr::unique_resource_checked(Handle{3}, Handle{6}, deleter);
-    static_cast<void>(guard);
-}
+        ThrowOnCopyMock& operator=(const ThrowOnCopyMock&)
+        {
+            throw std::exception{};
+        }
+    };
 
-TEST_CASE("deleter not called if checked invalid", "[UniqueResource]")
-{
-    REQUIRE_CALL(m, deleter(3)).TIMES(0);
-    auto guard = sr::unique_resource_checked(Handle{3}, Handle{3}, deleter);
-    static_cast<void>(guard);
-}
+    struct NotNothrowMoveMock
+    {
+        NotNothrowMoveMock(CallMock* mo) : m_mock(mo) { }
+        NotNothrowMoveMock(const NotNothrowMoveMock& other) : m_mock(other.m_mock)  { }
+        NotNothrowMoveMock(NotNothrowMoveMock&& other) noexcept(false) : m_mock(other.m_mock) { }
 
-TEST_CASE("release returns reference to resource", "[UniqueResource]")
-{
-    auto guard = sr::unique_resource(Handle{3}, [](auto) { });
-    const auto result = guard.release();
+        void operator()(Handle h) const
+        {
+            m_mock->deleter(h);
+        }
 
-    REQUIRE(3 == result);
-}
+        NotNothrowMoveMock& operator=(const NotNothrowMoveMock&)
+        {
+            throw "Not implemented";
+        }
 
-TEST_CASE("move releases moved-from object", "[UniqueResource]")
-{
-    REQUIRE_CALL(m, deleter(3));
-    auto movedFrom = sr::unique_resource(Handle{3}, deleter);
-    auto guard = std::move(movedFrom);
-    static_cast<void>(guard);
-}
-
-TEST_CASE("move transfers state", "[UniqueResource]")
-{
-    REQUIRE_CALL(m, deleter(3));
-    auto movedFrom = sr::unique_resource(Handle{3}, deleter);
-    auto guard = std::move(movedFrom);
-    static_cast<void>(guard);
-}
+        NotNothrowMoveMock& operator=(NotNothrowMoveMock&&)
+        {
+            throw "Not implemented";
+        }
 
-TEST_CASE("move transfers state if released", "[UniqueResource]")
-{
-    REQUIRE_CALL(m, deleter(ANY(Handle))).TIMES(0); // TODO: Use ANY(T)
+        CallMock* m_mock;
 
-    auto movedFrom = sr::unique_resource(Handle{3}, deleter);
-    movedFrom.release();
-    auto guard = std::move(movedFrom);
-    static_cast<void>(guard);
-}
+    };
 
-TEST_CASE("move assignment releases moved-from object", "[UniqueResource]")
-{
-    REQUIRE_CALL(m, deleter(5));
-    REQUIRE_CALL(m, deleter(3));
 
-    auto movedFrom = sr::unique_resource(Handle{3}, deleter);
-    auto guard = sr::unique_resource(Handle{5}, deleter);
-    guard = std::move(movedFrom);
-    static_cast<void>(guard);
 }
 
-TEST_CASE("move assignment transfers state", "[UniqueResource]")
+TEST_CASE("construction with move", "[UniqueResource]")
 {
-    REQUIRE_CALL(m, deleter(5));
-    REQUIRE_CALL(m, deleter(3));
-
-    auto movedFrom = sr::unique_resource(Handle{3}, deleter);
-    auto guard = sr::unique_resource(Handle{5}, deleter);
-    guard = std::move(movedFrom);
+    auto guard = sr::make_unique_resource(Handle{3}, deleter);
     static_cast<void>(guard);
 }
 
-TEST_CASE("move assignment transfers state if released", "[UniqueResource]")
+TEST_CASE("construction with copy calls deleter and rethrows on failed copy", "[UniqueResource]")
 {
-    REQUIRE_CALL(m, deleter(5));
+    REQUIRE_THROWS([] {
+        const ThrowOnCopyMock noMove;
+        const auto d = [](const auto&) { m.deleter(3); };
+        REQUIRE_CALL(m, deleter(3));
 
-    auto movedFrom = sr::unique_resource(Handle{3}, deleter);
-    movedFrom.release();
-    auto guard = sr::unique_resource(Handle{5}, deleter);
-    guard = std::move(movedFrom);
-    static_cast<void>(guard);
-}
-
-TEST_CASE("no exception propagation from deleter", "[UniqueResource]")
-{
-    REQUIRE_NOTHROW([] {
-        auto guard = sr::unique_resource(Handle{3}, [](auto) { throw "Don't propagate this!"; });
+        sr::unique_resource<decltype(noMove), decltype(d)> guard{noMove, d};
         static_cast<void>(guard);
-        }());
+    }());
 }
 
-TEST_CASE("invoke executes deleter on resource", "[UniqueResource]")
+TEST_CASE("move-construction with move", "[UniqueResource]")
 {
-    REQUIRE_CALL(m, deleter(3));
-
-    auto guard = sr::unique_resource(Handle{3}, deleter);
-    guard.invoke();
-}
-
-TEST_CASE("invoke executes only multiple times if again strategy", "[UniqueResource]")
-{
-    REQUIRE_CALL(m, deleter(3)).TIMES(3);
-
-    auto guard = sr::unique_resource(Handle{3}, deleter);
-    guard.invoke(sr::invoke_it::again);
-    guard.invoke(sr::invoke_it::again);
-}
-
-TEST_CASE("invoke does nothing if released", "[UniqueResource]")
-{
-    REQUIRE_CALL(m, deleter(ANY(Handle))).TIMES(0);
-
-    auto guard = sr::unique_resource(Handle{3}, deleter);
-    guard.release();
-    guard.invoke(sr::invoke_it::once);
-}
-
-TEST_CASE("invoke executes after release if again strategy", "[UniqueResource]")
-{
-    REQUIRE_CALL(m, deleter(3));
-
-    auto guard = sr::unique_resource(Handle{3}, deleter);
-    guard.release();
-    guard.invoke(sr::invoke_it::again);
-}
-
-TEST_CASE("invoke does not propagate exception", "[UniqueResource]")
-{
-    auto guard = sr::unique_resource(Handle{3}, [](auto) { throw "Don't propagate this!"; });
-    REQUIRE_NOTHROW(guard.invoke());
-}
-
-TEST_CASE("reset releases old ressource", "[UniqueResource]")
-{
-    REQUIRE_CALL(m, deleter(3));
-    REQUIRE_CALL(m, deleter(7));
-
-    auto guard = sr::unique_resource(Handle{3}, deleter);
-    guard.reset(Handle{7});
-}
-
-TEST_CASE("reset sets ressource", "[UniqueResource]")
-{
-    auto guard = sr::unique_resource(Handle{3}, [](auto) { });
-    guard.reset(Handle{7});
-    REQUIRE(guard.get() == 7);
-}
-
-TEST_CASE("get accesses ressource", "[UniqueResource]")
-{
-    auto guard = sr::unique_resource(Handle{3}, [](auto) { });
-    REQUIRE(guard.get() == 3);
-}
-
-TEST_CASE("conversion operator", "[UniqueResource]")
-{
-    auto guard = sr::unique_resource(Handle{3}, [](auto) { });
-    const auto& ref = guard;
-    REQUIRE(ref == 3);
-}
-
-TEST_CASE("pointer access operator" "[UniqueResource]")
-{
-    const auto p = std::make_pair(3, 4);
-    auto guard = sr::unique_resource(&p, [](auto) { });
-    const auto x = guard.operator->();
-    REQUIRE(x->first == 3);
-}
-
-TEST_CASE("dereference operator", "[UniqueResource]")
-{
-    Handle h{4};
-    auto guard = sr::unique_resource(PtrHandle{&h}, [](auto) { });
-    const auto x = guard.operator*();
-    REQUIRE(x == 4);
+    auto movedFrom = sr::make_unique_resource(Handle{3}, deleter);
+    auto guard = std::move(movedFrom);
+    static_cast<void>(guard);
+    // TODO: Check value of guard
 }
 
-TEST_CASE("deleter access", "[UniqueResource]")
+TEST_CASE("move-construction with copy", "[UniqueResource]")
 {
-    std::size_t value{0};
-    auto guard = sr::unique_resource(Handle{3}, [&value](auto v) { value = v; });
-    REQUIRE(value == 0);
-    guard.get_deleter()(6);
-    REQUIRE(value == 6);
+    CallMock mock;
+    const NotNothrowMoveMock notNothrow{&mock};
+    Handle h{3};
+    sr::unique_resource<Handle, decltype(notNothrow)> movedFrom{h, notNothrow};
+    auto guard = std::move(movedFrom);
+    static_cast<void>(guard);
+    // TODO: Check value of guard
 }
 
+// TODO: Implement move assignment