opt: fix move ctor, move assign and emplace (#5)

Additionally * move all destruction logic into Opt<T>::clear(). * add Opt unittest counting object instances.
TheNumbat · Apr 8, 2024 · e24d71e · e24d71e
1 parent f00a7b4
commit e24d71e
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 16 deletions.
diff --git a/rpp/opt.h b/rpp/opt.h
@@ -30,10 +30,7 @@ struct Opt {
     }
 
     ~Opt() noexcept {
-        if constexpr(Must_Destruct<T>) {
-            if(ok_) value_.destruct();
-        }
-        ok_ = false;
+        clear();
     }
 
     Opt(const Opt& src) noexcept
@@ -46,29 +43,27 @@ struct Opt {
     Opt(Opt&& src) noexcept
         requires Move_Constructable<T>
     {
-        ok_ = src.ok_;
-        src.ok_ = false;
-        if(ok_) {
+        if(src.ok_) {
             value_.construct(rpp::move(*src.value_));
+            ok_ = true;
         }
     }
 
     Opt& operator=(Opt&& src) noexcept
         requires Move_Constructable<T>
     {
-        this->~Opt();
-        ok_ = src.ok_;
-        src.ok_ = false;
-        if(ok_) {
+        clear();
+        if(src.ok_) {
             value_.construct(rpp::move(*src.value_));
+            ok_ = true;
         }
         return *this;
     }
 
     Opt& operator=(T&& value) noexcept
         requires Move_Constructable<T>
     {
-        this->~Opt();
+        clear();
         value_.construct(rpp::move(value));
         ok_ = true;
         return *this;
@@ -78,15 +73,16 @@ struct Opt {
     void emplace(Args&&... args) noexcept
         requires Constructable<T, Args...>
     {
-        ok_ = true;
+        clear();
         value_.construct(rpp::forward<Args>(args)...);
+        ok_ = true;
     }
 
     void clear() noexcept {
-        if(ok_) {
-            value_.destruct();
-            ok_ = false;
+        if constexpr(Must_Destruct<T>) {
+            if(ok_) value_.destruct();
         }
+        ok_ = false;
     }
 
     Opt clone() const noexcept

diff --git a/test/opt.cpp b/test/opt.cpp
@@ -1,6 +1,26 @@
 
 #include "test.h"
 
+struct InstCnt {
+    InstCnt() {
+        ++cnt;
+    }
+    ~InstCnt() {
+        --cnt;
+    }
+    InstCnt(const InstCnt&) {
+        ++cnt;
+    }
+    InstCnt(InstCnt&&) {
+        ++cnt;
+    }
+    InstCnt& operator=(const InstCnt&) = delete;
+    InstCnt& operator=(InstCnt&&) = delete;
+
+    static i32 cnt;
+};
+i32 InstCnt::cnt = 0;
+
 i32 main() {
     Test test{"empty"_v};
     Trace("Storage") {
@@ -42,5 +62,53 @@ i32 main() {
 
         Opt<Thread::Mutex> m;
     }
+    Trace("OptInstCnt") {
+        Opt<InstCnt> c1;
+        assert(InstCnt::cnt == 0);
+
+        // Move assign value type.
+        c1 = InstCnt{};
+        assert(InstCnt::cnt == 1);
+        c1 = InstCnt{};
+        assert(InstCnt::cnt == 1);
+        c1.clear();
+        assert(InstCnt::cnt == 0);
+
+        // Emplace value type.
+        c1.emplace();
+        assert(InstCnt::cnt == 1);
+        c1.emplace();
+        assert(InstCnt::cnt == 1);
+        c1.clear();
+        assert(InstCnt::cnt == 0);
+
+        // Move construct from value type.
+        Opt<InstCnt> c2(InstCnt{});
+        assert(InstCnt::cnt == 1);
+
+        // Move construct from Opt type, moves inner value.
+        c1 = rpp::move(c2);
+        assert(InstCnt::cnt == 2);
+        c2.clear();
+        assert(InstCnt::cnt == 1);
+        c1.clear();
+        assert(InstCnt::cnt == 0);
+
+        // Move construct from Opt type.
+        c1.emplace();
+        Opt<InstCnt> c3(rpp::move(c1));
+        assert(InstCnt::cnt == 2);
+
+        c1.clear();
+        assert(InstCnt::cnt == 1);
+        c3.clear();
+        assert(InstCnt::cnt == 0);
+
+        // Clone and move assign Opt type.
+        c1.emplace();
+        c2 = c1.clone();
+        assert(InstCnt::cnt == 2);
+    }
+    assert(InstCnt::cnt == 0);
     return 0;
 }