Fix issue with swap skipping non-trivial destructor calls

eggs-cpp · Jan 5, 2016 · e6581d0 · e6581d0
1 parent ba0c3b3
commit e6581d0
Show file tree

Hide file tree

Showing 3 changed files with 82 additions and 4 deletions.
diff --git a/include/eggs/variant/detail/storage.hpp b/include/eggs/variant/detail/storage.hpp
@@ -620,15 +620,22 @@ namespace eggs { namespace variants { namespace detail
 
         void swap(_storage& rhs)
         {
-            if (_which == 0)
+            if (_which == rhs._which)
             {
-                base_type::swap(rhs);
+                detail::swap{}(
+                    pack<Ts...>{}, _which
+                  , target(), rhs.target()
+                );
+            } else if (_which == 0) {
+                *this = std::move(rhs);
                 rhs._destroy();
+                rhs._which = 0;
             } else if (rhs._which == 0) {
-                base_type::swap(rhs);
+                rhs = std::move(*this);
                 _destroy();
+                _which = 0;
             } else {
-                base_type::swap(rhs);
+                std::swap(*this, rhs);
             }
         }
 

diff --git a/test/dtor.hpp b/test/dtor.hpp
@@ -17,4 +17,6 @@ struct Dtor
 
 bool Dtor::called = false;
 
+void swap(Dtor&, Dtor&) {}
+
 #endif /*EGGS_VARIANT_TEST_DTOR_HPP*/
diff --git a/test/swap.cpp b/test/swap.cpp
@@ -13,6 +13,7 @@
 #define CATCH_CONFIG_MAIN
 #include "catch.hpp"
 #include "constexpr.hpp"
+#include "dtor.hpp"
 
 EGGS_CXX11_STATIC_CONSTEXPR std::size_t npos = eggs::variant<>::npos;
 
@@ -36,6 +37,23 @@ TEST_CASE("variant<Ts...>::swap(variant<Ts...>&)", "[variant.swap]")
         CHECK(*v1.target<int>() == 42);
         CHECK(v2.which() == npos);
 
+        // dtor
+        {
+            eggs::variant<int, Dtor> v1;
+            eggs::variant<int, Dtor> v2(eggs::variants::in_place<Dtor>);
+
+            REQUIRE(v1.which() == npos);
+            REQUIRE(v2.which() == 1u);
+            REQUIRE(Dtor::called == false);
+
+            v1.swap(v2);
+
+            CHECK(v1.which() == 1u);
+            CHECK(v2.which() == npos);
+            CHECK(Dtor::called == true);
+        }
+        Dtor::called = false;
+
 #if EGGS_CXX14_HAS_CONSTEXPR
         // constexpr
         {
@@ -69,6 +87,23 @@ TEST_CASE("variant<Ts...>::swap(variant<Ts...>&)", "[variant.swap]")
         REQUIRE(v2.target<int>() != nullptr);
         CHECK(*v2.target<int>() == 42);
 
+        // dtor
+        {
+            eggs::variant<int, Dtor> v1(eggs::variants::in_place<Dtor>);
+            eggs::variant<int, Dtor> v2;
+
+            REQUIRE(v1.which() == 1u);
+            REQUIRE(v2.which() == npos);
+            REQUIRE(Dtor::called == false);
+
+            v1.swap(v2);
+
+            CHECK(v1.which() == npos);
+            CHECK(v2.which() == 1u);
+            CHECK(Dtor::called == true);
+        }
+        Dtor::called = false;
+
 #if EGGS_CXX14_HAS_CONSTEXPR
         // constexpr
         {
@@ -105,6 +140,23 @@ TEST_CASE("variant<Ts...>::swap(variant<Ts...>&)", "[variant.swap]")
         REQUIRE(v2.target<int>() != nullptr);
         CHECK(*v2.target<int>() == 42);
 
+        // dtor
+        {
+            eggs::variant<int, Dtor> v1(eggs::variants::in_place<Dtor>);
+            eggs::variant<int, Dtor> v2(eggs::variants::in_place<Dtor>);
+
+            REQUIRE(v1.which() == 1u);
+            REQUIRE(v2.which() == 1u);
+            REQUIRE(Dtor::called == false);
+
+            v1.swap(v2);
+
+            CHECK(v1.which() == 1u);
+            CHECK(v2.which() == 1u);
+            CHECK(Dtor::called == false);
+        }
+        Dtor::called = false;
+
 #if EGGS_CXX14_HAS_CONSTEXPR
         // constexpr
         {
@@ -141,6 +193,23 @@ TEST_CASE("variant<Ts...>::swap(variant<Ts...>&)", "[variant.swap]")
         REQUIRE(v2.target<int>() != nullptr);
         CHECK(*v2.target<int>() == 42);
 
+        // dtor
+        {
+            eggs::variant<int, Dtor> v1(42);
+            eggs::variant<int, Dtor> v2(eggs::variants::in_place<Dtor>);
+
+            REQUIRE(v1.which() == 0u);
+            REQUIRE(v2.which() == 1u);
+            REQUIRE(Dtor::called == false);
+
+            v1.swap(v2);
+
+            CHECK(v1.which() == 1u);
+            CHECK(v2.which() == 0u);
+            CHECK(Dtor::called == true);
+        }
+        Dtor::called = false;
+
 #if EGGS_CXX14_HAS_CONSTEXPR
         // constexpr
         {
Original file line number	Diff line number	Diff line change
Expand Up		@@ -17,4 +17,6 @@ struct Dtor

		bool Dtor::called = false;

		void swap(Dtor&, Dtor&) {}

		#endif /EGGS_VARIANT_TEST_DTOR_HPP/